diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 8add820..6f4128d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -78,6 +78,8 @@ jobs: disabled_stacks: ${{ steps.discover-stacks.outputs.disabled_stacks }} has_disabled_stacks: ${{ steps.discover-stacks.outputs.has_disabled_stacks }} critical_stacks: ${{ steps.detect-critical.outputs.critical_stacks }} + rollback_scope: ${{ steps.classify-scope.outputs.rollback_scope }} + changed_stacks: ${{ steps.classify-scope.outputs.changed_stacks }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -123,6 +125,10 @@ jobs: uses: tj-actions/changed-files@9426d40962ed5378910ee2e21d5f8c6fcbf2dd96 # v47.0.6 with: json: true + # escape_json defaults to true, which backslash-escapes every quote + # and makes the output unparseable by jq. Every consumer here pipes + # these outputs into jq, so the escaped form is useless to us. + escape_json: false sha: ${{ inputs.target-ref }} base_sha: ${{ steps.previous-sha.outputs.previous_sha }} @@ -182,6 +188,55 @@ jobs: --removed-files "$REMOVED_FILES" \ --added-files "$ADDED_FILES" + - name: Classify rollback scope + id: classify-scope + # Runs after detect-changes because it needs removed_stacks: a removed + # stack's directory is gone from the target tree, so discover-stacks + # never sees it. Without it in the known-dirs set, every stack deletion + # would look like a root-level change and force whole-tree rollback. + env: + CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files != '' && steps.changed-files.outputs.all_changed_files || '[]' }} + ACTIVE_STACKS: ${{ steps.discover-stacks.outputs.stacks }} + DISABLED_STACKS: ${{ steps.discover-stacks.outputs.disabled_stacks }} + REMOVED_STACKS: ${{ steps.detect-changes.outputs.removed_stacks || '[]' }} + run: | + set -euo pipefail + # `|| echo '[]'` rather than letting jq abort the job: a malformed + # upstream list is exactly the case the classifier is built to + # survive. `[]` hits its `dirs_count` guard and degrades to + # whole-tree โ€” the same disposition the script would pick itself. + # Aborting `prepare` instead would skip the deploy entirely. + stack_dirs=$(jq -cn \ + --argjson a "$ACTIVE_STACKS" \ + --argjson b "$DISABLED_STACKS" \ + --argjson c "$REMOVED_STACKS" \ + '($a + $b + $c) | unique' 2>/dev/null || echo '[]') + + # The exact set of stack directories this deploy touched: the first + # path segment of every changed file that names a known stack dir. + # + # This is NOT the same thing as `existing_stacks`. detect-stack-changes.sh + # defines existing_stacks as (all discovered stacks - new stacks), so it + # names the whole fleet on every run and cannot tell a touched stack from + # an untouched one. `rollback` needs the touched set to check that a + # culprit is a stack whose directory actually differs between + # PREVIOUS_SHA and TARGET_REF โ€” reverting one that does not is a no-op. + # + # Degrades to `[]` on any malformed input, which makes every non-empty + # culprit list fail the membership check in `Resolve rollback plan` and + # forces whole-tree. Uncertainty resolves to whole-tree, never per-stack. + changed_stacks=$(jq -cn \ + --argjson f "$CHANGED_FILES" \ + --argjson d "$stack_dirs" \ + '[ $f[] | split("/")[0] | select(. as $s | $d | index($s) != null) ] | unique' \ + 2>/dev/null || echo '[]') + echo "changed_stacks=$changed_stacks" >> "$GITHUB_OUTPUT" + echo "๐Ÿ“ฆ Stack dirs touched by this deploy: $changed_stacks" + + ./.compose-workflow/scripts/deployment/classify-rollback-scope.sh \ + --changed-files "$CHANGED_FILES" \ + --stack-dirs "$stack_dirs" + - name: Detect critical stacks id: detect-critical if: inputs.auto-detect-critical @@ -228,7 +283,15 @@ jobs: run: | set -euo pipefail CURRENT_SHA=$(git -C "$LIVE_REPO_PATH" rev-parse HEAD) - if [[ "$CURRENT_SHA" == "$TARGET_REF" && "$FORCE_DEPLOY" != "true" ]]; then + DIRTY="$(git -C "$LIVE_REPO_PATH" status --porcelain)" + if [[ -n "$DIRTY" ]]; then + # A dirty live tree almost always means a prior per-stack rollback + # left a stack pinned at the previous SHA (git checkout -- dir/ + # moves the worktree but not HEAD, so the SHA comparison below + # cannot see it). Deploy anyway so the reset restores a clean tree. + echo "::warning::live tree is dirty (likely a prior per-stack rollback); forcing deploy" + echo "skipped=false" >> "$GITHUB_OUTPUT" + elif [[ "$CURRENT_SHA" == "$TARGET_REF" && "$FORCE_DEPLOY" != "true" ]]; then echo "skipped=true" >> "$GITHUB_OUTPUT" echo "โ„น๏ธ Already at $TARGET_REF; skipping deploy phase" else @@ -585,6 +648,7 @@ jobs: timeout-minutes: 5 outputs: status: ${{ steps.h.outputs.status }} + failed_stacks: ${{ steps.h.outputs.failed_stacks }} env: LIVE_REPO_PATH: ${{ inputs.live-repo-path }} CRITICAL_STACKS: ${{ inputs.auto-detect-critical && needs.prepare.outputs.critical_stacks || inputs.critical-services }} @@ -644,9 +708,15 @@ jobs: fi done if [[ ${#failed[@]} -gt 0 ]]; then + # Emit the culprit list BEFORE `exit 1` โ€” a step that exits + # non-zero still has its already-written $GITHUB_OUTPUT honoured, + # but nothing written after the exit would be. + json=$(printf '"%s",' "${failed[@]}" | sed 's/,$//') + echo "failed_stacks=[$json]" >> "$GITHUB_OUTPUT" echo "status=failed" >> "$GITHUB_OUTPUT" exit 1 fi + echo "failed_stacks=[]" >> "$GITHUB_OUTPUT" echo "status=healthy" >> "$GITHUB_OUTPUT" docker-prune: @@ -689,13 +759,204 @@ jobs: && (needs.deploy.result == 'failure' || needs.health-check.result == 'failure') runs-on: [self-hosted, "${{ inputs.runner-label }}"] timeout-minutes: 15 + outputs: + mode: ${{ steps.plan.outputs.mode }} + culprits: ${{ steps.plan.outputs.culprits }} env: LIVE_REPO_PATH: ${{ inputs.live-repo-path }} PREVIOUS_SHA: ${{ needs.prepare.outputs.previous_sha }} OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} + ROLLBACK_SCOPE: ${{ needs.prepare.outputs.rollback_scope || 'whole-tree' }} + DEPLOY_EXISTING_FAILED: ${{ needs.deploy.outputs.existing_failed_stacks || '[]' }} + DEPLOY_NEW_FAILED: ${{ needs.deploy.outputs.new_failed_stacks || '[]' }} + HEALTH_FAILED: ${{ needs.health-check.outputs.failed_stacks || '[]' }} + NEW_STACKS: ${{ needs.prepare.outputs.new_stacks || '[]' }} + CHANGED_STACKS: ${{ needs.prepare.outputs.changed_stacks || '[]' }} + SERVICE_STARTUP_TIMEOUT: ${{ inputs.service-startup-timeout }} steps: + - name: Resolve rollback plan + id: plan + run: | + set -euo pipefail + + # Validate each stack list independently. A malformed value means an + # upstream output regressed or a `|| '[]'` default failed to apply โ€” + # a real bug worth shouting about. But this is the FIRST step of the + # recovery job: aborting here skips every rollback step and leaves + # production broken. So degrade to the conservative whole-tree path + # and raise a ::error:: annotation, which surfaces in the run log and + # the step summary without preventing recovery. + sanitize() { + local name="$1" value="$2" + if jq -e 'type == "array" and all(.[]; type == "string" and length > 0)' \ + <<<"$value" >/dev/null 2>&1; then + printf '%s' "$value" + return 0 + fi + echo "::error::$name was not a valid stack array (got: ${value:0:200}); forcing whole-tree rollback" >&2 + printf '[]' + return 1 + } + + # `var=$(f)` exposes f's exit status, but assignments inside f run in + # the command-substitution subshell and would not propagate โ€” so the + # degraded flag is set here in the parent from that status. + degraded="" + a=$(sanitize DEPLOY_EXISTING_FAILED "$DEPLOY_EXISTING_FAILED") || degraded="yes" + b=$(sanitize DEPLOY_NEW_FAILED "$DEPLOY_NEW_FAILED") || degraded="yes" + c=$(sanitize HEALTH_FAILED "$HEALTH_FAILED") || degraded="yes" + # NEW_STACKS is validated here rather than in the per-stack step: if + # it is unusable we cannot tell a new stack from an existing one, and + # the safe response is to not take the per-stack path at all. + sanitize NEW_STACKS "$NEW_STACKS" >/dev/null || degraded="yes" + + culprits=$(jq -cn --argjson a "$a" --argjson b "$b" --argjson c "$c" \ + '($a + $b + $c) | unique') + count=$(jq 'length' <<<"$culprits") + + # Any culprit name that fails the stack-name pattern means an + # upstream output is corrupt. Do not act on it โ€” but do not abort + # either: the whole-tree path never uses these names (it resets the + # tree and iterates prepare's own existing/removed lists, a different + # producer), so degrading discards the bad name and still recovers. + # Aborting here would leave production broken. + # Fed by process substitution, not a pipe, so the loop runs in this + # shell and `degraded` propagates. + while IFS= read -r name; do + [[ -z "$name" ]] && continue + if [[ ! "$name" =~ ^[a-zA-Z0-9._-]+$ ]]; then + echo "::error::invalid stack name in culprit list: ${name:0:100}; forcing whole-tree rollback" >&2 + degraded="yes" + fi + done < <(jq -r '.[]' <<<"$culprits") + + # C1: every culprit must be a stack whose directory this deploy + # actually changed. health-check iterates the *critical* stacks, not + # the changed ones, so `failed_stacks` can name a stack that is + # byte-identical at PREVIOUS_SHA and TARGET_REF โ€” reverting it is a + # no-op that leaves the genuinely-changed stack un-rolled-back while + # the job reports success. If any culprit is outside the touched set + # we cannot tell which stack to scope to, so take the whole tree. + # + # `all(.[]; โ€ฆ)` is vacuously true over an empty culprit list, so this + # never fires on []; the `count > 0` test below already forces + # whole-tree in that case. + changed_set=$(sanitize CHANGED_STACKS "$CHANGED_STACKS") || degraded="yes" + if ! jq -e --argjson c "$changed_set" \ + 'all(.[]; . as $x | $c | index($x) != null)' <<<"$culprits" >/dev/null 2>&1; then + echo "::warning::culprit list includes stacks this deploy did not change; forcing whole-tree rollback" + degraded="yes" + fi + + # Per-stack requires BOTH a stack-confined change set AND a known + # culprit. An empty culprit list means the failure was not attributed + # to any specific stack (a teardown failure, an infrastructure error, + # a timeout before any stack was named) โ€” there is nothing to scope + # to, so the whole-tree reset is the only correct response. + if [[ -n "$degraded" ]]; then + mode="whole-tree" + elif [[ "$ROLLBACK_SCOPE" == "per-stack" && "$count" -gt 0 ]]; then + mode="per-stack" + else + mode="whole-tree" + fi + + { + echo "mode=$mode" + echo "culprits=$culprits" + } >> "$GITHUB_OUTPUT" + echo "๐Ÿ”„ Rollback mode: $mode (culprits: $culprits, scope: $ROLLBACK_SCOPE)" + + if [[ -n "$degraded" ]]; then + { + echo "## Rollback input degraded" + echo "One or more stack lists were malformed; forced a whole-tree" + echo "rollback. See the \`::error::\` annotations on the" + echo "\`Resolve rollback plan\` step for the offending value." + } >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Roll back failed stacks only + if: steps.plan.outputs.mode == 'per-stack' + env: + CULPRITS: ${{ steps.plan.outputs.culprits }} + run: | + set -euo pipefail + for stack in $(jq -r '.[]' <<<"$CULPRITS"); do + # Unreachable by construction: this step only runs when mode is + # per-stack, and `Resolve rollback plan` forces whole-tree if any + # culprit name fails this same pattern. Kept as defence in depth, + # and skipping rather than exiting so a surprise cannot strand the + # remaining culprits mid-rollback. + [[ "$stack" =~ ^[a-zA-Z0-9._-]+$ ]] || { + echo "::error::invalid stack name: $stack; skipping"; continue; } + + # Was this stack introduced by the failing deploy? If so it has no + # previous state to restore โ€” tear it down and leave it gone, the + # same disposition the whole-tree path gives new stacks. + if jq -e --arg s "$stack" 'index($s) != null' <<<"$NEW_STACKS" >/dev/null; then + echo "๐Ÿ›‘ Tearing down failed new stack $stack" + if [[ -f "$LIVE_REPO_PATH/$stack/compose.yaml" || -f "$LIVE_REPO_PATH/$stack/compose.yml" ]]; then + (cd "$LIVE_REPO_PATH/$stack" && docker compose down) \ + || echo "::warning::down failed for $stack" + else + # Parity with the whole-tree teardown step's annotation: a + # missing compose file means containers may still be running + # with nothing left to address them by, which an operator needs + # to see rather than have silently skipped. + echo "::warning::compose file missing for failed new stack $stack" + fi + continue + fi + + echo "โช Reverting $stack to $PREVIOUS_SHA" + # Partial checkout: only this stack's directory moves back. HEAD + # stays at the target SHA and the tree is left dirty. That dirt is + # the record of the rollback and must survive for an operator to + # inspect, so there is deliberately no cleanup step here โ€” the next + # deploy's `git reset --hard $TARGET_REF` clears it. + # + # That guarantee depends on the deploy job's skip-gate treating a + # dirty tree as a reason to deploy. Without that branch a re-run at + # the same target-ref short-circuits on the HEAD==TARGET_REF test, + # the reset never runs, and the stack stays pinned here forever + # while the run reports success. Do not remove it. + # + # A stack absent at PREVIOUS_SHA makes this an unmatched pathspec, + # which exits non-zero and under `set -e` would abort the loop and + # strand every remaining culprit. Warn and move on instead. + if ! git -C "$LIVE_REPO_PATH" checkout "$PREVIOUS_SHA" -- "$stack/"; then + echo "::warning::cannot revert $stack to $PREVIOUS_SHA (not present at that SHA); skipping" + continue + fi + + [[ -f "$LIVE_REPO_PATH/$stack/compose.yaml" || -f "$LIVE_REPO_PATH/$stack/compose.yml" ]] || { + echo "::warning::no compose file for $stack after revert; skipping up" + continue + } + + # No --pull always and no --build: land on the locally-tagged + # previous image kept on disk by the prune policy, so recovery does + # not depend on a registry being reachable mid-incident. + # + # The `op run` wrapper is REQUIRED. Without it every ${VAR} in the + # compose file resolves to empty and the stack comes up + # misconfigured while reporting success. + # + # `timeout` matches every deploy-path `up`. Without it a container + # stuck in `starting` โ€” exactly what a bad image produces โ€” hangs + # `--wait` until the job's timeout-minutes cancels it, stranding + # every remaining culprit un-rolled-back with no whole-tree fallback + # (a cancelled job takes no fallback path at all). + (cd "$LIVE_REPO_PATH/$stack" && \ + 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::per-stack 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 + - name: Tear down new stacks (will not exist after reset) - if: needs.prepare.outputs.has_new_stacks == 'true' + if: steps.plan.outputs.mode == 'whole-tree' && needs.prepare.outputs.has_new_stacks == 'true' env: NEW_STACKS: ${{ needs.prepare.outputs.new_stacks }} run: | @@ -716,17 +977,27 @@ jobs: done - name: Reset live tree to previous SHA + if: steps.plan.outputs.mode == 'whole-tree' run: git -C "$LIVE_REPO_PATH" reset --hard "$PREVIOUS_SHA" - name: Redeploy stacks at previous SHA - # Only the stacks this deploy actually touched need reverting: - # - existing: roll their config back to the previous SHA + if: steps.plan.outputs.mode == 'whole-tree' + # 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 @@ -743,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: @@ -810,6 +1087,8 @@ jobs: HAS_DISABLED: ${{ needs.prepare.outputs.has_disabled_stacks }} DISABLED_STACKS: ${{ needs.prepare.outputs.disabled_stacks }} FORCE_DEPLOY: ${{ inputs.force-deploy }} + ROLLBACK_MODE: ${{ needs.rollback.outputs.mode }} + ROLLBACK_CULPRITS: ${{ needs.rollback.outputs.culprits }} run: | set -euo pipefail @@ -898,7 +1177,14 @@ jobs: rollback_line="" if [[ "$rollback_status" != "skipped" ]]; then rb_icon="โœ…"; [[ "$rollback_status" != "success" ]] && rb_icon="โŒ" - rollback_line=" โ†’ $rb_icon Rollback" + rb_detail="" + if [[ "$ROLLBACK_MODE" == "per-stack" ]]; then + rb_names=$(jq -r '. | join(", ")' <<<"${ROLLBACK_CULPRITS:-[]}" 2>/dev/null || echo "") + rb_detail=" (per-stack: ${rb_names:-unknown})" + elif [[ "$ROLLBACK_MODE" == "whole-tree" ]]; then + rb_detail=" (whole-tree)" + fi + rollback_line=" โ†’ $rb_icon Rollback$rb_detail" fi pipeline="$deploy_icon Deploy โ†’ $health_icon Health$rollback_line" { diff --git a/docs/superpowers/plans/2026-08-24-scoped-rollback-and-image-quarantine.md b/docs/superpowers/plans/2026-08-24-scoped-rollback-and-image-quarantine.md new file mode 100644 index 0000000..88f0f32 --- /dev/null +++ b/docs/superpowers/plans/2026-08-24-scoped-rollback-and-image-quarantine.md @@ -0,0 +1,782 @@ +# Scoped Rollback and Image Quarantine Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Confine a failed stack's rollback to that stack alone, and give the operator a one-command way to permanently block a known-bad image version from being re-proposed by Renovate. + +**Architecture:** Change A adds a scope classifier to `deploy.yml`'s `prepare` job that decides between a new per-stack rollback path and the existing whole-tree reset, plus the plumbing (a `failed_stacks` output on `health-check`) needed to know which stacks to roll back. Change B adds a standalone user skill that reverts a bad image bump and writes a negated-regex `allowedVersions` block into the consuming repo's Renovate config, in one atomic commit. + +**Tech Stack:** Bash 5, GitHub Actions reusable workflows, `jq`, `git`, Renovate config JSON, shellcheck/yamllint. + +**Spec:** `docs/superpowers/specs/2026-08-24-scoped-rollback-and-image-quarantine-design.md` + +--- + +## Context for the implementer + +You are working in `~/Git/Compose/compose-workflow`, a repository of **reusable GitHub Actions workflows** consumed by two private repos (`docker-piwine`, `docker-piwine-office`). Those repos each hold a set of Docker Compose "stacks" โ€” one directory per stack, each containing a `compose.yaml`. + +Facts you need that are not obvious from the code: + +- **Deploys run on a self-hosted runner** that owns a persistent clone of the caller repo at `inputs.live-repo-path`. The workflow mutates that clone directly. There is no ephemeral checkout for the deploy itself. +- **Every deploy starts with `git -C "$LIVE_REPO_PATH" reset --hard "$TARGET_REF"`** (`.github/workflows/deploy.yml:283`). This is why leaving the live tree dirty after a partial rollback is safe โ€” the next run cleans it. Do not add a cleanup step for this. +- **Secrets come from 1Password at runtime.** Every `docker compose up` is wrapped in `op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- โ€ฆ`. **A `compose up` without that wrapper comes up with empty environment variables and appears to succeed.** This is the single easiest way to break this change. Non-`up` compose calls (`down`, `ps`) are deliberately *not* wrapped. +- **Unit tests are local-only.** `scripts/testing/*.sh` are not invoked by any workflow. Run them by hand. Do not wire them into CI as part of this plan โ€” that is out of scope. +- **`jq` is available on the runner** and used throughout the workflow. + +### Deviation from the spec, and why + +The spec (ยงA1) places the scope classifier "after `Get changed files` (`deploy.yml:120`)". **Implement it after `Detect stack changes` instead.** + +Reason: the classifier decides whether a changed path's first segment names a stack directory. It therefore needs the full set of known stack directories โ€” and a *removed* stack's directory no longer exists in the target tree, so it is absent from `discover-stacks`'s output. Classifying against active stacks alone would misfile every stack deletion as a root-file change and force whole-tree rollback on routine removals. The `removed_stacks` list only exists after `Detect stack changes` runs, so the classifier must follow it. + +This is a placement change only. The classification rules in the spec's table are unchanged. + +--- + +## File Structure + +| File | Status | Responsibility | +|---|---|---| +| `scripts/deployment/classify-rollback-scope.sh` | Create | Pure function: changed-file list + known stack dirs โ†’ `rollback_scope`. No git, no docker, no network. | +| `scripts/testing/test-classify-rollback-scope.sh` | Create | Unit tests for the classifier. Mirrors `test-detect-stack-changes.sh` harness style. | +| `.github/workflows/deploy.yml` | Modify | Wire the classifier into `prepare`; add `failed_stacks` to `health-check`; add the per-stack branch to `rollback`; report scope in `notify`. | +| `~/.claude/skills/quarantine-image/SKILL.md` | Create | The `/quarantine-image` skill. Lives outside this repo (user skills dir), matching `renovate-trigger`. | + +The classifier is a separate script rather than inline YAML because inline `run:` blocks cannot be unit-tested, and this is the one piece of new logic with enough branches to be worth testing. Everything else in Change A is plumbing that is only meaningfully verified end-to-end on a real host. + +--- + +## Task 1: Rollback scope classifier + +**Files:** +- Create: `scripts/deployment/classify-rollback-scope.sh` +- Test: `scripts/testing/test-classify-rollback-scope.sh` + +- [ ] **Step 1: Write the failing test** + +Create `scripts/testing/test-classify-rollback-scope.sh`: + +```bash +#!/usr/bin/env bash +# Unit tests for scripts/deployment/classify-rollback-scope.sh +# Pure input/output tests โ€” no git repos, no docker. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +CLASSIFY="$REPO_ROOT/scripts/deployment/classify-rollback-scope.sh" + +TMPROOT=$(mktemp -d -t classify-tests.XXXXXX) +trap 'rm -rf "$TMPROOT"' EXIT + +PASS=0 +FAIL=0 +FAILURES=() + +# expect_scope +expect_scope() { + local name="$1" expected="$2" changed="$3" dirs="$4" + local out actual + out=$(mktemp -p "$TMPROOT") + GITHUB_OUTPUT="$out" "$CLASSIFY" \ + --changed-files "$changed" \ + --stack-dirs "$dirs" \ + >/dev/null 2>&1 || true + actual=$(grep '^rollback_scope=' "$out" 2>/dev/null | cut -d= -f2- || echo "") + if [[ "$actual" == "$expected" ]]; then + PASS=$((PASS + 1)) + echo " โœ… $name" + else + FAIL=$((FAIL + 1)) + FAILURES+=("$name: expected '$expected', got '$actual'") + echo " โŒ $name: expected '$expected', got '$actual'" + fi +} + +STACKS='["termix","monitoring","swag"]' + +echo "== classify-rollback-scope ==" + +# Stack-scoped: every changed path lives under a known stack dir. +expect_scope "single stack file" \ + "per-stack" '["termix/compose.yaml"]' "$STACKS" +expect_scope "two stacks" \ + "per-stack" '["termix/compose.yaml","monitoring/compose.yaml"]' "$STACKS" +expect_scope "nested file under stack" \ + "per-stack" '["swag/config/nginx.conf"]' "$STACKS" + +# Root-scoped: anything outside a known stack dir forces whole-tree. +expect_scope "compose.env at root" \ + "whole-tree" '["compose.env"]' "$STACKS" +expect_scope "mixed stack and root" \ + "whole-tree" '["termix/compose.yaml","compose.env"]' "$STACKS" +expect_scope "workflow file" \ + "whole-tree" '[".github/workflows/deploy.yml"]' "$STACKS" +expect_scope "unknown top-level dir" \ + "whole-tree" '["newstack/compose.yaml"]' "$STACKS" + +# Degenerate inputs default to the safe path. +expect_scope "empty changed list" \ + "whole-tree" '[]' "$STACKS" +expect_scope "empty string changed list" \ + "whole-tree" '' "$STACKS" +expect_scope "empty stack dirs" \ + "whole-tree" '["termix/compose.yaml"]' '[]' + +# A stack dir name that is a prefix of another must not match loosely. +expect_scope "prefix collision is not a match" \ + "whole-tree" '["termix-old/compose.yaml"]' "$STACKS" + +echo +echo "Passed: $PASS Failed: $FAIL" +if [[ $FAIL -gt 0 ]]; then + printf ' - %s\n' "${FAILURES[@]}" + exit 1 +fi +``` + +Make it executable: + +```bash +chmod +x scripts/testing/test-classify-rollback-scope.sh +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `./scripts/testing/test-classify-rollback-scope.sh` + +Expected: every case fails with `got ''`, because `classify-rollback-scope.sh` does not exist yet. Exit code 1. + +- [ ] **Step 3: Write the implementation** + +Create `scripts/deployment/classify-rollback-scope.sh`: + +```bash +#!/usr/bin/env bash +# Script Name: classify-rollback-scope.sh +# Purpose: Decide whether a failed deploy can be rolled back per-stack or +# requires the whole-tree reset. +# Usage: ./classify-rollback-scope.sh \ +# --changed-files '["termix/compose.yaml"]' \ +# --stack-dirs '["termix","monitoring"]' +# +# Emits: rollback_scope=per-stack|whole-tree (to $GITHUB_OUTPUT) +# +# A deploy is per-stack rollbackable only when EVERY changed path's first +# segment names a known stack directory. Anything else โ€” compose.env, +# .github/**, a README, an unrecognised top-level dir โ€” is repo-wide and +# cannot be undone by reverting one stack directory, so it falls back to the +# whole-tree reset. +# +# compose.env is the motivating case: it lives at the repo root and is passed +# to every stack via `op run --env-file`. A bad edit there (renamed var, dead +# 1Password reference) breaks stacks whose own directories never changed. +# +# Uncertainty always resolves to whole-tree. Never the reverse. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib/common.sh +source "$SCRIPT_DIR/lib/common.sh" + +CHANGED_FILES="[]" +STACK_DIRS="[]" + +while [[ $# -gt 0 ]]; do + case $1 in + --changed-files) CHANGED_FILES="${2:-[]}"; shift 2 ;; + --stack-dirs) STACK_DIRS="${2:-[]}"; shift 2 ;; + *) + log_error "Unknown argument: $1" + exit 1 + ;; + esac +done + +# Normalise empty/absent inputs to empty JSON arrays. +[[ -z "$CHANGED_FILES" ]] && CHANGED_FILES="[]" +[[ -z "$STACK_DIRS" ]] && STACK_DIRS="[]" + +emit_whole_tree() { + log_info "Rollback scope: whole-tree ($1)" + set_github_output "rollback_scope" "whole-tree" + exit 0 +} + +# Malformed JSON from an upstream step must not crash the deploy; fall back. +if ! jq -e 'type == "array"' <<<"$CHANGED_FILES" >/dev/null 2>&1; then + emit_whole_tree "changed-files was not a JSON array" +fi +if ! jq -e 'type == "array"' <<<"$STACK_DIRS" >/dev/null 2>&1; then + emit_whole_tree "stack-dirs was not a JSON array" +fi + +changed_count=$(jq 'length' <<<"$CHANGED_FILES") +if [[ "$changed_count" -eq 0 ]]; then + emit_whole_tree "no changed-file list available" +fi + +dirs_count=$(jq 'length' <<<"$STACK_DIRS") +if [[ "$dirs_count" -eq 0 ]]; then + emit_whole_tree "no known stack directories" +fi + +# Split each path on the first "/" and require the segment to be an exact +# member of the stack-dir set. Exact membership (not prefix matching) is what +# keeps "termix-old/โ€ฆ" from being mistaken for the "termix" stack. +# +# A path with no "/" is a root-level file: its first segment is the whole +# path, which will not match any stack dir, so it correctly forces whole-tree. +outside=$(jq -r --argjson dirs "$STACK_DIRS" ' + [ .[] | select((split("/")[0]) as $seg | ($dirs | index($seg)) == null) ] + | .[]' <<<"$CHANGED_FILES") + +if [[ -n "$outside" ]]; then + log_info "Paths outside known stack directories:" + while IFS= read -r p; do + [[ -n "$p" ]] && log_info " - $p" + done <<<"$outside" + emit_whole_tree "changes touch repo-root or unknown paths" +fi + +log_success "Rollback scope: per-stack (all changes confined to stack directories)" +set_github_output "rollback_scope" "per-stack" +``` + +Make it executable: + +```bash +chmod +x scripts/deployment/classify-rollback-scope.sh +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `./scripts/testing/test-classify-rollback-scope.sh` + +Expected: `Passed: 11 Failed: 0`, exit code 0. + +- [ ] **Step 5: Run shellcheck** + +Run: `shellcheck -x scripts/deployment/classify-rollback-scope.sh scripts/testing/test-classify-rollback-scope.sh` + +Expected: no output, exit code 0. This is the same invocation `.github/workflows/workflow-lint.yml:71` uses, so a clean run here means CI will be clean. + +- [ ] **Step 6: Commit** + +```bash +git add scripts/deployment/classify-rollback-scope.sh scripts/testing/test-classify-rollback-scope.sh +git commit -m "feat(deploy): add rollback scope classifier + +Decides per-stack vs whole-tree rollback from the changed-file list. +Uncertainty always resolves to whole-tree." +``` + +--- + +## Task 2: Wire the classifier into `prepare` + +**Files:** +- Modify: `.github/workflows/deploy.yml` (job outputs at `:65-80`; new step after `Detect stack changes` at `:165-186`) + +- [ ] **Step 1: Add the job output** + +In the `prepare` job's `outputs:` block, after the `critical_stacks` line, add: + +```yaml + rollback_scope: ${{ steps.classify-scope.outputs.rollback_scope }} +``` + +- [ ] **Step 2: Add the classifier step** + +Insert this step **after** the `Detect stack changes` step and **before** `Detect critical stacks`: + +```yaml + - name: Classify rollback scope + id: classify-scope + # Runs after detect-changes because it needs removed_stacks: a removed + # stack's directory is gone from the target tree, so discover-stacks + # never sees it. Without it in the known-dirs set, every stack deletion + # would look like a root-level change and force whole-tree rollback. + env: + CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files != '' && steps.changed-files.outputs.all_changed_files || '[]' }} + ACTIVE_STACKS: ${{ steps.discover-stacks.outputs.stacks }} + DISABLED_STACKS: ${{ steps.discover-stacks.outputs.disabled_stacks }} + REMOVED_STACKS: ${{ steps.detect-changes.outputs.removed_stacks || '[]' }} + run: | + set -euo pipefail + stack_dirs=$(jq -cn \ + --argjson a "$ACTIVE_STACKS" \ + --argjson b "$DISABLED_STACKS" \ + --argjson c "$REMOVED_STACKS" \ + '($a + $b + $c) | unique') + ./.compose-workflow/scripts/deployment/classify-rollback-scope.sh \ + --changed-files "$CHANGED_FILES" \ + --stack-dirs "$stack_dirs" +``` + +Note the `./.compose-workflow/` prefix: `prepare` checks the workflow's own repo out to that path (`deploy.yml:90-94`) pinned to `job.workflow_sha`. Every other script call in this job uses the same prefix โ€” match it. + +- [ ] **Step 3: Verify `all_changed_files` is the right output name** + +Run: `grep -n "changed-files.outputs" .github/workflows/deploy.yml` + +Expected: existing uses are `deleted_files` and `added_files`. Confirm `tj-actions/changed-files@v47` also exposes `all_changed_files` with `json: true` set (it does โ€” the action's `json: true` input applies to every file-list output). If the JSON array does not materialise at runtime, the classifier's array-type guard emits `whole-tree` and the deploy behaves exactly as it does today. The failure mode is safe. + +- [ ] **Step 4: Lint the workflow** + +Run: +```bash +yamllint --strict --config-file .yamllint .github/workflows/deploy.yml +actionlint .github/workflows/deploy.yml +``` + +Expected: no output from either, exit code 0. + +- [ ] **Step 5: Commit** + +```bash +git add .github/workflows/deploy.yml +git commit -m "feat(deploy): expose rollback_scope from prepare" +``` + +--- + +## Task 3: `health-check` reports which stacks failed + +**Files:** +- Modify: `.github/workflows/deploy.yml` (`health-check` outputs block; the `failed=()` handling at the end of step `h`) + +Why this is its own task: `health-check` currently emits only `status`. A bad image release most often fails *health*, not *deploy* โ€” so without this output the per-stack path would have an empty culprit list on exactly the case this whole change exists to handle, and would silently degrade to whole-tree. + +- [ ] **Step 1: Add the output declaration** + +In the `health-check` job's `outputs:` block, alongside `status`: + +```yaml + failed_stacks: ${{ steps.h.outputs.failed_stacks }} +``` + +- [ ] **Step 2: Emit the list before exiting** + +Replace the tail of step `h` (currently the `if [[ ${#failed[@]} -gt 0 ]]` block) with: + +```bash + if [[ ${#failed[@]} -gt 0 ]]; then + # Emit the culprit list BEFORE `exit 1` โ€” a step that exits + # non-zero still has its already-written $GITHUB_OUTPUT honoured, + # but nothing written after the exit would be. + json=$(printf '"%s",' "${failed[@]}" | sed 's/,$//') + echo "failed_stacks=[$json]" >> "$GITHUB_OUTPUT" + echo "status=failed" >> "$GITHUB_OUTPUT" + exit 1 + fi + echo "failed_stacks=[]" >> "$GITHUB_OUTPUT" + echo "status=healthy" >> "$GITHUB_OUTPUT" +``` + +The `printf`/`sed` JSON construction is copied verbatim from the `deploy-existing` step (`deploy.yml:424-425`) so both jobs build their arrays identically. Stack names are already constrained to `^[a-zA-Z0-9._-]+$` upstream, so no escaping is needed. + +- [ ] **Step 3: Lint** + +Run: +```bash +yamllint --strict --config-file .yamllint .github/workflows/deploy.yml +actionlint .github/workflows/deploy.yml +``` + +Expected: clean. + +- [ ] **Step 4: Commit** + +```bash +git add .github/workflows/deploy.yml +git commit -m "feat(deploy): emit failed_stacks from health-check" +``` + +--- + +## Task 4: Per-stack rollback branch + +**Files:** +- Modify: `.github/workflows/deploy.yml` (`rollback` job, currently `:684-748`) + +This is the highest-risk task in the plan. The rollback job is what runs when things are *already* broken; a bug here turns a one-stack outage into a fleet outage. + +- [ ] **Step 1: Extend the job's `needs` and `env`** + +The `rollback` job currently declares `needs: [prepare, deploy, health-check]` โ€” unchanged. Add to its `env:` block: + +```yaml + ROLLBACK_SCOPE: ${{ needs.prepare.outputs.rollback_scope || 'whole-tree' }} + DEPLOY_EXISTING_FAILED: ${{ needs.deploy.outputs.existing_failed_stacks || '[]' }} + DEPLOY_NEW_FAILED: ${{ needs.deploy.outputs.new_failed_stacks || '[]' }} + HEALTH_FAILED: ${{ needs.health-check.outputs.failed_stacks || '[]' }} + NEW_STACKS: ${{ needs.prepare.outputs.new_stacks || '[]' }} +``` + +The `|| 'whole-tree'` default matters: if `prepare` is ever changed such that the classifier step is skipped, the expression yields empty and this restores today's behavior rather than an empty scope string that matches neither branch. + +- [ ] **Step 2: Add the culprit-resolution step as the job's first step** + +```yaml + - name: Resolve rollback plan + id: plan + run: | + set -euo pipefail + culprits=$(jq -cn \ + --argjson a "$DEPLOY_EXISTING_FAILED" \ + --argjson b "$DEPLOY_NEW_FAILED" \ + --argjson c "$HEALTH_FAILED" \ + '($a + $b + $c) | unique') + count=$(jq 'length' <<<"$culprits") + + # Per-stack requires BOTH a stack-confined change set AND a known + # culprit. An empty culprit list means the failure was not attributed + # to any specific stack (a teardown failure, an infrastructure error, + # a timeout before any stack was named) โ€” there is nothing to scope + # to, so the whole-tree reset is the only correct response. + if [[ "$ROLLBACK_SCOPE" == "per-stack" && "$count" -gt 0 ]]; then + mode="per-stack" + else + mode="whole-tree" + fi + + { + echo "mode=$mode" + echo "culprits=$culprits" + } >> "$GITHUB_OUTPUT" + echo "๐Ÿ”„ Rollback mode: $mode (culprits: $culprits, scope: $ROLLBACK_SCOPE)" +``` + +- [ ] **Step 3: Add the per-stack rollback step** + +Insert after `Resolve rollback plan`, before the existing `Tear down new stacks` step: + +```yaml + - name: Roll back failed stacks only + if: steps.plan.outputs.mode == 'per-stack' + env: + CULPRITS: ${{ steps.plan.outputs.culprits }} + run: | + set -euo pipefail + for stack in $(jq -r '.[]' <<<"$CULPRITS"); do + [[ "$stack" =~ ^[a-zA-Z0-9._-]+$ ]] || { + echo "::error::invalid stack name: $stack"; exit 1; } + + # Was this stack introduced by the failing deploy? If so it has no + # previous state to restore โ€” tear it down and leave it gone, the + # same disposition the whole-tree path gives new stacks. + if jq -e --arg s "$stack" 'index($s) != null' <<<"$NEW_STACKS" >/dev/null; then + echo "๐Ÿ›‘ Tearing down failed new stack $stack" + if [[ -f "$LIVE_REPO_PATH/$stack/compose.yaml" || -f "$LIVE_REPO_PATH/$stack/compose.yml" ]]; then + (cd "$LIVE_REPO_PATH/$stack" && docker compose down) \ + || echo "::warning::down failed for $stack" + fi + continue + fi + + echo "โช Reverting $stack to $PREVIOUS_SHA" + # Partial checkout: only this stack's directory moves back. HEAD + # stays at the target SHA and the tree is left dirty โ€” that is + # fine, the next deploy's `git reset --hard $TARGET_REF` + # (deploy.yml:283) cleans it before anything else runs. + git -C "$LIVE_REPO_PATH" checkout "$PREVIOUS_SHA" -- "$stack/" + + [[ -f "$LIVE_REPO_PATH/$stack/compose.yaml" || -f "$LIVE_REPO_PATH/$stack/compose.yml" ]] || { + echo "::warning::no compose file for $stack after revert; skipping up" + continue + } + + # No --pull always and no --build: land on the locally-tagged + # previous image kept on disk by the prune policy, so recovery does + # not depend on a registry being reachable mid-incident. + # + # The `op run` wrapper is REQUIRED. Without it every ${VAR} in the + # compose file resolves to empty and the stack comes up + # misconfigured while reporting success. + (cd "$LIVE_REPO_PATH/$stack" && \ + op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- \ + docker compose up -d --quiet-pull --wait --remove-orphans) \ + || echo "::warning::per-stack rollback up failed for $stack" + done +``` + +- [ ] **Step 4: Gate the three existing whole-tree steps** + +Add `if: steps.plan.outputs.mode == 'whole-tree'` to each of the existing steps: + +| Step | Current `if:` | New `if:` | +|---|---|---| +| `Tear down new stacks (will not exist after reset)` | `needs.prepare.outputs.has_new_stacks == 'true'` | `steps.plan.outputs.mode == 'whole-tree' && needs.prepare.outputs.has_new_stacks == 'true'` | +| `Reset live tree to previous SHA` | *(none)* | `steps.plan.outputs.mode == 'whole-tree'` | +| `Redeploy stacks at previous SHA` | *(none)* | `steps.plan.outputs.mode == 'whole-tree'` | + +Change **only** the `if:` conditions. Leave the bodies of these three steps exactly as they are โ€” they are the tested status quo and the fallback for every case the new path declines to handle. + +- [ ] **Step 5: Lint** + +Run: +```bash +yamllint --strict --config-file .yamllint .github/workflows/deploy.yml +actionlint .github/workflows/deploy.yml +``` + +Expected: clean. + +- [ ] **Step 6: Re-read the diff against the two invariants** + +Run: `git diff` + +Confirm by eye: +1. Every `docker compose up` you added is wrapped in `op run --no-masking --env-file=โ€ฆ`. +2. The three original whole-tree steps have gained an `if:` and nothing else. + +- [ ] **Step 7: Commit** + +```bash +git add .github/workflows/deploy.yml +git commit -m "feat(deploy): scope rollback to failed stacks when safe + +Per-stack rollback runs only when the change set is confined to stack +directories AND a culprit stack was identified. Every other case keeps +the existing whole-tree reset." +``` + +--- + +## Task 5: Report rollback scope in the Discord notification + +**Files:** +- Modify: `.github/workflows/deploy.yml` (`notify` job `env:` block at `:805-811`; pipeline line construction at `:897-902`) + +- [ ] **Step 1: Pass the plan into `notify`** + +Add to the `notify` job's `env:` block for the summary step: + +```yaml + ROLLBACK_MODE: ${{ needs.rollback.outputs.mode }} + ROLLBACK_CULPRITS: ${{ needs.rollback.outputs.culprits }} +``` + +This requires the `rollback` job to expose them. Add to the `rollback` job: + +```yaml + outputs: + mode: ${{ steps.plan.outputs.mode }} + culprits: ${{ steps.plan.outputs.culprits }} +``` + +- [ ] **Step 2: Extend the pipeline line** + +Replace the `rollback_line` construction: + +```bash + rollback_line="" + if [[ "$rollback_status" != "skipped" ]]; then + rb_icon="โœ…"; [[ "$rollback_status" != "success" ]] && rb_icon="โŒ" + rb_detail="" + if [[ "$ROLLBACK_MODE" == "per-stack" ]]; then + rb_names=$(jq -r '. | join(", ")' <<<"${ROLLBACK_CULPRITS:-[]}" 2>/dev/null || echo "") + rb_detail=" (per-stack: ${rb_names:-unknown})" + elif [[ "$ROLLBACK_MODE" == "whole-tree" ]]; then + rb_detail=" (whole-tree)" + fi + rollback_line=" โ†’ $rb_icon Rollback$rb_detail" + fi +``` + +A whole-tree rollback and a one-stack rollback are currently indistinguishable in the alert. That distinction is the first thing the reader needs: it is the difference between "one app is down" and "the entire fleet just moved backwards". + +- [ ] **Step 3: Lint** + +Run: +```bash +yamllint --strict --config-file .yamllint .github/workflows/deploy.yml +actionlint .github/workflows/deploy.yml +``` + +Expected: clean. + +- [ ] **Step 4: Commit** + +```bash +git add .github/workflows/deploy.yml +git commit -m "feat(deploy): report rollback scope and culprits in notification" +``` + +--- + +## Task 6: The `quarantine-image` skill + +**Files:** +- Create: `~/.claude/skills/quarantine-image/SKILL.md` + +This file lives **outside this repository**, in the user's skills directory, matching `~/.claude/skills/renovate-trigger/SKILL.md`. It is a single markdown file: frontmatter, prose, inline bash recipes. No helper scripts, no plugin packaging. Read `renovate-trigger/SKILL.md` first and match its voice and structure. + +- [ ] **Step 1: Read the reference skill** + +Run: `cat ~/.claude/skills/renovate-trigger/SKILL.md` + +Note the conventions: `name` + `description` frontmatter where the description enumerates trigger phrases; `## Prerequisites`; one `##` section per operation; explicit guidance on what to do when a step finds nothing. + +- [ ] **Step 2: Write the skill** + +Create `~/.claude/skills/quarantine-image/SKILL.md` covering, in order: + +**Frontmatter.** `name: quarantine-image`. The `description` must enumerate trigger phrases: "quarantine an image", "block a bad image version", "this update broke the deploy", "stop Renovate from re-proposing", "revert and block", "unblock an image version". + +**Why revert alone does not work.** State the mechanism up front, because it is the reason the skill exists: `compose-workflow/default.json` groups minor/patch/digest updates into `all-deps` with `automerge: true` and `minimumReleaseAge: "1 hour"`. A `git revert` makes the old version current again, Renovate sees the newer tag as available, and re-proposes and re-merges it within the hour. + +**The blocking mechanism, with the trap called out.** `allowedVersions` accepts a version *range* interpreted by the active versioning scheme, or a *regex* in forward slashes, negated as `!/โ€ฆ/`. Use the negated regex. **`!=release-2.7.1` will not work** โ€” it is range syntax, and these repos pin images to custom `regex:` versioning schemes where range operators are not dependable. A negated regex filters the raw version string before any versioning scheme parses it, so one form covers `release-2.7.1`, `4.2.1-ls286`, and `2026-08-24` alike: + +```json +{ "matchPackageNames": ["ghcr.io/lukegus/termix"], + "allowedVersions": "!/^release-2\\.7\\.1$/" } +``` + +**Prerequisites.** Run from `~/Git/Compose`. `git` and `jq` available. `npx` available for config validation. + +**Locating the stack:** + +```bash +matches=$(ls -d docker-piwine*/"$STACK"/ 2>/dev/null) +count=$(wc -l <<<"$matches") +``` + +Zero matches or more than one is an error โ€” ask the user which repo, never guess. + +**Finding the last-known-good image line.** Walk the file's history and take the previous `image:` line verbatim, tag *and* digest together: + +```bash +git -C "$REPO" log --format=%H -- "$STACK/compose.yaml" \ + | while read -r sha; do + line=$(git -C "$REPO" show "$sha:$STACK/compose.yaml" | grep -m1 "image: $PKG:") + case "$line" in *"$BAD_VERSION"*) continue ;; esac + echo "$line"; break + done +``` + +Emphasise: **never compose a `@sha256:` digest by hand.** Restoring a tag/digest pair that a real Renovate run already wrote is what keeps this consistent with the project's "Renovate owns digest pinning" rule. + +**Writing the block.** Edit the consuming repo's `.github/renovate.json` โ€” never `compose-workflow/default.json`, since a bad `termix` build is not `docker-piwine-office`'s problem. Two cases, and the second is the one that bites: + +- No existing block for the package โ†’ append a new rule object. +- A block already exists โ†’ **widen the alternation in place**: `!/^release-(2\.7\.1|2\.8\.0)$/`. Do not append a second rule. Two `packageRules` entries matching the same package both apply and the later `allowedVersions` silently wins, discarding the earlier block. + +Keep the block rule **separate** from the package's existing `versioning` rule (see `docker-piwine/.github/renovate.json:19-23` for termix). Renovate merges matching rules in order so the two coexist, and separation lets `--unblock` delete a whole object rather than surgically editing a shared one. + +**Committing.** Both files in **one commit**. State why the ordering is load-bearing: the compose revert makes `2.7.0` current in the same commit that makes `2.7.1` invisible. Split across two commits there is a window where the pinned-current version is also the blocked version, which Renovate handles badly. + +**Validating before pushing:** + +```bash +npx --yes --package renovate -- renovate-config-validator "$REPO/.github/renovate.json" +``` + +**After pushing.** Do not trigger a Renovate run โ€” point at the existing `renovate-trigger` skill. + +**`--unblock `.** Remove the package's rule object and its comment, commit, note that Renovate will offer the version again on its next run. Do not touch the compose file: by unblock time you want the newest version, not the one you blocked. + +**Cases to refuse rather than guess at:** + +| Case | Behavior | +|---|---| +| Stack name matches zero or multiple repos | Error; ask the user to disambiguate | +| No prior version in history | Error; nothing to revert to โ€” tell the user to fix forward | +| `image:` uses a floating tag (`:latest`) | Error; no version to block โ€” the answer is a digest pin, not a quarantine | + +- [ ] **Step 3: Verify the skill is discoverable** + +Run: `ls ~/.claude/skills/quarantine-image/SKILL.md && head -5 ~/.claude/skills/quarantine-image/SKILL.md` + +Expected: the file exists and the frontmatter parses (opening `---`, `name:`, `description:`, closing `---`). The skill will be listed in a new Claude Code session. + +- [ ] **Step 4: Dry-run the recipes against the real termix history** + +Do not commit anything. Verify each recipe returns what the skill claims: + +```bash +cd ~/Git/Compose +# Stack resolution finds exactly one repo +ls -d docker-piwine*/termix/ 2>/dev/null +# The previous image line is recoverable verbatim, with its digest +git -C docker-piwine log --format=%H -- termix/compose.yaml | head -5 +git -C docker-piwine show 65606eb:termix/compose.yaml | grep -m1 'image: ghcr.io/lukegus/termix' +``` + +Expected: one directory; a list of SHAs; an `image:` line pinned to `release-2.7.0` **with** an `@sha256:` digest. If the digest is absent, the recipe in Step 2 needs adjusting before the skill is trusted. + +- [ ] **Step 5: Verify a block validates** + +Build a throwaway copy and run the real validator against it: + +```bash +cp docker-piwine/.github/renovate.json /tmp/rv.json +jq '.packageRules += [{"matchPackageNames":["ghcr.io/lukegus/termix"],"allowedVersions":"!/^release-2\\.7\\.1$/"}]' \ + /tmp/rv.json > /tmp/rv-blocked.json +npx --yes --package renovate -- renovate-config-validator /tmp/rv-blocked.json +``` + +Expected: the validator reports the config is valid. If it rejects the `allowedVersions` string, stop โ€” the escaping in the skill is wrong and must be fixed before Task 6 is considered done. + +- [ ] **Step 6: Commit (this repo's docs only)** + +The skill file lives outside this repo and is not committed here. Nothing to commit for this task unless the dry-run revealed a spec inaccuracy โ€” in which case amend the spec and commit that. + +--- + +## Task 7: Live-host validation โ€” NOT PERFORMED (decision, 2026-08-25) + +The user declined a live test. Deliberately breaking a stack on `docker-piwine` +to exercise both rollback paths was offered and turned down, and the per-stack +path was shipped **enabled** rather than behind an opt-in flag. + +**Decision:** ship active; the first real stack failure exercises the new path. + +### What this leaves unverified + +Everything logic-level is covered: 19 classifier unit tests, the 8 existing +transition tests, and stubbed dry runs of the rollback loop against real +throwaway git repos. Four things are not, and unit tests structurally cannot +reach them: + +1. `op run` + `docker compose up` against a rolled-back tree โ€” no 1Password or + Docker daemon in the dev environment. +2. `git checkout $PREVIOUS_SHA -- /` against the runner's *persistent* + clone (tested only against throwaway repos). +3. GitHub Actions expression evaluation of the new outputs โ€” specifically the + skipped-job empty-string cases and the `|| '[]'` defaults. +4. The classifier receiving a real `all_changed_files` value post-`escape_json` + fix (verified against the action's source, not a live run). + +### Known accepted risk + +The per-stack step swallows `up` failures as warnings: + +```bash +op run ... docker compose up ... || echo "::warning::per-stack rollback up failed for $stack" +``` + +This matches the pre-existing whole-tree step's behavior, so it is consistent โ€” +but it means a per-stack rollback that fails to bring the stack back up still +reports the job **green**, and the Discord line shows a success icon. Without a +live test, the first occurrence will be during a real incident. + +Mitigating factor: every malformed-input path in `Resolve rollback plan` +degrades to `whole-tree`, so the *scope* decision fails safe. The residual risk +is concentrated in the docker/`op`/Actions layer, not in the classification +logic. + +If this proves noisy in practice, the fix is to collect failed stacks in the +loop and `exit 1` at the end, so the job result reflects reality. + +## Out of scope + +Named here so they are not quietly added: + +- Automatic quarantine from the deploy workflow. The workflow cannot distinguish a bad image from a bad healthcheck, a bad config change, or a transient registry blip, and a false positive would block a good version. +- Changes to `minimumReleaseAge` or automerge policy in `default.json`. +- Expiry metadata or scheduled auditing of accumulated blocks. +- Wiring `scripts/testing/*.sh` into CI. Worth doing; not part of this change. diff --git a/docs/superpowers/specs/2026-08-24-scoped-rollback-and-image-quarantine-design.md b/docs/superpowers/specs/2026-08-24-scoped-rollback-and-image-quarantine-design.md new file mode 100644 index 0000000..a94e359 --- /dev/null +++ b/docs/superpowers/specs/2026-08-24-scoped-rollback-and-image-quarantine-design.md @@ -0,0 +1,320 @@ +# Scoped rollback and image quarantine + +**Status:** Approved (design) +**Date:** 2026-08-24 +**Scope:** `compose-workflow` reusable workflow (`deploy.yml`); new user skill `quarantine-image`; caller-repo `.github/renovate.json` + +## Problem + +A bad upstream image release (`ghcr.io/lukegus/termix:release-2.7.1`) could not start. That single stack +failure took down every subsequent deployment in `docker-piwine`. Two independent defects combined to +produce the outage, and neither one is fixed by fixing the other. + +### Defect 1 โ€” unbounded blast radius + +`deploy.yml` rolls back by resetting the entire live tree: + +``` +rollback: + - git -C "$LIVE_REPO_PATH" reset --hard "$PREVIOUS_SHA" # deploy.yml:719 + - docker compose up ... for every existing + removed stack # deploy.yml:736-748 +``` + +One stack's failure therefore reverts the configuration of all fifteen stacks in the repo. Stacks that +deployed successfully moments earlier are silently rolled backwards for a reason unrelated to them. + +### Defect 2 โ€” the failure is self-perpetuating + +Rollback changes the live tree; it does not change `main`. The bad `image:` line is still committed. +Every deploy begins with `git reset --hard "$TARGET_REF"` (`deploy.yml:283`), so the next commit to land +โ€” any unrelated Renovate PR โ€” re-applies the bad image, fails again, and rolls back again. The pipeline +stays red indefinitely and no further changes reach the host. + +The obvious manual fix, `git revert` of the bad bump, does not hold. `compose-workflow/default.json` +groups every `minor`/`patch`/`digest` update into `all-deps` with `automerge: true` and +`minimumReleaseAge: "1 hour"`. Reverting makes the old version current again; Renovate observes a newer +tag available and re-proposes and re-merges it within the hour. **Revert alone is structurally incapable +of holding**, because Renovate has no memory of a version being bad. + +## Design + +Two independent changes that compose. Change A makes a bad release *survivable*. Change B makes it +*stop recurring*. Neither is sufficient alone. + +--- + +## Change A โ€” scoped rollback in `deploy.yml` + +Rollback gains a per-stack path used when the failure is provably confined to stack directories. The +existing whole-tree path is retained unchanged as the fallback for every other case. + +### A1. `prepare` classifies rollback scope + +New step after `Get changed files` (`deploy.yml:120`). It reads the `all_changed_files` JSON already +produced by `tj-actions/changed-files` and takes the first path segment of each changed path. + +| Condition | `rollback_scope` | +|---|---| +| Every changed path's first segment is a known stack directory | `per-stack` | +| Any changed path is a root file (`compose.env`, `.github/**`, `README.md`, โ€ฆ) | `whole-tree` | +| `changed-files` step was skipped (`previous_sha == target-ref`, e.g. `force-deploy`) | `whole-tree` | + +New `prepare` job output: `rollback_scope`. + +The `whole-tree` default on uncertainty is deliberate. `compose.env` is repo-root and shared by every +stack via `op run --env-file="$LIVE_REPO_PATH/compose.env"`. A failure caused by a renamed variable or a +dead 1Password reference lives *outside* any stack directory, so reverting one stack directory would fix +nothing. Whole-tree reset is the correct behavior for that case and must remain reachable. + +### A2. `health-check` names its casualties + +`health-check` builds a `failed=()` array but currently exposes only `status`. Add a `failed_stacks` +JSON output alongside it. + +Without this, a *health* failure (as distinct from a *deploy* failure) has no stack list to scope +against, and every health failure would silently degrade to whole-tree rollback โ€” losing most of the +benefit of this change, since health failures are the common shape of a bad image release. + +### A3. `rollback` gains a per-stack branch + +A new first step unions the culprit lists: + +``` +culprits = deploy.outputs.existing_failed_stacks + โˆช deploy.outputs.new_failed_stacks + โˆช health-check.outputs.failed_stacks +``` + +Then two mutually exclusive branches: + +**Per-stack** โ€” when `rollback_scope == 'per-stack'` AND `culprits` is non-empty. For each culprit: + +| Culprit type | Action | +|---|---| +| new stack | `docker compose down`; stays gone (matches current behavior) | +| existing stack | `git checkout $PREVIOUS_SHA -- /`, then `op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose up -d --quiet-pull --wait --remove-orphans` | + +No `--pull always` and no `--build`, matching the current rollback step: the stack lands on the +locally-tagged previous image kept on disk by the docker-prune policy, so recovery does not depend on a +registry being reachable mid-incident. Healthy stacks are never touched. + +**Whole-tree** โ€” every other case. Today's exact behavior, unchanged: tear down new stacks, +`git reset --hard "$PREVIOUS_SHA"`, re-up existing + removed. + +### A4. Partial checkout leaves the live tree dirty โ€” safe, but only because of two other things + +`git checkout $PREVIOUS_SHA -- /` moves the index and the worktree but **not `HEAD`**. The live +tree is left with `HEAD` at `TARGET_REF` and one directory's content at `PREVIOUS_SHA` โ€” a dirty tree +that persists after the job ends: + +``` +$ git checkout $T0 -- termix/ +$ git rev-parse HEAD # 7db462aโ€ฆ โ€” still T1, unchanged +$ git status --porcelain # M termix/compose.yaml +``` + +That dirt is deliberate. It is the on-disk record of which stack was rolled back, and it must survive +for an operator to inspect, so there is no cleanup step in the `rollback` job. It is cleared by the +*next* deploy's `git reset --hard "$TARGET_REF"`. + +The original version of this section claimed that reset alone made the state safe, and that "no drift +accumulates across runs". That was wrong on both counts. Two separate mechanisms are actually required, +and each is load-bearing. + +**1. The skip-gate must treat a dirty tree as a reason to deploy.** + +The `deploy` job's skip-gate compares `git rev-parse HEAD` against `TARGET_REF` and skips the whole +deploy phase when they match. After a per-stack rollback they *do* match โ€” `HEAD` never moved. So a +re-run at the same `target-ref` (the on-call's first instinct: "Re-run failed jobs") short-circuited +before the reset, reported a green *"Repository already at target commit"*, and left the stack pinned at +the previous SHA indefinitely. The reset that this section relied on never ran. + +The skip-gate therefore tests `git status --porcelain` first and forces `skipped=false` when the tree is +dirty, ahead of the SHA comparison. Removing that branch reopens the hole. + +**2. The next deploy must re-`up` the rolled-back stack, not merely restore its files.** + +`reset --hard "$TARGET_REF"` puts the bad line back on disk. If nothing then runs `docker compose up` +for that stack, the containers keep serving the *previous* SHA's images while the tree claims the new +one โ€” silent drift, invisible until that stack changes again. + +Nothing does re-`up` it explicitly. It works because of an incidental property of +`detect-stack-changes.sh`: `existing_stacks` is computed as *(all discovered stacks โˆ’ new stacks)*, not +as the `PREVIOUS_SHA..TARGET_REF` diff. It therefore names the entire fleet on every run, and +`Deploy existing stacks` brings every stack up on every deploy. A previously rolled-back stack is +re-`up`ed along with everything else, fails again on the still-bad image, and is rolled back again โ€” +which is exactly the fallback described under "What Change A explicitly does not fix". + +This is a real guarantee but a fragile one, because it rests on a definition that reads like a bug. **If +`existing_stacks` is ever narrowed to the actual change set, this section's guarantee breaks and a +rolled-back stack will drift.** The fix at that point is to record the rolled-back stack names +(an untracked marker file in the live tree, unioned into `existing_stacks` by `prepare`) so they are +re-deployed explicitly rather than incidentally. Note that no code in this repository runs `git clean` +on the live tree, so an untracked marker would survive `reset --hard` โ€” verified by +`grep -rn "git clean" .github/ scripts/`, which matches nothing. + +**Scope of the culprit list.** Related, and fixed alongside: `health-check` iterates the *critical* +stacks, which are detected from labels across all discovered stacks, not from this deploy's change set. +Its `failed_stacks` output could therefore name a stack that is byte-identical at `PREVIOUS_SHA` and +`TARGET_REF`, making `git checkout $PREVIOUS_SHA -- /` a no-op while the stack that *did* change +was never reverted. `prepare` now also emits `changed_stacks` โ€” the first path segment of every changed +file that names a known stack directory โ€” and `Resolve rollback plan` requires every culprit to be a +member of it. A culprit outside that set means the failure cannot be attributed to a stack this deploy +touched, so the rollback degrades to whole-tree. `changed_stacks` is used rather than `existing_stacks` +precisely because, per the above, `existing_stacks` names the whole fleet and would make the check +vacuous. + +### A5. `notify` reports which path ran + +The Discord message currently renders a pipeline line such as `โœ… Deploy โ†’ โŒ Health โ†’ โœ… Rollback`. +Extend the rollback segment with the scope and culprit list: + +``` +๐Ÿ”„ Rollback: per-stack (termix) +๐Ÿ”„ Rollback: whole-tree +``` + +Without this, a narrowly-scoped rollback is indistinguishable in the alert from a full fleet revert โ€” +which is precisely the distinction the on-call reader needs first. + +### What Change A explicitly does not fix + +With no auto-quarantine, the bad `image:` line remains on `main`. The next deploy re-applies it, fails +on that stack, and rolls that stack back again. Unrelated updates in the same commit still land +successfully โ€” that is the win โ€” but the run is still red and still pages. Stopping recurrence is +Change B. + +--- + +## Change B โ€” the `quarantine-image` skill + +### B1. The blocking mechanism + +Renovate's `allowedVersions` accepts either a version *range* interpreted by the active versioning +scheme, or a *regex* delimited by forward slashes, with `!/โ€ฆ/` as the negated (exclude) form. + +**The negated regex is the correct mechanism here, and range syntax such as `!=release-2.7.1` is not.** +Ranges are interpreted by the versioning scheme, and these repos pin several images to custom `regex:` +versioning schemes where range operators are not dependable: + +```json +{ "matchPackageNames": ["ghcr.io/lukegus/termix"], + "versioning": "regex:^release-(?\\d+)\\.(?\\d+)\\.(?\\d+)$" } +``` + +A negated regex filters the **raw version string** before any versioning scheme parses it. One mechanism +therefore covers every tag format in the fleet โ€” `release-2.7.1`, LSIO's `4.2.1-ls286`, homebridge's +`2026-08-24`: + +```json +{ "matchPackageNames": ["ghcr.io/lukegus/termix"], + "allowedVersions": "!/^release-2\\.7\\.1$/" } +``` + +Because `allowedVersions` filters at lookup time, a blocked version is not merely un-automerged โ€” it is +invisible to Renovate. Paired with reverting the compose file, that is what makes a revert hold. + +### B2. Shape and invocation + +A single `~/.claude/skills/quarantine-image/SKILL.md`, matching the existing `renovate-trigger` and +`renovate-dashboard-triage` convention: prose with inline bash recipes, no helper scripts, no plugin +packaging. + +``` +/quarantine-image "" +/quarantine-image --unblock +``` + +The stack name locates the repo by globbing `docker-piwine*//compose.yaml`. A name matching more +than one repo is an error, not a guess. + +### B3. Quarantine steps + +1. **Resolve the image.** Read the `image:` line from the stack's compose file; split package name + (`ghcr.io/lukegus/termix`) from tag and digest. For multi-service stacks, the supplied bad version + identifies which service. + +2. **Find the last-known-good line.** Walk `git log -p -- /compose.yaml` back to the commit + before the bad version landed and take that `image:` line **verbatim** โ€” tag and digest together, + exactly as Renovate originally wrote them. The skill never composes a `@sha256:` by hand; it restores + a tag/digest pair a real Renovate run already verified. + +3. **Write the block.** Add or extend a `packageRules` entry in the **consuming repo's** + `.github/renovate.json` โ€” never the shared `compose-workflow/default.json`, since a bad `termix` + build is not `docker-piwine-office`'s problem. + + | Case | Action | + |---|---| + | No existing block for the package | Append a new rule object with the negated-regex `allowedVersions` | + | A block already exists | Widen the alternation in place: `!/^release-(2\.7\.1\|2\.8\.0)$/` | + + Extending in place rather than appending matters: two `packageRules` entries matching the same + package both apply, and the later `allowedVersions` silently wins, discarding the earlier block. + + This rule is kept **separate** from the package's existing `versioning` rule + (`docker-piwine/.github/renovate.json:19-23`). Renovate merges matching rules in order, so the two + coexist, and keeping them apart lets `--unblock` delete a whole object instead of surgically editing + a shared one. + +4. **Commit both files together, atomically.** The ordering is load-bearing: the compose revert makes + `2.7.0` current *in the same commit* that makes `2.7.1` invisible. Split across two commits, there is + a window in which the pinned-current version is also the blocked version โ€” a state Renovate handles + badly. The commit message records the reason and the failing run. + +5. **Report and stop.** Print the diff and push. The skill does **not** trigger a Renovate run; it + points at the existing `renovate-trigger` skill rather than reimplementing it. + +### B4. `--unblock` + +Removes the package's rule object and its comment from `.github/renovate.json`, commits, and notes that +Renovate will offer the previously-blocked version again on its next run. It does not touch the compose +file: by the time you unblock, you want the newest version, not the one you blocked. + +### B5. Block lifecycle + +Blocks are single-version and therefore self-expiring in effect. When upstream ships `2.7.2`, Renovate +offers it normally; the stale rule sits inert and documented until pruned. `--unblock` is the manual +prune. No scheduled audit and no expiry metadata โ€” that machinery would cost more than the stale JSON +objects it removes. + +### B6. Cases the skill must refuse rather than guess + +| Case | Behavior | +|---|---| +| Stack name matches zero or multiple repos | Error; ask the user to disambiguate | +| No prior version in history (bad version was the first ever pinned) | Error; nothing to revert to, tell the user to fix forward | +| `image:` uses a floating tag (`:latest`) | Error; there is no version to block โ€” the answer is a digest pin, not a quarantine | + +--- + +## Worked example: the termix incident under this design + +1. Renovate merges `release-2.7.1`; the image cannot start. +2. Deploy fails on `termix` only. `rollback_scope` is `per-stack` (only `termix/compose.yaml` changed), + so `termix/` reverts to `release-2.7.0` and comes back up. The other fourteen stacks keep their new + configuration. Discord reports `๐Ÿ”„ Rollback: per-stack (termix)`. +3. Operator runs `/quarantine-image termix release-2.7.1 "container exits on boot"`. One commit reverts + the compose line and adds `"allowedVersions": "!/^release-2\\.7\\.1$/"`. +4. Subsequent deploys are green. Renovate never re-proposes `2.7.1`. +5. Upstream ships `2.7.2`; Renovate offers it through the normal `all-deps` path. The stale block + remains until `/quarantine-image --unblock termix`. + +## Testing + +| Change | Verification | +|---|---| +| A1 scope classifier | Unit-test the path-segment logic against fixture file lists: stack-only, root-only, mixed, empty | +| A2 health-check output | Assert `failed_stacks` JSON is well-formed and matches `status=failed` | +| A3 per-stack branch | Deploy a deliberately-broken image to one non-critical stack on the office Pi; assert only that stack reverts and the others stay at the new SHA | +| A3 whole-tree fallback | Same, with a `compose.env` edit in the commit; assert whole-tree reset runs | +| A5 notify | Inspect the rendered Discord payload for both scopes | +| B | `npx --yes renovate-config-validator` on the edited `.github/renovate.json`; confirm on the Dependency Dashboard that the blocked version no longer appears | + +## Out of scope + +- Automatic quarantine from the deploy workflow. The workflow cannot distinguish a bad image from a bad + healthcheck, a bad config change, or a transient registry blip, and a false positive would block a + good version. +- Changes to `minimumReleaseAge` or automerge policy in `default.json`. Preventing bad releases from + being adopted at all is a separate trade-off from recovering when one is. +- Expiry metadata or scheduled auditing of blocks (see B5). diff --git a/scripts/deployment/classify-rollback-scope.sh b/scripts/deployment/classify-rollback-scope.sh new file mode 100755 index 0000000..42359ae --- /dev/null +++ b/scripts/deployment/classify-rollback-scope.sh @@ -0,0 +1,144 @@ +#!/usr/bin/env bash +# Script Name: classify-rollback-scope.sh +# Purpose: Decide whether a failed deploy can be rolled back per-stack or +# requires the whole-tree reset. +# Usage: ./classify-rollback-scope.sh \ +# --changed-files '["termix/compose.yaml"]' \ +# --stack-dirs '["termix","monitoring"]' +# +# Emits: rollback_scope=per-stack|whole-tree (to $GITHUB_OUTPUT) +# +# A deploy is per-stack rollbackable only when EVERY changed path's first +# segment names a known stack directory. Anything else โ€” compose.env, +# .github/**, a README, an unrecognised top-level dir โ€” is repo-wide and +# cannot be undone by reverting one stack directory, so it falls back to the +# whole-tree reset. +# +# compose.env is the motivating case: it lives at the repo root and is passed +# to every stack via `op run --env-file`. A bad edit there (renamed var, dead +# 1Password reference) breaks stacks whose own directories never changed. +# +# Uncertainty always resolves to whole-tree. Never the reverse. +# +# Invocation errors (unknown flag) are distinct from malformed runtime data: +# an unknown flag means the caller is wired up wrong and exits 1 loudly, so +# the mistake surfaces immediately instead of quietly deploying with the +# wrong scope every time. Malformed *data* (bad JSON, non-string elements, +# a missing value) instead degrades safely to whole-tree, rc=0 โ€” the deploy +# should not fail just because the classifier couldn't classify. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib/common.sh +source "$SCRIPT_DIR/lib/common.sh" + +CHANGED_FILES="[]" +STACK_DIRS="[]" + +while [[ $# -gt 0 ]]; do + case $1 in + --changed-files) + # A missing value โ€” flag at end of argv, or immediately followed by + # another flag โ€” leaves this empty, which normalises to [] -> whole-tree. + # Never consume the next flag, never `shift 2` past the end. Check $# + # (not the value of $2) so an explicit empty-string argument is still + # consumed as a value โ€” ${2:-} can't tell "unset" from "set to ''". + CHANGED_FILES="" + if [[ $# -ge 2 && $2 != --* ]]; then CHANGED_FILES=$2; shift; fi + shift + ;; + --stack-dirs) + STACK_DIRS="" + if [[ $# -ge 2 && $2 != --* ]]; then STACK_DIRS=$2; shift; fi + shift + ;; + *) + log_error "Unknown argument: $1" + exit 1 + ;; + esac +done + +# Normalise empty/absent inputs to empty JSON arrays. +[[ -z "$CHANGED_FILES" ]] && CHANGED_FILES="[]" +[[ -z "$STACK_DIRS" ]] && STACK_DIRS="[]" + +# emit_whole_tree [severity] +# severity defaults to "info" for expected/normal degradations (empty lists, +# no known stacks, changes outside a stack dir). Pass "warn" for genuine +# anomalies in the upstream data (malformed JSON) so an operator notices +# without having to go dig through the upstream step's output. +emit_whole_tree() { + local reason="$1" severity="${2:-info}" + if [[ "$severity" == "warn" ]]; then + log_warning "Rollback scope: whole-tree ($reason)" + else + log_info "Rollback scope: whole-tree ($reason)" + fi + set_github_output "rollback_scope" "whole-tree" + exit 0 +} + +# is_string_array : true iff is a JSON array whose every element +# is a non-empty string. Shared by both input guards below so tightening the +# predicate can't be done to one and forgotten on the other. +is_string_array() { + jq -e 'type == "array" and all(.[]; type == "string" and length > 0)' <<<"$1" >/dev/null 2>&1 +} + +# Malformed JSON from an upstream step must not crash the deploy; fall back. +# Every element must be a non-empty string: a non-string element (number, +# null, nested array) would blow up the split()/index() pipeline below under +# `set -euo pipefail`, killing the script before any output is written โ€” and +# an empty-string element would vacuously satisfy the "no paths outside a +# stack dir" check further down, silently landing on the unsafe per-stack +# side. Both must be caught here, before they reach the pipeline. These are +# genuine anomalies (not a normal empty-list degradation), hence "warn". +if ! is_string_array "$CHANGED_FILES"; then + emit_whole_tree "changed-files was not an array of non-empty strings: ${CHANGED_FILES:0:200}" warn +fi +if ! is_string_array "$STACK_DIRS"; then + emit_whole_tree "stack-dirs was not an array of non-empty strings: ${STACK_DIRS:0:200}" warn +fi + +# An empty changed-file list must NOT be read as "nothing outside a stack dir": +# the jq filter below is vacuously true over [], which yields per-stack. This +# guard is load-bearing โ€” deleting it inverts the safe default. Do not remove. +changed_count=$(jq 'length' <<<"$CHANGED_FILES") +if [[ "$changed_count" -eq 0 ]]; then + emit_whole_tree "no changed-file list available" +fi + +# Empty stack-dirs would already fall out as whole-tree (index() over [] is null +# for every path). This guard exists only for the clearer reason string. +dirs_count=$(jq 'length' <<<"$STACK_DIRS") +if [[ "$dirs_count" -eq 0 ]]; then + emit_whole_tree "no known stack directories" +fi + +# Split each path on the first "/" and require the segment to be an exact +# member of the stack-dir set. Exact membership (not prefix matching) is what +# keeps "termix-old/โ€ฆ" from being mistaken for the "termix" stack. +# +# A path with no "/" is a root-level file, so its first segment is the whole +# path. If that path happens to be spelled identically to a known stack +# directory name (e.g. a root-level file literally named "termix"), it WILL +# match and count as per-stack โ€” this is the same exact first-segment +# membership rule applied uniformly, not a special case. In practice repo +# root files (compose.env, README, .github/**) don't collide with stack dir +# names, but this is not a semantic guarantee. +outside=$(jq -r --argjson dirs "$STACK_DIRS" ' + [ .[] | select((split("/")[0]) as $seg | ($dirs | index($seg)) == null) ] + | .[]' <<<"$CHANGED_FILES") + +if [[ -n "$outside" ]]; then + log_info "Paths outside known stack directories:" + while IFS= read -r p; do + [[ -n "$p" ]] && log_info " - $p" + done <<<"$outside" + emit_whole_tree "changes touch repo-root or unknown paths" +fi + +log_success "Rollback scope: per-stack (all changes confined to stack directories)" +set_github_output "rollback_scope" "per-stack" diff --git a/scripts/testing/test-classify-rollback-scope.sh b/scripts/testing/test-classify-rollback-scope.sh new file mode 100755 index 0000000..0dfa99d --- /dev/null +++ b/scripts/testing/test-classify-rollback-scope.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# Unit tests for scripts/deployment/classify-rollback-scope.sh +# Pure input/output tests โ€” no git repos, no docker. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +CLASSIFY="$REPO_ROOT/scripts/deployment/classify-rollback-scope.sh" + +TMPROOT=$(mktemp -d -t classify-tests.XXXXXX) +trap 'rm -rf "$TMPROOT"' EXIT + +PASS=0 +FAIL=0 +FAILURES=() + +# expect_case "> -- +# Runs the classifier with an arbitrary argv and asserts BOTH the emitted +# rollback_scope output AND the exit code. Asserting rc matters: a case that +# only checks output would miss a regression that writes the right value and +# then exits nonzero. +expect_case() { + local name="$1" expected="$2" expected_rc="$3" + shift 4 # drop name, expected, expected_rc, and the "--" separator + local out actual rc=0 + out=$(mktemp -p "$TMPROOT") + GITHUB_OUTPUT="$out" "$CLASSIFY" "$@" >/dev/null 2>&1 || rc=$? + actual=$(grep '^rollback_scope=' "$out" 2>/dev/null | cut -d= -f2- || echo "") + if [[ "$actual" == "$expected" && "$rc" == "$expected_rc" ]]; then + PASS=$((PASS + 1)) + echo " โœ… $name" + else + FAIL=$((FAIL + 1)) + FAILURES+=("$name: expected scope='$expected' rc=$expected_rc, got scope='$actual' rc=$rc") + echo " โŒ $name: expected scope='$expected' rc=$expected_rc, got scope='$actual' rc=$rc" + fi +} + +# expect_scope +# Thin two-flag wrapper over expect_case, always asserting rc=0. All existing +# call sites below stay textually unchanged and now also assert exit code. +expect_scope() { + local name="$1" expected="$2" changed="$3" dirs="$4" + expect_case "$name" "$expected" 0 -- --changed-files "$changed" --stack-dirs "$dirs" +} + +STACKS='["termix","monitoring","swag"]' + +echo "== classify-rollback-scope ==" + +# Stack-scoped: every changed path lives under a known stack dir. +expect_scope "single stack file" \ + "per-stack" '["termix/compose.yaml"]' "$STACKS" +expect_scope "two stacks" \ + "per-stack" '["termix/compose.yaml","monitoring/compose.yaml"]' "$STACKS" +expect_scope "nested file under stack" \ + "per-stack" '["swag/config/nginx.conf"]' "$STACKS" + +# Root-scoped: anything outside a known stack dir forces whole-tree. +expect_scope "compose.env at root" \ + "whole-tree" '["compose.env"]' "$STACKS" +expect_scope "mixed stack and root" \ + "whole-tree" '["termix/compose.yaml","compose.env"]' "$STACKS" +expect_scope "workflow file" \ + "whole-tree" '[".github/workflows/deploy.yml"]' "$STACKS" +expect_scope "unknown top-level dir" \ + "whole-tree" '["newstack/compose.yaml"]' "$STACKS" + +# Degenerate inputs default to the safe path. +expect_scope "empty changed list" \ + "whole-tree" '[]' "$STACKS" +expect_scope "empty string changed list" \ + "whole-tree" '' "$STACKS" +expect_scope "empty stack dirs" \ + "whole-tree" '["termix/compose.yaml"]' '[]' + +# A stack dir name that is a prefix of another must not match loosely. +expect_scope "prefix collision is not a match" \ + "whole-tree" '["termix-old/compose.yaml"]' "$STACKS" + +# Non-string / empty-string elements must not crash the script or silently +# fall through to the unsafe per-stack side. +expect_scope "non-string element" \ + "whole-tree" '[1]' "$STACKS" +expect_scope "null element" \ + "whole-tree" '[null]' "$STACKS" +expect_scope "empty-string element" \ + "whole-tree" '[""]' "$STACKS" +expect_scope "nested-array element" \ + "whole-tree" '[["a"]]' "$STACKS" +expect_scope "non-string stack dir" \ + "whole-tree" '["termix/compose.yaml"]' '[1]' + +# Missing-value invocation shapes must not crash the script โ€” they normalise +# to [] and fall through to whole-tree, rc=0. +expect_case "trailing flag with no value" \ + "whole-tree" 0 -- --changed-files +expect_case "missing value before next flag" \ + "whole-tree" 0 -- --changed-files --stack-dirs "$STACKS" + +# Invocation errors (unknown flag) are deliberately NOT degraded to +# whole-tree: they mean the caller is wired up wrong, and should fail loudly +# (rc=1, no output) rather than silently deploy with a scope decision nobody +# intended. This is the most opinionated behavior in the script โ€” cover it so +# a well-meaning "make everything fail-safe" edit can't remove it unnoticed. +expect_case "unknown flag fails loudly" \ + "" 1 -- --bogus + +echo +echo "Passed: $PASS Failed: $FAIL" +if [[ $FAIL -gt 0 ]]; then + printf ' - %s\n' "${FAILURES[@]}" + exit 1 +fi