From bee7520b4e01a384c6b2e73f64ab19f837b4d340 Mon Sep 17 00:00:00 2001 From: Tonio Date: Thu, 27 Aug 2026 09:14:46 -0700 Subject: [PATCH] fix(smoke): bound each bootstrap request and run the tests in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from Greptile on #12316. The retry loop only checks its deadline between attempts, so an unbounded curl that connects and then stalls never returns to be checked. That would have run the job to its 45-minute timeout printing nothing — strictly worse than the single-attempt failure this change set out to fix. Cap each request with --max-time and --connect-timeout. The repository enumerates its script-level node:test files one by one in pr.yml, and the new suite was not among them, so six regression cases sat in the tree without running. Add it. Co-Authored-By: Claude Fable 5 --- .github/workflows/pr.yml | 3 +++ .../__tests__/docker-onboard-smoke-bootstrap.test.mjs | 11 +++++++++++ scripts/docker-onboard-smoke.sh | 8 ++++++++ 3 files changed, 22 insertions(+) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index e356bb8a096..6be7eb3bc2d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -72,6 +72,9 @@ jobs: - name: Test standalone package build concurrency run: node --test ./scripts/__tests__/build-standalone-concurrency.test.mjs + - name: Test release smoke admin bootstrap + run: node --test ./scripts/__tests__/docker-onboard-smoke-bootstrap.test.mjs + - name: Validate release package manifest run: node ./scripts/release-package-map.mjs check diff --git a/scripts/__tests__/docker-onboard-smoke-bootstrap.test.mjs b/scripts/__tests__/docker-onboard-smoke-bootstrap.test.mjs index b0bf2e97577..beb410035d0 100644 --- a/scripts/__tests__/docker-onboard-smoke-bootstrap.test.mjs +++ b/scripts/__tests__/docker-onboard-smoke-bootstrap.test.mjs @@ -140,6 +140,17 @@ test("gives up at the deadline rather than hanging", () => { assert.ok(r.elapsedMs < 30000, `overran the deadline: ${r.elapsedMs}ms`); }); +test("bounds every request so the deadline can actually be reached", () => { + // The retry loop only checks its deadline between attempts. An unbounded curl + // that connects and then stalls never returns to be checked, so the job would + // run to its own 45-minute timeout printing nothing — strictly worse than the + // single-attempt failure this change replaced. Asserted on the script text + // because the harness above stubs the helper out. + const helper = extractFunction("post_json_with_cookies"); + assert.match(helper, /--max-time/, "post_json_with_cookies must cap request duration"); + assert.match(helper, /--connect-timeout/, "post_json_with_cookies must cap connect time"); +}); + test("reports the HTTP status it saw, not just an empty body", () => { // The reason this flake went a week without a diagnosis: the failure printed // the response body, and a sign-up that never connected has none. The status diff --git a/scripts/docker-onboard-smoke.sh b/scripts/docker-onboard-smoke.sh index dcb5573169f..10dbd61ae76 100755 --- a/scripts/docker-onboard-smoke.sh +++ b/scripts/docker-onboard-smoke.sh @@ -135,11 +135,19 @@ generate_bootstrap_invite_url() { printf '%s\n' "$invite_url" } +# Seconds any single bootstrap request may take. Without this a connection that +# is accepted and then stalls hangs curl forever: the retry loop below only +# checks its deadline between attempts, so an unbounded call never returns to be +# checked and the job runs to its own timeout printing nothing. +SMOKE_REQUEST_TIMEOUT_SECONDS="${SMOKE_REQUEST_TIMEOUT_SECONDS:-20}" + post_json_with_cookies() { local url="$1" local body="$2" local output_file="$3" curl -sS \ + --connect-timeout 5 \ + --max-time "$SMOKE_REQUEST_TIMEOUT_SECONDS" \ -o "$output_file" \ -w "%{http_code}" \ -c "$COOKIE_JAR" \