From 8c29599899c101eeee5d31f799362ed603161808 Mon Sep 17 00:00:00 2001 From: owine Date: Tue, 25 Aug 2026 09:50:37 -0500 Subject: [PATCH 1/2] fix(deploy): add timeout to whole-tree rollback up; correct its comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-existing issues in `Redeploy stacks at previous SHA`, left untouched by the scoped-rollback work because that change deliberately did not modify this step's body. 1. Missing `timeout`. Every other `docker compose up` in this workflow is wrapped in `timeout "$SERVICE_STARTUP_TIMEOUT"`; this one was not. `--wait` blocks indefinitely on a container stuck in `starting` — exactly what a bad image produces — until the job's timeout-minutes cancels the whole job, stranding every stack after it with no further fallback. Verified with exec-able stubs: a 30s hang now aborts at the 2s budget, emits a warning, and the loop continues. The failure message also now carries the manual recovery command, since a failed rollback up leaves the job green. 2. Inaccurate comment. It claimed this step reverts "only the stacks this deploy actually touched" and that skipping untouched stacks "avoids needlessly recreating the whole fleet". Both are false: detect-stack-changes.sh:401 computes existing_stacks as (all discovered stacks - new stacks), so this loop covers the whole fleet on every run. The comment now says so, and records that the fleet-wide scope is load-bearing — it is what pulls a stack pinned by a prior per-stack rollback back into line with the tree. --- .github/workflows/deploy.yml | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index c24469b..3549e35 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -982,14 +982,22 @@ jobs: - name: Redeploy stacks at previous SHA if: steps.plan.outputs.mode == 'whole-tree' - # Only the stacks this deploy actually touched need reverting: - # - existing: roll their config back to the previous SHA + # Which stacks this covers: + # - existing: NOT the changed set. detect-stack-changes.sh computes + # existing_stacks as (all discovered stacks - new stacks), + # so this is effectively the whole fleet on every run. # - removed: deleted this deploy (torn down already), so they # reappear after the reset and must be brought back up - # `new` stacks were torn down above and no longer exist post-reset, and - # untouched stacks are byte-identical before/after the reset — skipping - # them avoids needlessly recreating the whole fleet on a single-stack - # failure. No `--pull always`/`--build`: roll back onto the + # `new` stacks were torn down above and no longer exist post-reset. + # + # The fleet-wide scope is load-bearing, not an oversight: it is what + # brings a stack pinned by a prior per-stack rollback back into line + # with the tree. Narrowing existing_stacks to the real change set would + # let such a stack drift — its containers on the old image while the + # tree claims the new one — until it next changed. See the design doc, + # "Scoped rollback and image quarantine", section A4. + # + # No `--pull always`/`--build`: roll back onto the # locally-tagged previous images (kept on disk by the docker-prune # policy) so rollback is fast and doesn't depend on a registry being # reachable mid-incident; compose still builds on demand if an image is @@ -1006,9 +1014,15 @@ jobs: stack_dir="$LIVE_REPO_PATH/$stack" [[ -f "$stack_dir/compose.yaml" || -f "$stack_dir/compose.yml" ]] || continue cd "$stack_dir" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- \ + # `timeout` matches every other `up` in this workflow. Without it a + # container stuck in `starting` makes `--wait` block until the job's + # timeout-minutes cancels the whole job, stranding every stack after + # this one with no further fallback — the exact state a bad image + # tends to produce. + timeout "$SERVICE_STARTUP_TIMEOUT" \ + op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- \ docker compose up -d --quiet-pull --wait --remove-orphans \ - || echo "::warning::rollback up failed for $stack" + || echo "::warning::rollback up failed for $stack; recover manually with: cd $LIVE_REPO_PATH/$stack && op run --no-masking --env-file=$LIVE_REPO_PATH/compose.env -- docker compose up -d --wait" done notify: From 7e3b0d264bb391f6540571f88271a770923d596f Mon Sep 17 00:00:00 2001 From: owine Date: Tue, 25 Aug 2026 09:54:40 -0500 Subject: [PATCH 2/2] fix(deploy): quote paths in the whole-tree rollback recovery hint Same fix as the per-stack hint on the base branch, applied to the whole-tree step's warning. An unquoted $LIVE_REPO_PATH produced a copy-paste command that word-splits on a deploy path containing whitespace, so pasting it fails instead of recovering the stack. Reported by Sourcery on this PR. --- .github/workflows/deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 3549e35..6f4128d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1022,7 +1022,7 @@ jobs: timeout "$SERVICE_STARTUP_TIMEOUT" \ op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- \ docker compose up -d --quiet-pull --wait --remove-orphans \ - || echo "::warning::rollback up failed for $stack; recover manually with: cd $LIVE_REPO_PATH/$stack && op run --no-masking --env-file=$LIVE_REPO_PATH/compose.env -- docker compose up -d --wait" + || echo "::warning::rollback up failed for $stack; recover manually with: cd \"$LIVE_REPO_PATH/$stack\" && op run --no-masking --env-file=\"$LIVE_REPO_PATH/compose.env\" -- docker compose up -d --wait" done notify: