From d7526c1c49c0ad6607d29ae3f7e3b88fb2fe8137 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Sun, 24 May 2026 19:47:58 -0300 Subject: [PATCH 1/4] fix(ci): hard-fail deploy on sustained 429 instead of silent oauth pass The post-deploy OAuth redirect smoke check had an escape hatch that exited 0 (success) when all retry attempts were rate-limited (429) by Discord, meaning the OAuth contract was never verified yet the deploy was marked passing. Replace the silent exit 0 with exit 1 and informative error messages. Add a documented workflow_dispatch input (allow_deploy_unverified_oauth) for the rare case where Discord rate-limiting is confirmed transient and the operator needs to force through after the auth-config check has passed. ADR: docs/decisions/2026-05-24-deploy-oauth-redirect-smoke-check-429-handling.md --- .github/workflows/deploy.yml | 16 ++++++- ...oauth-redirect-smoke-check-429-handling.md | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 docs/decisions/2026-05-24-deploy-oauth-redirect-smoke-check-429-handling.md diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index bf0f3f582..af6ae34dd 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -2,6 +2,12 @@ name: Deploy to Homelab on: workflow_dispatch: + inputs: + allow_deploy_unverified_oauth: + description: 'Skip OAuth redirect contract check (use only if Discord is rate-limiting and auth-config check passed)' + required: false + default: 'false' + type: boolean workflow_run: workflows: ["Build & Push Docker Images"] types: [completed] @@ -406,6 +412,7 @@ jobs: env: WEBHOOK_URL: ${{ secrets.DEPLOY_WEBHOOK_URL }} EXPECTED_CLIENT_ID: ${{ secrets.WEBAPP_EXPECTED_CLIENT_ID }} + ALLOW_DEPLOY_UNVERIFIED_OAUTH: ${{ inputs.allow_deploy_unverified_oauth || '' }} run: | set -euo pipefail @@ -532,8 +539,13 @@ jobs: done if [ "$non_rate_limited_attempts" -eq 0 ] && [ "$rate_limited_count" -gt 0 ]; then - echo "::warning::OAuth redirect contract check hit sustained Discord rate limiting (429) across all retries; treating as non-blocking after auth-config contract pass." - exit 0 + if [ "${ALLOW_DEPLOY_UNVERIFIED_OAUTH:-false}" = "true" ]; then + echo "::warning::OAuth redirect contract check was rate-limited on all attempts; bypassing because ALLOW_DEPLOY_UNVERIFIED_OAUTH=true — verify OAuth flow manually." + exit 0 + fi + echo "::error::OAuth redirect contract check was rate-limited (429) on all $rate_limited_count attempts — contract not verified." + echo "::error::Options: (1) redeploy after Discord rate-limit clears (~5 min); (2) trigger workflow_dispatch with allow_deploy_unverified_oauth=true to bypass" + exit 1 fi echo "::error::Timed out waiting for OAuth redirect contract" diff --git a/docs/decisions/2026-05-24-deploy-oauth-redirect-smoke-check-429-handling.md b/docs/decisions/2026-05-24-deploy-oauth-redirect-smoke-check-429-handling.md new file mode 100644 index 000000000..85f24071b --- /dev/null +++ b/docs/decisions/2026-05-24-deploy-oauth-redirect-smoke-check-429-handling.md @@ -0,0 +1,47 @@ +# ADR: Deploy OAuth Redirect Smoke Check — Hard Fail on Sustained 429 + +**Date:** 2026-05-24 +**Status:** Accepted + +## Context + +`deploy.yml` includes an "OAuth redirect contract smoke check" step that hits `/api/auth/discord` and validates the redirect to Discord's OAuth2 authorize URL carries the correct `client_id` and `redirect_uri`. It retries 18 times (10s intervals, ~3 min total). + +The prior implementation had a silent-pass escape hatch: if every attempt returned HTTP 429 (Discord rate-limiting), the step exited 0 with only a `::warning::` message. The OAuth contract was never actually verified, but the deploy was marked as passing. + +A preceding "Auth config smoke check" step validates server-side OAuth fields (`clientId`, `redirectUri`, `sessionSecretConfigured`, `redisHealthy`, etc.) but does not exercise the live Discord redirect — it only checks that the values are present and correctly shaped. + +## Decision + +Remove the silent-pass escape hatch. If all attempts are rate-limited (429) and the OAuth redirect contract was never verified, the step now **exits 1 (failure)**. + +A documented `ALLOW_DEPLOY_UNVERIFIED_OAUTH` flag (available as a `workflow_dispatch` input) allows an operator to explicitly bypass the check when Discord's rate-limiting is confirmed to be a transient infrastructure issue and the auth-config check has passed. The bypass is intentional, visible, and auditable — unlike the previous silent escape. + +## Alternatives Considered + +**Keep status quo (silent pass on 429):** Preserves deploy continuity when Discord's API is rate-limiting. Rejected because the step title claims "contract verified" while exit 0 fires without any verification — a semantic mismatch that erodes operator trust and can mask real config problems (e.g., malformed redirect_uri) when a 429 storm coincides with a misconfiguration. + +**Delete the OAuth redirect check entirely:** Relies solely on auth-config smoke check. Rejected because auth-config validates that fields are _present and shaped correctly_ — it does not test the live redirect URL. A malformed `client_id` encoding or wrong `redirect_uri` domain would pass auth-config but fail at the actual OAuth handshake. + +## Consequences + +**Positive:** + +- "Deploy passed" unambiguously means "OAuth contract was verified." +- Config problems (malformed redirect URL, wrong client_id) are caught at deploy time, not after users hit sign-in failures. +- Operator is forced to make a conscious decision if the check fails, rather than silently proceeding. + +**Negative:** + +- A deploy can now fail due to Discord's rate-limiting rather than a Lucky-side issue. This requires operator intervention (wait ~5 min, redeploy). +- If Discord's rate-limit window grows beyond ~3 min, the check will fail routinely and operator friction increases. + +**Neutral:** + +- The escape hatch has fired zero times across 44 production deploy runs (2026-03-14 to 2026-05-24), so the operational impact of removing it is expected to be negligible. + +## Revisit When + +- The `ALLOW_DEPLOY_UNVERIFIED_OAUTH` bypass is used more than twice per month (suggests Discord's rate-limit window exceeds the retry budget, warranting a longer retry loop or a rescheduled check). +- Discord changes its OAuth rate-limit semantics (new headers, different window, per-token vs per-IP). +- A monitoring gap is detected: if users report sign-in failures post-deploy that were not caught at deploy time, re-evaluate whether the auth-config check covers the missed scenario. From 235917905a3de0f930f5a713665f2166f1a0bd6e Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Sun, 24 May 2026 20:19:41 -0300 Subject: [PATCH 2/4] fix(ci): surface async deploy failures via commit statuses deploy.sh now posts pending/success/failure to the github statuses api (context: homelab-deploy) keyed to the post-sync HEAD SHA. the validate-deployed-version step polls these statuses in the no-rebuild path instead of exiting on http 200 from the already-running service. graceful degradation: if GITHUB_DEPLOY_STATUS_TOKEN is absent on the homelab, no status is posted and ci falls back to the existing http 200 health check after a 90s grace period. adds ADR docs/decisions/2026-05-24-deploy-async-completion-signal.md. --- .github/workflows/deploy.yml | 46 ++++++++++++--- ...26-05-24-deploy-async-completion-signal.md | 58 +++++++++++++++++++ scripts/deploy.sh | 24 +++++++- 3 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 docs/decisions/2026-05-24-deploy-async-completion-signal.md diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index af6ae34dd..4924bb435 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -21,6 +21,7 @@ jobs: runs-on: ubuntu-latest permissions: actions: read + contents: read if: | github.event_name == 'workflow_dispatch' || (github.event_name == 'workflow_run' && github.event.workflow_run.conclusion == 'success') @@ -263,6 +264,7 @@ jobs: env: WEBHOOK_URL: ${{ secrets.DEPLOY_WEBHOOK_URL }} DOCKER_REBUILT: ${{ steps.docker_wait.outputs.docker_rebuilt }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail base_url=$(node -e 'const u = new URL(process.env.WEBHOOK_URL); console.log(`${u.protocol}//${u.host}`)') @@ -274,19 +276,49 @@ jobs: if [ "${DOCKER_REBUILT:-false}" != "true" ]; then health_url="${base_url}/api/health" - echo "==> No Docker rebuild for this commit — verifying service health at $health_url..." + echo "==> No Docker rebuild — checking for homelab-deploy commit status (SHA: $expected)..." + + status_was_seen=false for attempt in $(seq 1 20); do echo "Attempt ${attempt}/20..." - response=$(curl -sS --max-time 20 -w "\n%{http_code}" "$health_url" || true) - http_code=$(echo "$response" | tail -1) - if [ "$http_code" = "200" ]; then - echo "==> Service healthy after deploy. Done." + deploy_state=$(gh api \ + "repos/${{ github.repository }}/commits/${expected}/statuses" \ + --jq '[.[] | select(.context == "homelab-deploy")] | first | .state // ""' \ + 2>/dev/null || echo "") + + [ "$deploy_state" = "pending" ] && status_was_seen=true + + if [ "$deploy_state" = "success" ]; then + echo "==> Homelab deploy completed successfully." exit 0 fi - echo "Not ready (HTTP $http_code). Waiting 15s..." + if [ "$deploy_state" = "failure" ] || [ "$deploy_state" = "error" ]; then + echo "::error::Homelab deploy reported failure — check deploy logs on homelab" + exit 1 + fi + + if [ "$status_was_seen" = "false" ] && [ "$attempt" -eq 6 ]; then + echo "::notice::No homelab-deploy status after ~90s — GITHUB_DEPLOY_STATUS_TOKEN not configured on homelab. Falling back to HTTP health check." + for http_attempt in $(seq 1 20); do + echo "HTTP attempt ${http_attempt}/20..." + response=$(curl -sS --max-time 20 -w "\n%{http_code}" "$health_url" || true) + http_code=$(echo "$response" | tail -1) + if [ "$http_code" = "200" ]; then + echo "==> Service healthy. Done." + exit 0 + fi + echo "Not ready (HTTP $http_code). Waiting 15s..." + sleep 15 + done + echo "::error::Service did not become healthy after all attempts" + exit 1 + fi + + echo "Deploy status: ${deploy_state:-none}. Waiting 15s..." sleep 15 done - echo "::error::Service did not become healthy after all attempts" + + echo "::error::Deploy validation timed out" exit 1 fi diff --git a/docs/decisions/2026-05-24-deploy-async-completion-signal.md b/docs/decisions/2026-05-24-deploy-async-completion-signal.md new file mode 100644 index 000000000..794a5022e --- /dev/null +++ b/docs/decisions/2026-05-24-deploy-async-completion-signal.md @@ -0,0 +1,58 @@ +# ADR: Async Deploy Completion Signal via GitHub Commit Statuses + +**Date:** 2026-05-24 +**Status:** Accepted + +## Context + +`deploy-wrapper.sh` runs `nohup bash deploy.sh & disown` so the webhook HTTP response returns immediately. `deploy.sh` executes fully detached from the CI process. + +The "Validate deployed version" step in `deploy.yml` handles two paths: + +- **Rebuild path** (`docker_rebuilt=true`): polls `/api/health/version` for SHA match. Correct — the SHA changes when a new image is built, so HTTP 200 from the old container won't satisfy the check. +- **No-rebuild path** (`docker_rebuilt=false`): polled `/api/health` for HTTP 200. Incorrect — the already-running service returns 200 immediately, before `deploy.sh` has executed (git sync, container restart if needed, health checks). A `deploy.sh` failure is invisible to CI. + +The no-rebuild path fires on every commit that does not change Docker-relevant files (e.g., config changes, bot logic, backend logic). This is the majority of deploys. + +## Decision + +Bridge the async gap using GitHub commit statuses (`context: homelab-deploy`), posted by `deploy.sh` directly to the GitHub Statuses API: + +1. After a successful git sync, `deploy.sh` posts `pending` keyed to the post-sync HEAD SHA. +2. On exit (via an EXIT trap), `deploy.sh` posts `success` (happy path) or `failure` (any other exit). +3. CI polls `GET /repos/{owner}/{repo}/commits/{sha}/statuses` for the `homelab-deploy` context, using `GITHUB_TOKEN` (which has `statuses:read` via the `contents: read` job permission). + +**Graceful degradation:** If `GITHUB_DEPLOY_STATUS_TOKEN` is absent from the homelab server environment, `post_deploy_status` is a no-op. CI detects no status after ~90 s (6 attempts × 15 s) and falls back to the original HTTP 200 health check. This preserves current behavior with zero breakage while the token is being configured. + +## Alternatives Considered + +**Keep HTTP 200 poll in no-rebuild path:** Simple, zero new infrastructure. Rejected because it creates a false-pass guarantee — a `deploy.sh` git-sync failure or container restart failure exits the script with a non-zero code that is never surfaced to the operator. + +**Write deploy result to a file, have CI fetch it:** Requires the homelab server to expose a new endpoint or the CI to SSH into the homelab. Rejected — adds a network dependency and complicates the homelab surface. The GitHub Statuses API is already available and authenticated. + +**Poll the deploy log via SSH:** Requires SSH access from CI runners to the homelab. Rejected — increases attack surface and deviates from the existing webhook-only interface. + +**Add a callback webhook from deploy.sh to CI:** CI would need to receive inbound connections from the homelab. Rejected — GitHub Actions runners are ephemeral and do not have stable inbound addresses. + +## Consequences + +**Positive:** + +- A `deploy.sh` failure on the no-rebuild path now causes the CI step to exit 1 within seconds of the failure being posted — not silently pass. +- Operators see the failure in the CI run, not later from user reports. +- The status mechanism reuses existing GitHub infrastructure; no new services required. + +**Negative:** + +- Requires a new `GITHUB_DEPLOY_STATUS_TOKEN` secret configured on the homelab webhook container (GitHub PAT, `repo:statuses` scope on `LucasSantana-Dev/Lucky`). Until configured, behavior degrades gracefully to the old HTTP 200 poll. +- `deploy.sh` gains a `curl` call per deploy. The call is fire-and-forget (`|| true`); a network failure does not abort the deploy. + +**Neutral:** + +- The 90 s fallback threshold is a one-time cost paid only when the token is not yet configured. Once the token is present, the loop exits on `success` or `failure` — typically within 15–30 s of `deploy.sh` completing. + +## Revisit When + +- The `GITHUB_DEPLOY_STATUS_TOKEN` token is rotated or revoked without updating the homelab environment (status posts will silently stop; CI will fall back to HTTP 200 poll — same as before, but masked). Add a monitoring alert if status posts stop appearing on deploys. +- `deploy.sh` is split into multiple async stages; each stage would need its own status post. +- Discord-rate-limit or GitHub API outage causes the `gh api` status poll to fail consistently; the 20-attempt cap (5 min) bounds the worst case. diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 6e16e2071..3f931eee1 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -12,6 +12,11 @@ COMPOSE_PROJECT_NAME="${COMPOSE_PROJECT_NAME:-lucky}" SCRIPT_DIR="$(CDPATH= cd -- "$(dirname "${BASH_SOURCE[0]}")" && pwd)" export COMPOSE_PROJECT_NAME +GITHUB_DEPLOY_STATUS_TOKEN="${GITHUB_DEPLOY_STATUS_TOKEN:-}" +GITHUB_REPO="${GITHUB_REPO:-LucasSantana-Dev/Lucky}" +DEPLOY_FINAL_STATE="failure" +DEPLOY_FINAL_DESC="Deploy failed" +DEPLOYED_SHA="" log() { echo "$LOG_PREFIX $(date '+%H:%M:%S') $1"; } @@ -304,6 +309,17 @@ acquire_lock() { return 0 } +post_deploy_status() { + [[ -z "$GITHUB_DEPLOY_STATUS_TOKEN" ]] && return 0 + [[ -z "$DEPLOYED_SHA" ]] && return 0 + curl -s -o /dev/null \ + -X POST \ + -H "Authorization: token $GITHUB_DEPLOY_STATUS_TOKEN" \ + -H "Content-Type: application/json" \ + "https://api.github.com/repos/${GITHUB_REPO}/statuses/${DEPLOYED_SHA}" \ + -d "{\"state\":\"${1}\",\"description\":\"${2}\",\"context\":\"homelab-deploy\"}" || true +} + if [[ -z "$EXPECTED_SECRET" ]]; then log "ERROR: DEPLOY_WEBHOOK_SECRET not configured" exit 1 @@ -319,7 +335,8 @@ if ! acquire_lock; then notify 16711680 "Deploy Skipped" "Another deploy is already in progress" exit 1 fi -trap 'rm -rf "$LOCK_DIR" 2>/dev/null || true' EXIT +_on_exit() { rm -rf "$LOCK_DIR" 2>/dev/null || true; post_deploy_status "$DEPLOY_FINAL_STATE" "$DEPLOY_FINAL_DESC"; } +trap _on_exit EXIT COMPOSE_WORKDIR="$(resolve_compose_workdir)" CLOUDFLARED_CONFIG_DIR="$(resolve_cloudflared_config_dir)" @@ -348,6 +365,9 @@ if ! sync_checkout_to_origin_main; then exit 1 fi +DEPLOYED_SHA=$(git -C "$DEPLOY_DIR" rev-parse HEAD 2>/dev/null || true) +post_deploy_status "pending" "Deploy in progress" + # Rebuild webhook early: after git pull lands new hooks.json/Dockerfile but # BEFORE the long build/migrate/rollout phase. The -V flag renews anonymous # volumes so the COPY'd hooks.json from the Dockerfile takes effect. @@ -482,5 +502,7 @@ fi log "Pruning old images..." docker image prune -f --filter "until=24h" +DEPLOY_FINAL_STATE="success" +DEPLOY_FINAL_DESC="All services healthy" log "Deploy complete!" notify 65280 "Deploy Successful" "All services healthy and running" From da71fddf4c45e0a8354a28d079db09a08fc1f1eb Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Sun, 24 May 2026 20:39:30 -0300 Subject: [PATCH 3/4] fix(bot): healthcheck gateway readiness instead of redis tcp ping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit replaces the bot container's HEALTHCHECK with an HTTP GET against the existing /healthz endpoint on the metrics server (port 9091), which returns 200 only when client.isReady(). deploy.sh already fails the deploy on unhealthy containers, so no deploy-script changes are needed. start-period bumped 30s→45s to account for discord gateway login time. adds ADR docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md. --- Dockerfile | 9 ++- ...4-bot-docker-healthcheck-gateway-signal.md | 61 +++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md diff --git a/Dockerfile b/Dockerfile index 988616406..82f533b7c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -134,11 +134,10 @@ RUN mkdir -p downloads logs && \ USER bot -# Liveness via Redis TCP PING — confirms node can run AND the bot's -# Redis dependency is reachable. A wedged node process or broken Redis -# link both fail this; the old `console.log` check did neither. -HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \ - CMD node -e "const net=require('net');const s=net.createConnection({host:process.env.REDIS_HOST||'redis',port:+(process.env.REDIS_PORT||6379)},()=>s.write('*1\r\n\$4\r\nPING\r\n'));s.on('data',d=>process.exit(d.toString().startsWith('+PONG')?0:1));s.on('error',()=>process.exit(1));setTimeout(()=>process.exit(1),3000);" || exit 1 +# Gateway readiness via /healthz — returns 200 when client.isReady(), 503 otherwise. +# Covers Redis reachability implicitly (the bot cannot complete login without it). +HEALTHCHECK --interval=15s --timeout=5s --start-period=45s --retries=3 \ + CMD node -e "require('http').get('http://127.0.0.1:9091/healthz',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))" CMD ["sh", "-c", "npx prisma migrate deploy --config prisma/prisma.config.ts && node packages/bot/dist/index.js"] diff --git a/docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md b/docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md new file mode 100644 index 000000000..f536936fb --- /dev/null +++ b/docs/decisions/2026-05-24-bot-docker-healthcheck-gateway-signal.md @@ -0,0 +1,61 @@ +# ADR: Bot Docker HEALTHCHECK — Gateway Readiness over Redis TCP Ping + +**Date:** 2026-05-24 +**Status:** Accepted + +## Context + +The `production-bot` Dockerfile stage had a HEALTHCHECK that pinged the Redis TCP socket. This answered "can the bot process reach Redis?" but not "is the Discord gateway connected?" + +A bot container could be `healthy` under this check while: + +- Failing to authenticate with Discord (invalid token) +- Stuck in gateway reconnect backoff +- Blocked by a Discord API outage + +`deploy.sh` already checks for `unhealthy` Docker containers and fails the deploy. The HEALTHCHECK signal is therefore the correct place to surface bot gateway connectivity — no CI or deploy-script changes are needed. + +The bot's metrics server (`metricsServer.ts`) already exposes a `/healthz` route on `0.0.0.0:9091` that returns HTTP 200 when `client.isReady()` is true and 503 otherwise. + +## Decision + +Replace the Redis TCP ping in the bot's HEALTHCHECK with an HTTP GET against `localhost:9091/healthz`. Increase `start-period` from 30 s to 45 s to account for the additional time needed to complete Discord gateway login after process start. + +```dockerfile +HEALTHCHECK --interval=15s --timeout=5s --start-period=45s --retries=3 \ + CMD node -e "require('http').get('http://127.0.0.1:9091/healthz',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))" +``` + +`curl` is not available in the Alpine base image, so the probe uses the native Node.js `http` module. + +## Alternatives Considered + +**Keep Redis TCP ping:** Preserved fast startup health signals (~1 s). Rejected because Redis connectivity does not imply gateway connectivity — the two failure modes are different and independent. + +**Add a second HEALTHCHECK:** Docker only permits one HEALTHCHECK directive per stage. Not possible. + +**Add `botReady` field to backend `/api/health` via Redis flag:** Requires bot + backend changes + deploy-script probe update. Rejected: the gateway check is indirect (mediated by Redis), has a race condition at startup (flag absent before first `ready` event), and does not auto-fail the deploy — an operator must check the JSON manually. See rejected Option B in research notes. + +**`docker exec` introspection from deploy.sh:** Requires importing the full bot module graph; fragile and couples deploy logic to bot internals. Rejected. + +## Consequences + +**Positive:** + +- A bot container that is running but not connected to Discord becomes `unhealthy`, which fails the deploy automatically. No CI or deploy-script changes needed. +- The healthcheck implicitly covers Redis reachability — the bot cannot complete `client.login()` and become ready if Redis is unreachable (session state, queue, etc.). +- Faster failure signal: `interval=15s` means gateway disconnect is detected within 45 s (interval × retries). + +**Negative:** + +- `start-period=45s` delays the first health check pass. If the bot connects to Discord in under 10 s (typical), the container is marked healthy immediately; if Discord is slow (> 45 s), the first check fires while the client is still connecting and is counted as a failure. Three consecutive failures (135 s total) would mark the container `unhealthy`. This is acceptable for a deploy-time gate. + +**Neutral:** + +- The metrics server (`METRICS_DISABLED=true`) is a no-op in tests. The healthcheck fires in production containers only. + +## Revisit When + +- The bot's startup time consistently exceeds 45 s (increase `start-period`). +- Discord gateway latency at cold start grows beyond 30 s routinely — monitor with `HEALTHCHECK_GATEWAY_CONNECT_MS` metric once Prometheus scraping is wired. +- `metricsServer.ts`'s `/healthz` route is removed or the metrics server is disabled by default. From f03b7b07f5f59418559a4cb1320c5c5322673bd5 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Sun, 24 May 2026 20:54:33 -0300 Subject: [PATCH 4/4] fix(ci): post error commit status on deploy lock contention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a second webhook fires while a deploy is already running, deploy.sh exits immediately with lock contention. Previously, DEPLOYED_SHA was empty at that point so the EXIT trap's status post was silently dropped — CI polled for 5 minutes and printed "Deploy validation timed out" instead of an actionable signal. Now, a git fetch resolves the incoming SHA before the lock attempt. On contention, an 'error' status is posted for that SHA (context: homelab-deploy) so CI exits within seconds with a clear description ("Deploy skipped — another deploy in progress"). The fetch and status post are both best-effort: a network failure or absent GITHUB_DEPLOY_STATUS_TOKEN leaves behavior unchanged. ADR: docs/decisions/2026-05-24-deploy-lock-contention-signal.md --- ...026-05-24-deploy-lock-contention-signal.md | 74 +++++++++++++++++++ scripts/deploy.sh | 14 ++++ 2 files changed, 88 insertions(+) create mode 100644 docs/decisions/2026-05-24-deploy-lock-contention-signal.md diff --git a/docs/decisions/2026-05-24-deploy-lock-contention-signal.md b/docs/decisions/2026-05-24-deploy-lock-contention-signal.md new file mode 100644 index 000000000..1ce7fb176 --- /dev/null +++ b/docs/decisions/2026-05-24-deploy-lock-contention-signal.md @@ -0,0 +1,74 @@ +# ADR: Deploy Lock Contention — Fail Fast with Error Commit Status + +**Date:** 2026-05-24 +**Status:** Accepted + +## Context + +`deploy.sh` uses an atomic `mkdir` lock (`$LOCK_DIR`) to prevent concurrent deploys. When a second webhook fires while a deploy is already running, `acquire_lock` fails and `deploy.sh` exits 1 immediately. + +Before this change, the contention path had two problems: + +1. `DEPLOYED_SHA` is set only after `sync_checkout_to_origin_main` succeeds, so the EXIT trap's `post_deploy_status` call had no SHA to post against — the status was silently dropped. +2. CI polled the `homelab-deploy` commit status for the incoming SHA for up to 5 minutes (20 × 15s), found nothing, and printed "Deploy validation timed out" with exit 1 — giving a confusing signal rather than an actionable one. + +The second commit's deploy was silently dropped; an operator had to dig through deploy logs to understand why. + +## Decision + +Before the lock acquisition attempt, run `git fetch origin main` to determine the incoming SHA regardless of what the working tree contains. On contention, immediately post `state: error` keyed to that SHA via the GitHub Statuses API — then exit 1. + +```bash +incoming_sha="" +if git -C "$DEPLOY_DIR" fetch origin main 2>/dev/null; then + incoming_sha=$(git -C "$DEPLOY_DIR" rev-parse FETCH_HEAD 2>/dev/null || true) +fi + +if ! acquire_lock; then + if [[ -n "$GITHUB_DEPLOY_STATUS_TOKEN" && -n "$incoming_sha" ]]; then + curl -s -o /dev/null \ + -X POST \ + -H "Authorization: token $GITHUB_DEPLOY_STATUS_TOKEN" \ + -H "Content-Type: application/json" \ + "https://api.github.com/repos/${GITHUB_REPO}/statuses/${incoming_sha}" \ + -d '{"state":"error","description":"Deploy skipped — another deploy in progress","context":"homelab-deploy"}' || true + fi + notify 16711680 "Deploy Skipped" "Another deploy is already in progress" + exit 1 +fi +``` + +The `git fetch` result is silently swallowed if the network is unavailable — `incoming_sha` stays empty and the status post is skipped (same behavior as missing token). + +## Alternatives Considered + +**Keep exit-1 with no status post:** Zero change, CI continues to time out confusingly after 5 minutes. Rejected — the root cause (concurrent deploy) is immediately knowable; masking it costs operator time. + +**Post `failure` instead of `error`:** `failure` implies the deploy ran and the service is broken. `error` means the deploy could not start — the correct semantic per the GitHub Statuses API. Operator sees "error" in the deploy check and knows to look at lock contention, not service health. + +**Wait for the running deploy to finish, then re-trigger:** Requires a retry loop inside deploy.sh and a way to re-webhook the same SHA. Over-engineered for the frequency of occurrence. The operator can re-push or re-trigger if needed. + +**Extend EXIT trap to capture incoming SHA:** The EXIT trap fires after lock failure too, but `DEPLOYED_SHA` is empty at that point. Patching the trap to use `incoming_sha` would work but is more invasive and less readable than the targeted pre-lock block. + +## Consequences + +**Positive:** + +- CI exits immediately on `error` status instead of timing out after 5 minutes. +- The error description ("Deploy skipped — another deploy in progress") is surfaced directly in the CI step output via `gh api` polling. +- The operator can re-trigger the second deploy after the first finishes. + +**Negative:** + +- `git fetch origin main` adds one network call before every deploy, including the common non-contention path. The call is a `fetch` (read-only, ~100ms on LAN), not a full clone. A fetch failure is silently ignored and does not abort the deploy. +- If both `GITHUB_DEPLOY_STATUS_TOKEN` is absent and the fetch fails, the error is invisible to CI — same as before this change. The token must be configured for full benefit. + +**Neutral:** + +- The same `homelab-deploy` context string is used, so the error status appears on the same row as the success/failure status in the GitHub UI. + +## Revisit When + +- Rapid-fire deploys (>2 concurrent) become common — the lock pattern itself may need a queue rather than a drop. +- `git fetch` latency from homelab to GitHub increases consistently beyond ~2s (measure via deploy logs). +- `GITHUB_DEPLOY_STATUS_TOKEN` is rotated without updating the homelab environment — contention will revert to silent CI timeout until the token is refreshed. diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 3f931eee1..5f41dfb9c 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -330,8 +330,22 @@ if [[ "$RECEIVED_SECRET" != "$EXPECTED_SECRET" ]]; then exit 1 fi +incoming_sha="" +if git -C "$DEPLOY_DIR" fetch origin main 2>/dev/null; then + incoming_sha=$(git -C "$DEPLOY_DIR" rev-parse FETCH_HEAD 2>/dev/null || true) +fi + if ! acquire_lock; then log "ERROR: LOCK_CONTENTION (another deploy is already running)" + if [[ -n "$GITHUB_DEPLOY_STATUS_TOKEN" && -n "$incoming_sha" ]]; then + curl -s -o /dev/null \ + -X POST \ + -H "Authorization: token $GITHUB_DEPLOY_STATUS_TOKEN" \ + -H "Content-Type: application/json" \ + "https://api.github.com/repos/${GITHUB_REPO}/statuses/${incoming_sha}" \ + -d '{"state":"error","description":"Deploy skipped — another deploy in progress","context":"homelab-deploy"}' || true + log "INFO: posted error status for ${incoming_sha}" + fi notify 16711680 "Deploy Skipped" "Another deploy is already in progress" exit 1 fi