From 70e675afee220c1f870544e0e9e5b53f81886825 Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Tue, 8 Sep 2026 23:31:48 +0200 Subject: [PATCH] fix(smoke): keep prose out of the single-quoted exec_test body The agent-browser execution guard added on 2026-09-07 carried its explanation INSIDE the single-quoted script body, and the explanation contained an apostrophe ("the fleet\'s only recurring amd64 runtime proof"). Inside '...' bash treats a backslash as literal, so \' does not escape the quote -- it CLOSES the string. The body truncated at that point and the remaining lines were parsed by the calling shell. Consequences, both measured rather than inferred: - exec_test received 12 arguments instead of 2 (verified two-sided: the fixed tree yields argc=2, HEAD yields argc=12). - the leaked `v=$(agent-browser --version)` ran on the CI RUNNER instead of inside the image. The runner has no agent-browser, so smoke and smoke-studio both failed with "line 770: command not found" after build-base had already spent ~46 minutes. Every downstream job was skipped. - the truncated body still passed inside the container and printed its green tick first, so the log shows a PASS immediately followed by the failure -- the tick was real, it just no longer covered the assertion. The prose now sits above the exec_test call, where an apostrophe cannot terminate anything, and a comment at that spot records why it must stay there. Not a new failure class: shellcheck flagged it as SC2289 at severity error the same day, so the lint job has been red since run 186 (2026-09-07 21:21) and was not read. The gate did its job; nobody looked. --- CHANGELOG.md | 23 +++++++++++++++++++++++ scripts/smoke-test.sh | 34 +++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c60f716..e11fc15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,29 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). ## v1.8.14 — 2026-09-08 +> **First release attempt failed; fixed in this same entry.** The `smoke` and +> `smoke-studio` jobs both failed at `scripts/smoke-test.sh:770` with +> `agent-browser: command not found`, after `build-base` had already succeeded +> (~46 min spent). Root cause was in the agent-browser execution guard added the +> day before: the explanatory comment inside the **single-quoted** `exec_test` +> body contained an apostrophe (`the fleet\'s`). Inside `'...'` bash treats a +> backslash literally, so `\'` does not escape — it **closes the string**. The +> body silently truncated (measured: `exec_test` received **12** arguments +> instead of 2), and the remaining lines, including the `agent-browser --version` +> assertion, were parsed by the **runner's** shell instead of executing inside +> the image — and the runner has no agent-browser. The prose now lives above the +> call, where an apostrophe is harmless. +> +> **The lint job had already caught this, and it went unread for 24 hours.** +> `shellcheck` flagged it as `SC2289` at severity *error*, so the `actionlint` +> job went red at run 186 on 2026-09-07 21:21 — the exact push that introduced +> the guard — and stayed red for runs 187 and 188. `lint.yml` deliberately +> excludes tag pushes (documented: the tagged tree was already linted on main, +> and a tag-ref lint run would sort above the publish run), which is sound; the +> broken assumption was different, namely that a tree whose lint FAILED would not +> then be released. `docker-publish.yml` has no dependency on lint, so it built +> for 50 minutes on a tree known to be defective. + **A test that was quietly checking nothing, and a version number that was wrong.** Both found by delegating a read-only audit of this repo to a headless worker (`pi-toolkit` `bin/pi-task`) and then spot-checking its pointers from the diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 8c6ef06..5b879c5 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -751,22 +751,30 @@ exec_test "pi-atelier registered from /opt, not npm: (volume-shadowing guard)" \ # shadows it. The check that actually bites lives in # recreate-sanity-check.sh, which runs where the volume is real — that is # where a 7-week-old 0.27.0 was caught shadowing 0.35.2 on 2026-09-06. +# EXECUTION is ASSERTED here, not printed. Until 2026-09-07 the version was +# captured inside an echo with 2>/dev/null, so a binary that could not run at all +# still PASSED and simply printed version=[] -- the same failure class as the bare +# `node --version` two hundred lines up: a value displayed rather than compared. +# +# Why this exit code matters more than most: smoke runs `platforms: linux/amd64` +# on an x86 runner, i.e. NATIVE amd64, so this is the fleet's only recurring +# amd64 runtime proof for the linux-x64 ELF. No devbox can supply one -- every +# machine in the pi fleet is an Apple Silicon Mac (mbp-m1-2020; tor-ms22 = Mac +# Studio Mac13,1 M1 Max, verified 2026-08-17 by system_profiler; emb-7kj4vr4g = +# Apple Silicon, 4 routes 2026-09-07). Asking a device for that proof is asking +# for the impossible; CI already had it and was discarding it. +# +# KEEP PROSE OUT OF THE QUOTED BODY BELOW. On 2026-09-07 this explanation lived +# INSIDE the single-quoted argument and contained an apostrophe ("the fleet's"). +# Inside '...' bash treats a backslash literally, so \' does not escape -- it +# CLOSES the string. The body silently truncated, the remaining lines were parsed +# by the RUNNER's shell instead of the container's, and `agent-browser --version` +# ran on a host that has no agent-browser: "line 770: command not found", release +# v1.8.14's smoke job failed after the base had already built. shellcheck caught +# it as SC2289 the same day and the red lint job went unread for 24h. exec_test "agent-browser resolves under /usr (volume-shadowing guard, build-time half)" ' p=$(command -v agent-browser) || { echo "agent-browser not on PATH" >&2; exit 1; } r=$(readlink -f "$p") - # EXECUTION is now ASSERTED, not printed. Until 2026-09-07 the version was - # captured inside an echo with 2>/dev/null, so a binary that could not run at - # all still PASSED this test and simply printed version=[]. Same failure class - # as the bare `node --version` two hundred lines up: a value displayed rather - # than compared. - # Why this particular exit code matters more than most: smoke runs - # `platforms: linux/amd64` on an x86 runner, i.e. NATIVE amd64, so this line is - # the fleet\'s only recurring amd64 runtime proof for the linux-x64 ELF. No - # devbox can ever supply one -- every machine in the pi fleet is an Apple - # Silicon Mac (mbp-m1-2020; tor-ms22 = Mac Studio Mac13,1 M1 Max, verified - # 2026-08-17 by system_profiler; emb-7kj4vr4g = Apple Silicon, 4 routes - # 2026-09-07). Asking a device for that proof is asking for the impossible; - # CI already had it and was discarding it. v=$(agent-browser --version) || { echo "agent-browser did not EXECUTE" >&2; exit 1; } test -n "$v" || { echo "agent-browser --version produced no output" >&2; exit 1; } echo "resolved=[$r] version=[$(printf %s "$v" | head -n1)]" >&2