fix(ci): fail production deploys loudly when the environment is unconfigured (#1143)

[frankbria]

Closes #1143.

The decision, recorded

The issue asks for one of three: populate production, delete the job, or guard it. Option 3 — guard, do not delete.

Staging is the only deploy target today, and the operator's framing is explicit: "Nothing's live, there are no users. We're just trying to get to 'beta stage' on a staging server." But the repo carries open Launch: issues, so deleting deploy-production would only have to be rewritten. Populating production is an operator decision a code change cannot make. Failing loudly first is not.

Why the current state is worse than "it doesn't work"

production holds zero secrets while deploy-production references 24. GitHub resolves a missing environment secret to an empty string with no warning.

• Today: it fails five steps in as an SSH auth error — a symptom that says nothing about the cause.
• The trap: populate only the SSH secrets and the job proceeds, writing a remote .env containing AUTH_SECRET=. That is the JWT signing key.

What this adds

A preflight step that runs first — before checkout, before SSH — and refuses to continue unless all 15 genuinely-required secrets are present and non-blank (a secret set to " " counts as missing):

::error::The production environment is not configured.

Missing (or blank) secrets:
  - SSH_KEY
  - AUTH_SECRET
  ...

GitHub exposes environment secrets only to jobs declaring that environment, and
'production' currently holds none. Staging is the only working deploy target.
Set them with:
  gh secret set <NAME> --env production

Values are read from env, never interpolated into the run script — the rule #1130 established after finding that an expression inside a run: block executes on the runner.

Also drops environment.url: https://codeframe.example.com. A placeholder domain renders on the GitHub deployments page as a live link to a target that has never existed — the same class of thing as the stale staging URL fixed in #1145.

The exemption became conditional

tests/test_workflow_lint_wiring_1122.py carried KNOWN_GAPS = {("deploy.yml", "deploy-production")} — a bare exemption, which is a promise nobody checks. It is now GUARDED, and the exemption itself asserts the preflight step still exists. Delete the guard and the scanner re-fails.

Three new tests:

Test	What it prevents
the guard runs first	behind SSH setup it reports nothing the SSH failure had not already reported
it covers every secret it wires into its own env	a guard that omits one says "configured" while a blank AUTH_SECRET reaches the remote .env — worse than no guard
no fictional URL	the deployments page linking to a host that does not exist

Verified all three bite: removing AUTH_SECRET from the checked list fails the coverage test; renaming the step fails three tests including the original scanner.

Acceptance criteria

• A decision recorded on whether production deploys are a supported path — not currently; the job is kept and guarded, with the reasoning in the workflow itself
• AUTH_SECRET confirmed non-empty before any deploy writes the remote .env — twice now: the preflight, plus the job's existing fail-fast at the .env step
• The bare exemption in TestEnvironmentSecretsAreReachable is gone
• If kept: production holds every secret and one deploy is verified — deliberately not done. That is the operator action this PR cannot take, and it is exactly what the preflight now demands before a deploy can proceed

Testing

The production job cannot be exercised end to end for the same reason it needs this guard. What is verified: actionlint (with shellcheck) clean, the preflight's shell logic run standalone against set/blank/empty values, the three scanner tests proven to fail when the guard is removed, and the full backend suite (reported below).

[github-actions]

Claude finished @frankbria's task in 4m 26s —— View job

---

GLM precision review complete

Scope: concrete defects only (logic errors, security, data loss, contract breaks) in this PR's workflow + test changes.

• Gather PR diff and context
• Read deploy.yml and its callers/context
• Read test_workflow_lint_wiring_1122.py
• Verify preflight secret list vs. actual job usage (15 checked of 24 referenced; 9 excluded each have safe defaults)
• Verify preflight ordering and shell logic (set -euo pipefail, indirect expansion, whitespace-strip)
• Post summary

Result: ✅ No defects found. Summary posted as a separate comment on the PR. The preflight shell logic is correct under set -euo pipefail, secrets are read via env: (never interpolated into run:), no secret values are echoed, and the four new/changed tests are logically sound and fail on the regressions they target. The 15-of-24 secret coverage is a defensible design choice — every excluded secret has a safe default or is optional.

[claude]

Review

Solid, well-scoped fix. I read the full deploy.yml (not just the diff) and the updated test file, and checked the preflight's REQUIRED list against every secret actually consumed later in deploy-production.

What's good:

• Coverage is correct. All 15 secrets used later in the job for things that would either break the deploy or produce an unsafe result (SSH/Tailscale auth, HOST/USER/PROJECT_PATH, AUTH_SECRET, the health-check port, the NEXT_PUBLIC URLs, PM2 names) are in REQUIRED. The intentionally-optional ones (OPENAI_API_KEY, DATABASE_PATH, WORKSPACE_ROOT, API_HOST, logging/env flags) are correctly left out — they default or are blank-tolerant in the .env write, matching the staging job's existing behavior.
• No secret leakage. The failure path only ever echoes the names in $missing, never $value — good discipline given this step handles 15 secrets at once.
• Whitespace-only handling works. tr -d '[:space:]' before the -z check correctly treats a secret set to " " as missing, as claimed.
• ${!name:-} is safe under set -u. Indirect expansion combined with the :- default means an unset variable won't trip set -euo pipefail and abort with a raw "unbound variable" instead of the intended message — and since every REQUIRED name has a matching env: key, the variable is always at least an empty string anyway.
• Ordering is enforced, not just documented. test_the_production_guard_runs_before_anything_uses_a_secret pins it as steps[0], so a future edit that slots something above it (or moves it down) fails CI rather than silently reintroducing the "SSH failure five steps in" symptom.
• The GUARDED/_has_preflight change is the right fix for the thing it targets — a bare exemption set that nobody would notice going stale now self-checks that the preflight step still exists before granting the exemption.
• Dropping the codeframe.example.com placeholder url: and pinning it with a test is a good, low-risk cleanup consistent with the fix(deploy): point the staging deployment URL at the host that actually serves it #1145 precedent cited in the PR description.

Minor, non-blocking observations:

• test_the_production_guard_covers_every_secret_it_claims_to only checks one direction (every env: key is referenced in the script). It doesn't check the reverse — that every secrets.X used elsewhere in deploy-production (for things that should be required) is actually in REQUIRED. Today that's fine since I verified the list by hand, but there's nothing stopping a future PR from adding a new required secret to a later step (e.g. a new SSH or PM2 var) without anyone remembering to add it to the preflight — the job would then fail deep in SSH setup again, i.e. regress to exactly the symptom this PR fixes. A test that diffs "all secrets.* used in the job" against "REQUIRED ∪ a documented optional-allowlist" would close that gap, but it's a reasonable follow-up rather than something this PR needs to solve.
• The preflight step waits on needs: test like the rest of the job. Since it's a pure static config check with no code dependency, it could in principle run before/independent of the test suite for slightly faster failure feedback on a misconfigured environment. Not a real issue — it only matters on the (currently nonexistent) path where production is actually configured and someone iterates on this workflow directly, and keeping it inside the gated job is simpler.

No bugs, security issues, or test-coverage gaps that should block this. The exit-1 preflight logic was also validated standalone per the PR description, and the injection-safety pattern (values via env:, never interpolated into run:) correctly follows the #1130/#933 precedent already established in this file.

[github-actions]

✅ GLM precision review: no defects found.

Scoped to concrete defects only (logic errors, security, data loss, contract breaks). What was checked in deploy.yml and tests/test_workflow_lint_wiring_1122.py:

• Preflight shell logic — set -euo pipefail is safe: ${!name:-} indirect expansion is guarded by :- so set -u does not abort on unset/empty; the [ -z ... ] and [ -n "$missing" ] tests sit inside if conditions so set -e never fires on them; the multi-line REQUIRED string word-splits correctly into exactly 15 names; tr -d '[:space:]' correctly treats " " as missing.
• No secret leakage — only secret names are echoed; values are read into value= and consumed inside a command substitution that feeds [ -z ], never printed.
• Expression placement ([P2.34] actionlint's shellcheck findings are suppressed in workflow-lint — 44 pre-existing issues in run: blocks #1130/[P1.15] Harden deployment: quote the release tag in the production SSH heredoc and add security headers to the Caddy example #933) — all ${{ secrets.* }} references are in the env: block (data), none interpolated into run: (source). No injection surface.
• 15-vs-24 secret coverage — verified all 24 referenced secrets; the 9 not in REQUIRED (WORKSPACE_ROOT, OPENAI_API_KEY, DATABASE_PATH, API_HOST, LOG_LEVEL/LOG_FILE, ENVIRONMENT/DEBUG/HOT_RELOAD) each carry a safe default (e.g. WORKSPACE_ROOT="${ENV_WORKSPACE_ROOT:-${REMOTE_PATH}/workspaces}") or are genuinely optional. No concrete failure scenario where the deploy breaks unsafely.
• Test logic — _has_preflight, the regex/env cross-check, the first-step ordering assertion, and the example.com URL assertion are all logically correct and would fail on the regressions they target.

View job run

[frankbria]

Checks: all 14 green. Full backend suite 6481 passed, 49 skipped (507s), ruff clean, actionlint (shellcheck on) clean.

Third-party review (codex review vs main): no findings.

The workflow preflight guard is placed before secret-consuming steps, checks the required production deployment secrets, and the added tests cover the intended wiring.

Demo of the guard's own logic, run standalone against set / blank / empty values — the whitespace case is the one a naive -z check misses:

$ A="ok"; B=" "; C=""   # then the preflight's loop verbatim
missing: B C

The production job itself cannot be run end to end — that is the condition this guard exists to report, and it is now what the job says instead of an SSH auth error.