From b185f35cd4de4ba20f60ac0b2dc5876e6c710343 Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 31 Oct 2026 13:42:02 -0400 Subject: [PATCH 1/4] ci(deploy): retry the website deploy step on a transient Cloudflare failure Run 40103130697 failed at `Deploy the preview worker` with Cloudflare API error code 10013 after the build had already succeeded, and the next push deployed the same commit successfully. Both website deploys now run through .github/scripts/deploy-with-retry.sh, which wraps the deploy step and not the build. Three attempts, 10 s then 30 s backoff, a per-attempt timeout and a step-level timeout-minutes bound the added time to about a minute. Only code 10013, an HTTP 5xx, a connection reset and an expired attempt timeout are retried: an authentication failure, a missing environment and a wrangler validation error still fail on the first attempt, because retrying those triples the noise and delays the real message. Every retry emits a ::warning:: naming the attempt and the reason, so the flake rate stays visible instead of disappearing behind a passing job. The command is passed through from each workflow, so the preview keeps its `npx wrangler deploy --config wrangler.preview.jsonc` and production keeps `pnpm exec wrangler deploy --config wrangler.router.jsonc --var CANONICAL_HOST:$CANONICAL_HOST`. test/unit/scripts/deploy-with-retry.spec.ts runs the real script against a stub wrangler: a 10013 is retried once with a warning naming attempt 2, a 401 gets one attempt and no retry, a 5xx and a connection reset are retried, three transient failures exit non-zero with the last error on stderr, the arguments survive unchanged, and an attempt that exceeds its timeout is retried. Disabling the retry makes five of the eight fail. --- .changeset/deploy-retry-transient.md | 19 ++ .github/scripts/deploy-with-retry.sh | 112 ++++++++++++ .github/workflows/checks.yml | 3 + .github/workflows/deploy-website-preview.yml | 13 +- .github/workflows/deploy-website.yml | 8 +- test/unit/scripts/deploy-with-retry.spec.ts | 177 +++++++++++++++++++ 6 files changed, 330 insertions(+), 2 deletions(-) create mode 100644 .changeset/deploy-retry-transient.md create mode 100755 .github/scripts/deploy-with-retry.sh create mode 100644 test/unit/scripts/deploy-with-retry.spec.ts diff --git a/.changeset/deploy-retry-transient.md b/.changeset/deploy-retry-transient.md new file mode 100644 index 000000000..0063d9ff9 --- /dev/null +++ b/.changeset/deploy-retry-transient.md @@ -0,0 +1,19 @@ +--- +"nostream": patch +--- + +ci(deploy): retry the website deploy on a transient Cloudflare failure + +Run 40103130697 failed at `Deploy the preview worker` with Cloudflare API error +`code: 10013` after a build that had already succeeded, and the next push deployed +the same commit successfully — a transient upstream failure, not a defect in the +site or the workflow. + +Both website deploys now run through `.github/scripts/deploy-with-retry.sh`, which +wraps the deploy step (not the build) in three attempts with a 10 s then 30 s +backoff, a per-attempt timeout, and a step-level `timeout-minutes`. It retries only +`code: 10013`, an HTTP 5xx, a connection reset and an expired attempt timeout; an +authentication failure, a missing environment or a wrangler validation error still +fails on the first attempt. Every retry emits a `::warning::` naming the attempt and +the reason, so the flake rate stays visible instead of disappearing behind a +passing job. diff --git a/.github/scripts/deploy-with-retry.sh b/.github/scripts/deploy-with-retry.sh new file mode 100755 index 000000000..aa2fb9fe5 --- /dev/null +++ b/.github/scripts/deploy-with-retry.sh @@ -0,0 +1,112 @@ +#!/usr/bin/env bash +# +# Run a wrangler deploy, retrying only a transient Cloudflare failure. +# +# .github/scripts/deploy-with-retry.sh +# +# The command is taken from the caller so each workflow keeps its own +# invocation (`npx wrangler deploy --config wrangler.preview.jsonc` for a +# preview, `pnpm exec wrangler deploy --config wrangler.router.jsonc` with +# `--var` for production). +# +# Only three signals are retried, because retrying anything else triples the +# noise and delays the message that matters: +# +# - `code: 10013`, the Cloudflare API's "unknown error" seen in #970 +# - an HTTP 5xx from the Cloudflare API +# - a connection reset +# +# An expired per-attempt timeout is retried too: a hung upload is the case the +# timeout exists for, and failing on the first one would defeat it. +# +# Authentication failures, a missing environment and wrangler validation +# errors fail on the first attempt. +# +# Every attempt after the first is announced with a `::warning::` naming the +# attempt and the reason, so the flake rate stays visible in the Actions UI +# instead of disappearing behind a passing job. +# +# Environment: +# DEPLOY_ATTEMPTS total attempts (default 3) +# DEPLOY_ATTEMPT_TIMEOUT seconds allowed per attempt (default 180) +# DEPLOY_BACKOFFS seconds to wait before each retry (default "10 30") +set -uo pipefail + +attempts=${DEPLOY_ATTEMPTS:-3} +attempt_timeout=${DEPLOY_ATTEMPT_TIMEOUT:-180} +read -r -a backoffs <<< "${DEPLOY_BACKOFFS:-10 30}" + +if [ "$#" -eq 0 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +if ! [[ "$attempts" =~ ^[1-9][0-9]*$ ]]; then + echo "DEPLOY_ATTEMPTS must be a positive integer, got '$attempts'" >&2 + exit 2 +fi + +logs=$(mktemp -d) +trap 'rm -rf "$logs"' EXIT + +# Echo the transient reason found in an attempt's output, or return 1 when the +# failure is one we must not retry. +transient_reason() { + local log=$1 + local http + + if grep -qE 'code: ?10013' "$log"; then + printf 'Cloudflare API error code 10013' + return 0 + fi + http=$(grep -oE 'HTTP 5[0-9][0-9]' "$log" | head -1 || true) + if [ -n "$http" ]; then + printf '%s from the Cloudflare API' "$http" + return 0 + fi + if grep -qiE 'ECONNRESET|connection reset|socket hang up' "$log"; then + printf 'connection reset' + return 0 + fi + + return 1 +} + +for (( attempt = 1; attempt <= attempts; attempt++ )); do + log="$logs/attempt-$attempt.log" + status=0 + echo "deploy attempt $attempt/$attempts: $*" + # The whole attempt is bounded: an upload that stops making progress must not + # hold the job until the workflow's own timeout. `--kill-after` covers a + # process that ignores TERM. + timeout --kill-after=10 "$attempt_timeout" "$@" >"$log" 2>&1 || status=$? + cat "$log" + + if [ "$status" -eq 0 ]; then + echo "deploy succeeded on attempt $attempt/$attempts" + exit 0 + fi + + reason=$(transient_reason "$log") || reason= + if [ -z "$reason" ]; then + if [ "$status" -eq 124 ] || [ "$status" -eq 137 ]; then + reason="attempt timed out after ${attempt_timeout}s" + else + echo "::error::deploy failed on attempt $attempt/$attempts with a non-transient error (exit $status) — not retrying" >&2 + tail -n 30 "$log" >&2 + exit "$status" + fi + fi + + if [ "$attempt" -ge "$attempts" ]; then + echo "::error::deploy failed after $attempts attempts — last failure: $reason" >&2 + tail -n 30 "$log" >&2 + exit "$status" + fi + + delay=${backoffs[$((attempt - 1))]:-0} + echo "::warning::wrangler deploy attempt $((attempt + 1))/$attempts after attempt $attempt/$attempts failed with: $reason — waiting ${delay}s" + if [ "$delay" -gt 0 ]; then + sleep "$delay" + fi +done diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 8c1d4c523..5ef5a96c7 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -43,6 +43,9 @@ jobs: # must run the suite that asserts on it. #965 changed release.yml # and skipped the very tests that change broke. - '.github/workflows/**' + # test/unit/scripts/deploy-with-retry.spec.ts runs the deploy + # script, so a change to it must run the suite that asserts on it. + - '.github/scripts/**' commit-lint: name: Lint commits diff --git a/.github/workflows/deploy-website-preview.yml b/.github/workflows/deploy-website-preview.yml index 50c4b097f..5c586bd44 100644 --- a/.github/workflows/deploy-website-preview.yml +++ b/.github/workflows/deploy-website-preview.yml @@ -36,6 +36,9 @@ on: # at all: PR 885 touches none of the website paths.) - ".github/workflows/deploy-website.yml" - "website/site/worker/**" + # The deploy step above runs this script; a change to it changes how the + # preview is deployed, so the previews must be rebuilt by it. + - ".github/scripts/**" concurrency: group: website-preview-${{ github.event.pull_request.number }} @@ -131,11 +134,19 @@ jobs: - name: Deploy the preview worker if: steps.guard.outputs.skip == 'false' && github.event.action != 'closed' working-directory: website/site + # A transient Cloudflare failure must not leave the previous build being + # served while the PR reads as deployed (#970). The shared script retries + # code 10013, an HTTP 5xx and a reset connection — about a minute of + # backoff in total — and fails on the first attempt for anything else, + # including an auth failure. timeout-minutes bounds the retries plus the + # per-attempt timeout the script applies. + timeout-minutes: 12 env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} run: | - npx wrangler deploy \ + bash "$GITHUB_WORKSPACE/.github/scripts/deploy-with-retry.sh" \ + npx wrangler deploy \ --config wrangler.preview.jsonc \ --name nostream-web-pr-${{ github.event.pull_request.number }} diff --git a/.github/workflows/deploy-website.yml b/.github/workflows/deploy-website.yml index 573d12274..21ea84f76 100644 --- a/.github/workflows/deploy-website.yml +++ b/.github/workflows/deploy-website.yml @@ -61,6 +61,10 @@ jobs: - name: Deploy to Cloudflare Workers working-directory: website/site + # The shared script retries only a transient failure (#970) and fails on + # the first attempt for an auth or validation error; timeout-minutes + # bounds the retries plus the per-attempt timeout the script applies. + timeout-minutes: 12 env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} @@ -71,7 +75,9 @@ jobs: # http included, 301s to it, so www/apex cannot serve a second copy # and split search ranking. --var keeps the deployed value from # drifting away from the SITE_URL the build above baked in. - run: pnpm exec wrangler deploy --config wrangler.router.jsonc --var CANONICAL_HOST:"$CANONICAL_HOST" + run: | + bash "$GITHUB_WORKSPACE/.github/scripts/deploy-with-retry.sh" \ + pnpm exec wrangler deploy --config wrangler.router.jsonc --var CANONICAL_HOST:"$CANONICAL_HOST" - name: Purge the site's CDN cache # Same reason as the preview workflow's purge: a request that lands diff --git a/test/unit/scripts/deploy-with-retry.spec.ts b/test/unit/scripts/deploy-with-retry.spec.ts new file mode 100644 index 000000000..8a4cf240e --- /dev/null +++ b/test/unit/scripts/deploy-with-retry.spec.ts @@ -0,0 +1,177 @@ +import { expect } from 'chai' +import { spawnSync } from 'child_process' +import fs from 'fs' +import os from 'os' +import path from 'path' + +const projectRoot = process.cwd() +const scriptPath = path.join(projectRoot, '.github', 'scripts', 'deploy-with-retry.sh') + +/** + * Each entry describes what the stub `wrangler` does on that attempt: print + * `stderr` and exit with `code`, or `sleep` for a number of seconds first. An + * attempt beyond the list succeeds. The stub appends its arguments to a file so + * the test can check what the caller actually invoked. + */ +type Attempt = { stderr?: string; code?: number; sleep?: number } + +const writeStub = (dir: string, attempts: Attempt[]): string => { + const binDir = path.join(dir, 'bin') + fs.mkdirSync(binDir, { recursive: true }) + const cases = attempts + .map((attempt, index) => { + const body = [ + attempt.sleep ? `sleep ${attempt.sleep}` : '', + attempt.stderr ? `printf '%s\\n' ${JSON.stringify(attempt.stderr)} >&2` : '', + // A sleep-only attempt is expected to be killed by the attempt timeout, + // so it never reaches its own exit. + `exit ${attempt.code ?? 0}`, + ] + .filter(Boolean) + .join('; ') + return ` ${index + 1}) ${body};;` + }) + .join('\n') + const stub = `#!/usr/bin/env bash +printf '%s\\n' "$*" >> "$STUB_ARGS" +attempt=$(wc -l < "$STUB_ARGS") +case "$attempt" in +${cases} + *) echo "deployed"; exit 0;; +esac +` + + fs.writeFileSync(path.join(binDir, 'wrangler'), stub, { mode: 0o755 }) + return binDir +} + +type Run = { status: number | null; stdout: string; stderr: string; attempts: number; args: string[] } + +const runScript = (dir: string, attempts: Attempt[], args: string[], env: NodeJS.ProcessEnv = {}): Run => { + const binDir = writeStub(dir, attempts) + const counter = path.join(dir, 'invocations.txt') + fs.writeFileSync(counter, '') + const result = spawnSync('bash', [scriptPath, ...args], { + cwd: projectRoot, + env: { + ...process.env, + ...env, + STUB_ARGS: counter, + PATH: `${binDir}:${process.env.PATH}`, + // The production defaults wait 10 s then 30 s; the tests assert the + // retry happened, not how long it took. + DEPLOY_BACKOFFS: '0 0', + }, + encoding: 'utf-8', + timeout: 30_000, + }) + const invocations = fs.readFileSync(counter, 'utf-8').trim() + + return { + status: result.status, + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + attempts: invocations === '' ? 0 : invocations.split('\n').length, + args: invocations === '' ? [] : invocations.split('\n'), + } +} + +const deploy = (dir: string, attempts: Attempt[], env: NodeJS.ProcessEnv = {}): Run => + runScript( + dir, + attempts, + ['wrangler', 'deploy', '--config', 'wrangler.router.jsonc', '--var', 'CANONICAL_HOST:nostream.dev'], + env, + ) + +describe('deploy-with-retry.sh', () => { + let dir: string + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'nostream-deploy-retry-')) + }) + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('retries a Cloudflare 10013 failure and names the attempt and reason', () => { + const result = deploy(dir, [ + { + stderr: + 'A request to the Cloudflare API (/accounts/x/workers/scripts/y/assets-upload-session) failed.\n An unknown error has occurred. [code: 10013]', + code: 1, + }, + ]) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::wrangler deploy attempt 2\/3.*10013/) + }) + + it('does not retry an authentication failure', () => { + const result = deploy(dir, [ + { stderr: 'Authentication error [code: 10000]\n Invalid API Token (HTTP 401)', code: 1 }, + ]) + + expect(result.status).to.not.equal(0) + expect(result.attempts).to.equal(1) + expect(result.stdout).to.not.include('::warning::') + expect(result.stderr).to.include('::error::') + }) + + it('does not retry a wrangler validation error', () => { + const result = deploy(dir, [ + { stderr: '✘ [ERROR] Missing entry-point: The entry-point file at "dist/_worker.js" was not found.', code: 1 }, + ]) + + expect(result.status).to.not.equal(0) + expect(result.attempts).to.equal(1) + expect(result.stdout).to.not.include('::warning::') + }) + + it('retries an HTTP 5xx from the Cloudflare API', () => { + const result = deploy(dir, [{ stderr: 'A request to the Cloudflare API failed: HTTP 502', code: 1 }]) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::.*HTTP 502/) + }) + + it('retries a connection reset', () => { + const result = deploy(dir, [{ stderr: 'Error: read ECONNRESET: connection reset by peer', code: 1 }]) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::.*connection reset/) + }) + + it('gives up after the last attempt and keeps the last error on stderr', () => { + const failing: Attempt = { stderr: 'An unknown error has occurred. [code: 10013]', code: 1 } + const result = deploy(dir, [failing, failing, failing]) + + expect(result.status).to.not.equal(0) + expect(result.attempts).to.equal(3) + expect(result.stderr).to.include('::error::deploy failed after 3 attempts') + expect(result.stderr).to.include('10013') + // One warning per retry, none for the first attempt. + expect(result.stdout.match(/::warning::/g)).to.have.length(2) + }) + + it('passes the wrangler arguments through unchanged', () => { + const result = deploy(dir, []) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(1) + expect(result.args[0]).to.equal('deploy --config wrangler.router.jsonc --var CANONICAL_HOST:nostream.dev') + }) + + it('retries an attempt that exceeds the per-attempt timeout', function () { + this.timeout(20_000) + const result = deploy(dir, [{ sleep: 30 }], { DEPLOY_ATTEMPT_TIMEOUT: '1' }) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::.*timed out after 1s/) + }) +}) From 502a3558b376098f3dbeaa42746d4b508a2a35dc Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 31 Oct 2026 13:49:36 -0400 Subject: [PATCH 2/4] ci(deploy): degrade without coreutils and prove the argument quoting Two review findings on the new script, both real: - `timeout` is coreutils. Every GitHub runner has it and a stock macOS does not, so the unit suite would have failed on a laptop with exit 127 before wrangler ever ran. The script now prefers `timeout`, falls back to `gtimeout`, and otherwise runs the attempt unbounded behind a `::warning::`. The empty-array expansion is the bash 3.2 form, because that is what macOS ships. - The argument test recorded `"$*"`, which joins the arguments into one string. Unquoted `$@` in the script would still have passed it. The stub now records one field per argument and the test includes a value with spaces. Both new tests are load-bearing: reintroducing either defect fails exactly one of them, verified by editing the script and re-running the suite. --- .github/scripts/deploy-with-retry.sh | 18 ++- test/unit/scripts/deploy-with-retry.spec.ts | 120 +++++++++++++++++--- 2 files changed, 118 insertions(+), 20 deletions(-) diff --git a/.github/scripts/deploy-with-retry.sh b/.github/scripts/deploy-with-retry.sh index aa2fb9fe5..91d9d9168 100755 --- a/.github/scripts/deploy-with-retry.sh +++ b/.github/scripts/deploy-with-retry.sh @@ -46,6 +46,19 @@ if ! [[ "$attempts" =~ ^[1-9][0-9]*$ ]]; then exit 2 fi +# `timeout` is coreutils: every GitHub runner has it, a stock macOS does not. +# Falling back keeps the script usable from a laptop (the unit suite runs it), +# and says so rather than silently dropping the bound. Bash 3.2 ships on macOS, +# so an empty array cannot be expanded with "${timeout_cmd[@]}" under `set -u`. +timeout_cmd=() +if command -v timeout >/dev/null 2>&1; then + timeout_cmd=(timeout --kill-after=10 "$attempt_timeout") +elif command -v gtimeout >/dev/null 2>&1; then + timeout_cmd=(gtimeout --kill-after=10 "$attempt_timeout") +else + echo "::warning::no 'timeout' or 'gtimeout' on PATH — deploy attempts are not bounded (install coreutils to restore the per-attempt limit)" +fi + logs=$(mktemp -d) trap 'rm -rf "$logs"' EXIT @@ -78,8 +91,9 @@ for (( attempt = 1; attempt <= attempts; attempt++ )); do echo "deploy attempt $attempt/$attempts: $*" # The whole attempt is bounded: an upload that stops making progress must not # hold the job until the workflow's own timeout. `--kill-after` covers a - # process that ignores TERM. - timeout --kill-after=10 "$attempt_timeout" "$@" >"$log" 2>&1 || status=$? + # process that ignores TERM. Without coreutils the bound is dropped and the + # warning above says so, rather than turning every deploy into exit 127. + ${timeout_cmd[@]+"${timeout_cmd[@]}"} "$@" >"$log" 2>&1 || status=$? cat "$log" if [ "$status" -eq 0 ]; then diff --git a/test/unit/scripts/deploy-with-retry.spec.ts b/test/unit/scripts/deploy-with-retry.spec.ts index 8a4cf240e..5d8be3245 100644 --- a/test/unit/scripts/deploy-with-retry.spec.ts +++ b/test/unit/scripts/deploy-with-retry.spec.ts @@ -33,7 +33,10 @@ const writeStub = (dir: string, attempts: Attempt[]): string => { }) .join('\n') const stub = `#!/usr/bin/env bash -printf '%s\\n' "$*" >> "$STUB_ARGS" +# One line per attempt, one field per argument: "$*" would join the arguments +# and hide an unquoted "$@" in the script under test. +for arg in "$@"; do printf '%s' "$arg" >> "$STUB_ARGS"; done +printf '\\n' >> "$STUB_ARGS" attempt=$(wc -l < "$STUB_ARGS") case "$attempt" in ${cases} @@ -45,19 +48,49 @@ esac return binDir } -type Run = { status: number | null; stdout: string; stderr: string; attempts: number; args: string[] } +const ARG_SEPARATOR = '' -const runScript = (dir: string, attempts: Attempt[], args: string[], env: NodeJS.ProcessEnv = {}): Run => { +const parseAttempts = (recorded: string): string[][] => { + const trimmed = recorded.trim() + if (trimmed === '') { + return [] + } + return trimmed.split('\n').map((line) => { + const fields = line.split(ARG_SEPARATOR) + // Every argument is followed by the separator, so the split leaves a + // trailing empty field. + return fields[fields.length - 1] === '' ? fields.slice(0, -1) : fields + }) +} + +type Run = { + status: number | null + stdout: string + stderr: string + attempts: number + invocations: string[][] + /** Arguments of the first attempt, the one every test's expectation is about. */ + args: string[] +} + +type RunOptions = { env?: NodeJS.ProcessEnv; pathPrefixes?: string[]; systemPath?: boolean } + +const runScript = (dir: string, attempts: Attempt[], args: string[], options: RunOptions = {}): Run => { const binDir = writeStub(dir, attempts) const counter = path.join(dir, 'invocations.txt') fs.writeFileSync(counter, '') + const searchPath = [ + ...(options.pathPrefixes ?? []), + binDir, + ...(options.systemPath === false ? [] : [process.env.PATH]), + ].join(':') const result = spawnSync('bash', [scriptPath, ...args], { cwd: projectRoot, env: { ...process.env, - ...env, + ...options.env, STUB_ARGS: counter, - PATH: `${binDir}:${process.env.PATH}`, + PATH: searchPath, // The production defaults wait 10 s then 30 s; the tests assert the // retry happened, not how long it took. DEPLOY_BACKOFFS: '0 0', @@ -65,24 +98,40 @@ const runScript = (dir: string, attempts: Attempt[], args: string[], env: NodeJS encoding: 'utf-8', timeout: 30_000, }) - const invocations = fs.readFileSync(counter, 'utf-8').trim() + const invocations = parseAttempts(fs.readFileSync(counter, 'utf-8')) return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '', - attempts: invocations === '' ? 0 : invocations.split('\n').length, - args: invocations === '' ? [] : invocations.split('\n'), + attempts: invocations.length, + invocations, + args: invocations[0] ?? [], } } -const deploy = (dir: string, attempts: Attempt[], env: NodeJS.ProcessEnv = {}): Run => - runScript( - dir, - attempts, - ['wrangler', 'deploy', '--config', 'wrangler.router.jsonc', '--var', 'CANONICAL_HOST:nostream.dev'], - env, - ) +const WRANGLER_ARGS = [ + 'wrangler', + 'deploy', + '--config', + 'wrangler.router.jsonc', + '--var', + 'CANONICAL_HOST:nostream.dev', +] + +const deploy = (dir: string, attempts: Attempt[], options: RunOptions = {}): Run => + runScript(dir, attempts, WRANGLER_ARGS, options) + +/** Resolve a command the way a shell would, for building a restricted PATH. */ +const resolveOnPath = (command: string): string | undefined => { + for (const entry of (process.env.PATH ?? '').split(':')) { + const candidate = path.join(entry, command) + if (fs.existsSync(candidate)) { + return candidate + } + } + return undefined +} describe('deploy-with-retry.sh', () => { let dir: string @@ -158,17 +207,52 @@ describe('deploy-with-retry.sh', () => { expect(result.stdout.match(/::warning::/g)).to.have.length(2) }) - it('passes the wrangler arguments through unchanged', () => { + it('passes the wrangler arguments through unchanged, one argument at a time', () => { const result = deploy(dir, []) expect(result.status, result.stderr).to.equal(0) expect(result.attempts).to.equal(1) - expect(result.args[0]).to.equal('deploy --config wrangler.router.jsonc --var CANONICAL_HOST:nostream.dev') + expect(result.args).to.deep.equal(['deploy', '--config', 'wrangler.router.jsonc', '--var', 'CANONICAL_HOST:nostream.dev']) + }) + + it('keeps an argument that contains spaces as one argument', () => { + const args = ['wrangler', 'deploy', '--var', 'SITE_TITLE:Oh My Relay'] + const result = runScript(dir, [], args) + + expect(result.status, result.stderr).to.equal(0) + // Unquoted `$@` in the script would split this into four arguments. + expect(result.args).to.deep.equal(['deploy', '--var', 'SITE_TITLE:Oh My Relay']) + }) + + it('still deploys when no timeout command is installed (stock macOS)', () => { + // A PATH with the tools the script needs and, deliberately, neither + // `timeout` nor `gtimeout`. The script must degrade to an unbounded + // attempt and say so, not exit 127 before the deploy runs. + const restricted = path.join(dir, 'restricted-bin') + fs.mkdirSync(restricted, { recursive: true }) + for (const tool of ['bash', 'cat', 'grep', 'head', 'mktemp', 'rm', 'sleep', 'tail', 'wc']) { + const resolved = resolveOnPath(tool) + if (!resolved) { + throw new Error(`${tool} not found on PATH; cannot build the restricted environment`) + } + fs.symlinkSync(resolved, path.join(restricted, tool)) + } + + const result = deploy(dir, [], { pathPrefixes: [restricted], systemPath: false }) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(1) + expect(result.stdout).to.match(/::warning::no 'timeout' or 'gtimeout' on PATH/) }) it('retries an attempt that exceeds the per-attempt timeout', function () { + if (!resolveOnPath('timeout') && !resolveOnPath('gtimeout')) { + // Only reached on a host without coreutils, where the script has no bound + // to exceed in the first place. + this.skip() + } this.timeout(20_000) - const result = deploy(dir, [{ sleep: 30 }], { DEPLOY_ATTEMPT_TIMEOUT: '1' }) + const result = deploy(dir, [{ sleep: 30 }], { env: { DEPLOY_ATTEMPT_TIMEOUT: '1' } }) expect(result.status, result.stderr).to.equal(0) expect(result.attempts).to.equal(2) From 3bbf8c313c42cc4e17385c1e3c072624d7e15969 Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 31 Oct 2026 13:59:22 -0400 Subject: [PATCH 3/4] test(deploy): normalise the stub's attempt counter for BSD wc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BSD wc pads its count with leading spaces, so `case "$attempt"` matched no branch on macOS and the stub always answered success — which made the retry tests fail on a laptop and pass in CI. The arithmetic expansion strips it; demonstrated with a padded count, where the plain string misses "3" and the normalised one matches. --- test/unit/scripts/deploy-with-retry.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/scripts/deploy-with-retry.spec.ts b/test/unit/scripts/deploy-with-retry.spec.ts index 5d8be3245..db85febc0 100644 --- a/test/unit/scripts/deploy-with-retry.spec.ts +++ b/test/unit/scripts/deploy-with-retry.spec.ts @@ -37,7 +37,9 @@ const writeStub = (dir: string, attempts: Attempt[]): string => { # and hide an unquoted "$@" in the script under test. for arg in "$@"; do printf '%s' "$arg" >> "$STUB_ARGS"; done printf '\\n' >> "$STUB_ARGS" -attempt=$(wc -l < "$STUB_ARGS") +# BSD wc -l pads its count with leading spaces, which would make every case +# branch miss; the arithmetic expansion normalises it to a plain integer. +attempt=$(( $(wc -l < "$STUB_ARGS") )) case "$attempt" in ${cases} *) echo "deployed"; exit 0;; From 770b9d839ea399e6acea37ed49a2450906703b8a Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 31 Oct 2026 14:06:35 -0400 Subject: [PATCH 4/4] test(deploy): assert on retry warnings, not on any warning On a host without coreutils the script warns about the missing timeout on every run, so a blanket 'no ::warning::' assertion failed there while passing in CI, and the exhausted-retry count saw one extra line. The assertions now name the retry warning. Proved by running the suite with a PATH that has no timeout or gtimeout: 9 passing with the timeout case skipped, where before the same environment failed three tests. --- test/unit/scripts/deploy-with-retry.spec.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/unit/scripts/deploy-with-retry.spec.ts b/test/unit/scripts/deploy-with-retry.spec.ts index db85febc0..a106e4741 100644 --- a/test/unit/scripts/deploy-with-retry.spec.ts +++ b/test/unit/scripts/deploy-with-retry.spec.ts @@ -135,6 +135,11 @@ const resolveOnPath = (command: string): string | undefined => { return undefined } +// The script also warns when coreutils is absent (stock macOS), and that line is +// present on every run there. Assertions about retries must name the retry line, +// or they would fail on a laptop and pass in CI. +const RETRY_WARNING = /::warning::wrangler deploy attempt/g + describe('deploy-with-retry.sh', () => { let dir: string @@ -167,7 +172,7 @@ describe('deploy-with-retry.sh', () => { expect(result.status).to.not.equal(0) expect(result.attempts).to.equal(1) - expect(result.stdout).to.not.include('::warning::') + expect(result.stdout).to.not.match(RETRY_WARNING) expect(result.stderr).to.include('::error::') }) @@ -178,7 +183,7 @@ describe('deploy-with-retry.sh', () => { expect(result.status).to.not.equal(0) expect(result.attempts).to.equal(1) - expect(result.stdout).to.not.include('::warning::') + expect(result.stdout).to.not.match(RETRY_WARNING) }) it('retries an HTTP 5xx from the Cloudflare API', () => { @@ -206,7 +211,7 @@ describe('deploy-with-retry.sh', () => { expect(result.stderr).to.include('::error::deploy failed after 3 attempts') expect(result.stderr).to.include('10013') // One warning per retry, none for the first attempt. - expect(result.stdout.match(/::warning::/g)).to.have.length(2) + expect(result.stdout.match(RETRY_WARNING)).to.have.length(2) }) it('passes the wrangler arguments through unchanged, one argument at a time', () => {