RCA — queue-docker-smoke detect-changes fails: CI_COMMIT_BEFORE unset under set -eu

Incident ID: 2026-08-10-queue-docker-smoke-ci-commit-before-unset Date: 2026-08-10 Severity: SEV-3 (non-blocking degradation — not a required merge check; silently broken since introduction) Duration: N/A (latent defect, not a live outage — found during unrelated PR diagnosis, fixed same day as grooming) Blast radius: ci/woodpecker/pr/queue-docker-smoke on PR events targeting release/main where WP leaves CI_COMMIT_BEFORE unset (confirmed on cherry-pick/rebuild-style hotfix branches, e.g. #4442). No production traffic affected (CI-only, non-required check). Author: sre-agent

Summary

.woodpecker/queue-docker-smoke.yaml's detect-changes step referenced $CI_COMMIT_BEFORE under set -eu with no default expansion. The step already had fallback logic for the documented zero-SHA force-push case, but WP v3.16 can leave CI_COMMIT_BEFORE entirely unset (not zero-SHA) on some PR events — observed on hotfix/getraxx-restore-blr-disclaimers-4441 (PR #4442, base main) — and set -u aborts on an unset-variable reference before the existing fallback if can even evaluate it. The step failed deterministically (reproduced on two consecutive runs, 5096 and 5102) with /bin/sh: 58: CI_COMMIT_BEFORE: parameter not set. Not currently a required merge check, so it did not block #4442 or any other PR, but it was silently broken and would false-block if ever promoted to required. Fixed by guarding the reference with ${CI_COMMIT_BEFORE:-}, routing the unset case into the same existing HEAD~1 fallback used for the zero-SHA case.

Timeline (all times UTC)

• 2026-08-09 ~19:38 — PR #4442 (hotfix, base main) merged. queue-docker-smoke pipelines 5096/5102 both failed at detect-changes with parameter not set — noticed while diagnosing an unrelated required-check failure on #4442, not itself blocking (confirmed non-required via gh api repos/raxx-app/TradeMasterAPI/branches/main/protection).
• 2026-08-09 — Issue #4452 filed by sre-agent with root cause and proposed fix.
• 2026-08-10 — Grooming pass confirms line-for-line match against origin/develop@516e720db7 (set -eu line 61, unguarded BEFORE="$CI_COMMIT_BEFORE" line 71) and tightens acceptance criteria: guard the reference, preserve the existing HEAD~1 fallback unchanged, verify the zero-SHA regression case still passes. Marked ready-for-dev.
• 2026-08-10 — sre-agent branches fresh off origin/develop (516e720db7, matching the grooming-verified SHA).
• 2026-08-10 — Local reproduction: sh -eu with CI_COMMIT_BEFORE unset, unguarded BEFORE="$CI_COMMIT_BEFORE" → unbound variable abort (same failure class as WP's parameter not set).
• 2026-08-10 — Fix applied: BEFORE="$CI_COMMIT_BEFORE" → BEFORE="${CI_COMMIT_BEFORE:-}". Local verification: all three cases (unset, zero-SHA, normally-populated) exit 0 and reach the correct branch.
• 2026-08-10 — Live scoped verification: opened throwaway draft PR #4468 (branch cut from origin/release, cherry-picking only the fix commit) to trigger queue-docker-smoke (its when: only fires for PR events targeting release/main). WP pipeline 5171, workflow queue-docker-smoke (id 9862): detect-changes step (id 29848, pid 32) — state: success, exit_code: 0, 34s runtime — confirmed via GET /api/repos/1/pipelines/5171 (structured JSON, not the SPA-fallback log endpoint, which 404s for this WP install). ecr-auth (id 29849) also succeeded; docker-build-smoke (id 29850) failed separately (exit_code: 2, 2s runtime — an unrelated pre-existing issue in the throwaway PR's build context, not investigated further, out of scope for #4452). Draft PR closed (not merged) immediately after capturing the run.
• 2026-08-10 — Runbook correction: docs/ops/runbooks/ci-woodpecker.md Failure mode E-5 previously over-broadly listed ${VAR:-default} as a second global-compile-breakage pattern (alongside the confirmed ${{ cause). Corrected based on multiple already-live, already-passing precedents using unescaped ${VAR:-default} on WP built-ins (.woodpecker/ci-pr.yaml, merged 2026-07-10; this runbook's own §Failure mode E-6 fix, #4347, merged 2026-07-25) plus this incident's own live verification.
• 2026-08-10 — Fix PR opened against develop. Resolved.

Impact

• Users affected: none (internal CI/CD only, non-required check).
• User-visible symptoms: none.
• Data integrity: ok.
• Revenue / billing: ok.
• Engineering impact: queue-docker-smoke's change-detection silently failed on PR branches with unset CI_COMMIT_BEFORE (cherry-pick/rebuild-style branches at minimum) since the step was authored. No false-block occurred because the check isn't required, but a future policy change adding it to required checks would have false-blocked unrelated PRs with no clear signal why.

What went well

• The step's own documented "safe default" design (any git-diff failure defaults to SKIP=false, i.e. the build runs rather than silently skipping) meant this bug produced a visible, red step rather than a silent false-skip — it announced itself as broken instead of quietly not doing its job.
• Grooming caught and tightened the fix scope before dev work started: pinned the fix to reusing the existing HEAD~1 fallback rather than introducing new merge-base logic, keeping the change to one line.
• A directly analogous fix (${CI_COMMIT_BEFORE:-} guard) was already live and passing as a required check in .woodpecker/ci-pr.yaml, giving a proven syntax precedent to mirror rather than inventing new WP-YAML syntax.

What didn't go well

• The runbook's own Failure mode E-5 entry (written 2026-07-04) gave overly broad guidance — recommending the $$-escaped form for all ${VAR:-default} usage — that, if followed literally, would have led to unnecessarily escaping a WP built-in variable reference. The entry's root cause was under-verified: the 2026-07-04 incident's own RCA already noted "Hypothesis 1 [${VAR:-}] was plausible but incomplete," yet the runbook fix guidance treated it as confirmed.
• No .woodpecker/*.yaml lint gate exists to catch unguarded WP built-in variable references under set -eu before merge — this class of bug (a variable that WP sometimes leaves unset, referenced without a default) is not mechanically detectable by the existing ${{ / colon-space greps.

Root cause analysis

• Contributing factor 1 — the step's fallback logic anticipated the documented failure mode (zero-SHA force-push) but not the observed one (unset entirely): git diff --name-only "$BEFORE" HEAD under set -eu requires BEFORE to be a defined variable before the emptiness/zero-SHA if check can run at all. set -u treats the unset-variable reference itself as an error, short-circuiting before the fallback logic — the fallback was correctly written for the case it anticipated but the system did not account for WP populating CI_COMMIT_BEFORE inconsistently across PR-event subtypes.
• Contributing factor 2 — no test coverage exercising detect-changes with CI_COMMIT_BEFORE unset: the step was authored and reviewed against the documented zero-SHA case only; nothing in CI or local testing exercised the "entirely absent" case, so the gap wasn't caught until a real cherry-pick hotfix branch hit it in production CI.
• Contributing factor 3 — stale/overbroad runbook guidance for WP variable escaping: the corrected Failure mode E-5 entry (this incident) shows the existing documentation would have pushed a fix author toward an unnecessary $$-escape, adding friction and second-guessing to what should have been a one-line, well-precedented fix.

Detection

• What alerted us: sre-agent noticed the failure while diagnosing an unrelated required-check failure on PR #4442 — not a targeted alert.
• How long between cause and detection: latent since the step was authored; detection was incidental (piggybacked on unrelated diagnosis), not via a monitor.
• How to detect faster next time: none of queue-docker-smoke's steps are currently required checks, so there is no merge-time signal at all for this class of failure. A lightweight periodic check (e.g., a weekly sweep of non-required-check failure rates across .woodpecker/*.yaml pipelines) would surface silently-broken-but-non-blocking steps like this one before they accumulate or get promoted to required without being fixed first.

Resolution

• What was changed: .woodpecker/queue-docker-smoke.yaml line 71, BEFORE="$CI_COMMIT_BEFORE" → BEFORE="${CI_COMMIT_BEFORE:-}". No other logic changed — the existing [ -z "$BEFORE" ] / zero-SHA check on the next line already routes both the unset and zero-SHA cases into the same HEAD~1 fallback. Comment updated to document the unset case alongside the existing zero-SHA documentation.
• docs/ops/runbooks/ci-woodpecker.md Failure mode E-5 corrected to remove the overbroad ${VAR:-default} guidance for WP built-ins, with references to the confirmed-working precedents and this incident's live verification.
• Validation:
• Local (sh -eu): unguarded reference with CI_COMMIT_BEFORE unset reproduces unbound variable (same failure class as WP's parameter not set). Guarded version: unset case falls through to HEAD~1 (exit 0); zero-SHA case falls through to HEAD~1 unchanged (exit 0, regression check); normally-populated case uses the value directly (exit 0).
• Live WP v3.16 (scoped): throwaway draft PR #4468 (branch off release, fix commit only) triggered queue-docker-smoke's detect-changes step (pipeline 5171, step id 29848): state: success, exit_code: 0, per GET /api/repos/1/pipelines/5171. Draft PR closed without merging.
• python3 -c "import yaml; yaml.safe_load(...)" confirms the file parses cleanly; raw-grep of touched lines for ': ' found no colon-space compile hazards.

Action items

References

• Runbook: docs/ops/runbooks/ci-woodpecker.md (Failure mode E-4, E-5 — corrected by this incident)
• Issue: #4452
• Related PR (where the bug was first observed, unrelated to its content): #4442
• Related incident: docs/incidents/2026-07-04-waf-webhook-block-batch1-parse-error.md
• Precedent fix pattern: .woodpecker/ci-pr.yaml (b1-migration-path-lint, flag-promotion-check, merged 2026-07-10)