fix: harden production deployment gates

[protostatis]

Summary

• create a coherent online SQLite/state recovery set and fail closed on missing production configuration
• require an uncached OpenRouter candidate canary plus post-cutover heartbeat IDs, integrity, and exact source-weight publication checks
• preserve and verify rollback containers until all runtime/proxy gates pass, serialize releases, and promote GHCR latest only after a verified deployment

Verification

• pytest tests/ -q — 264 passed, 1 skipped
• ruff check crypto_sentiment_crawler/maintenance/deployment_checks.py tests/test_deployment_checks.py
• go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/deploy.yml
• parsed embedded SSH script with bash -n
• built the API Docker target and ran the deployment-check CLI from that image

[protostatis]

Sky's Code Review

This PR substantially hardens the production deployment workflow: it fail-closes on missing .env / empty database / empty orchestrator state, removes the automatic state-seeding path, adds an uncached OpenRouter embedding canary against the candidate crawler image, introduces a coherent SQLite+JSON backup/restore pair with publication-integrity verification, extends rollback protection until runtime heartbeats and Nginx proxy checks pass, serializes releases via a concurrency group, and defers GHCR 'latest' promotion to a separate post-deploy job. The new deployment_checks.py module is well-structured, NULL-safe in its SQL comparisons, uses SQLite's backup API correctly, and has reasonable test coverage. The dominant concern is one concrete correctness bug: the new promote-latest job reads needs.build-and-push.outputs.version, but the build-and-push job in this diff declares no outputs.version mapping, so VERSION will be empty and the image promotion will target an empty ref. There are also several shell/robustness nits worth addressing.

Verdict: Changes requested

Comments

• The rollback hardening is well-thought-through: RESTORE_ORDER enforces deterministic restart ordering, rollback now verifies each service is actually running after restore, and a CRITICAL operator alert is emitted when rollback is incomplete. This is a meaningful reliability improvement.
• The deployment_checks.py SQL uses IS NOT for all NULL-aware comparisons and the backup path uses SQLite's online backup API plus PRAGMA quick_check, which is the correct approach for a live WAL database. Good.
• The concurrency group (${{ github.repository }}-production-deploy with cancel-in-progress: false) correctly serializes releases; combined with the deferred 'latest' promotion this is a sound release-safety model.
• Consider pinning the OpenRouter canary's expected model/dimensions and the heartbeat component list in a single source (settings or constants) so the shell script and deployment_checks.py cannot drift out of sync; today the components default to price/crawl/belief_update only in Python while the shell passes no --component flags.

---

Reviewed by Sky — Unchained Sky engineering agent

Inline Comments (could not attach to lines)

.github/workflows/deploy.yml:693 — VERSION is read from needs.build-and-push.outputs.version, but the build-and-push job does not declare an outputs: block (and the diff shows no id: meta / outputs: version: ${{ steps.meta.outputs.version }}). With no such output, $VERSION is empty and docker buildx imagetools create --tag "$IMAGE:latest" "$IMAGE:" will construct an empty/incorrect source ref and fail. Add outputs: version: ${{ steps.meta.outputs.version }} to build-and-push (or reference the metadata step output directly) before relying on it here.

.github/workflows/deploy.yml:451 — trap 'exit 130' INT TERM replaces the rollback handler for SIGINT/SIGTERM. It does still trigger rollback via the EXIT trap (because exit 130 fires the EXIT trap), but on SIGINT/SIGTERM the eventual exit code is fixed at 130 rather than the real failure code, and rollback_on_exit's exit_code=$? will observe 130/0 instead of the interrupted command's status. Prefer leaving the EXIT trap as the single rollback path and not shadowing INT/TERM, or capture the real status explicitly.

.github/workflows/deploy.yml:471 — CANDIDATE_STARTED_AT is built with date -u +%Y-%m-%dT%H:%M:%S+00:00, where +00:00 is a literal suffix. This happens to be correct only because -u forces UTC; if anyone later drops -u the timestamp becomes silently wrong and post-cutover heartbeat comparisons (success_at <= since) will misbehave. Prefer date -u +%Y-%m-%dT%H:%M:%SZ or include a real %z offset so the offset and the clock source cannot drift apart.

crypto_sentiment_crawler/maintenance/deployment_checks.py:203 — get_heartbeat_watermark returns COALESCE(MAX(id), 0), so an empty pipeline_heartbeats table yields watermark 0 and _check_publication proceeds. That is reasonable, but note the runtime gate then only requires heartbeat_id > 0, which is satisfied by the very first heartbeat of any component. If you intend the gate to prove the candidate actually produced new output, consider gating on last_success_at > since (already done) AND confirming the watermark advanced past a non-zero prior value; a watermark of 0 makes the 'did not come from the candidate' check weaker on first-ever deploy.

.github/workflows/deploy.yml:230 — The OpenRouter canary runs docker run ... crypto-sentiment-crawler:current with --network crypto-sentiment_crypto-net, but at this point in the script the candidate images may not yet be loaded/tagged as :current (the visible diff does not show the load/tag step before this block). Confirm the :current tag is established before this canary runs, otherwise the canary will fail with 'image not found' and abort every deploy.

[protostatis]

Review follow-up:

• build-and-push.outputs.version is already declared at workflow lines 23–24 and maps to steps.meta.outputs.version; meta has id: meta. The promotion source ref is therefore populated.
• Each immutable release image is pulled and tagged locally as :current in the pull loop before the OpenRouter canary runs.
• Updated signal handling to preserve conventional INT/TERM statuses and changed the candidate timestamp to canonical UTC Z form in a6b5759.
• A zero heartbeat watermark still requires every candidate heartbeat timestamp to be strictly newer than candidate startup. Production bootstrap is also prohibited by the mandatory DB/state and existing rollback-container gates.

[protostatis]

Sky's Code Review

This PR meaningfully hardens the production deployment pipeline: it replaces ad-hoc backup/'seed-if-missing' logic with a coherent, verified SQLite+state recovery set; fail-closes when production config/data is missing; adds an uncached OpenRouter embedding canary; requires post-cutover heartbeat IDs + integrity + exact source-weight publication checks; preserves rollback containers until all runtime/proxy gates pass; serializes releases via a concurrency group; and promotes GHCR latest only in a separate post-verification job. The Python deployment-check CLI is well-structured, fail-closed, and well-tested (264 passing). The shell script's rollback path is substantially more robust than before (explicit restore order, post-restore verification, RESTORE_ORDER including belief-auto). The main risks are (1) the new promote-latest job depends on needs.build-and-push.outputs.version but no job-level outputs declaration is visible in the diff, which would cause the promote step to reference an empty version tag and break; and (2) the rollback trap returns $exit_code, a variable never assigned in the script, potentially masking the true failure exit status. A few smaller items (aggressive docker image prune -af, all-three-heartbeat strictness, cache_path=None assumption) are worth confirming.

Verdict: Changes requested

Comments

• Strong overall hardening; the fail-closed philosophy, coherent backup/restore set, and deferred latest promotion are all correct directions. The deployment_checks CLI is clean and well-tested.
• The runtime check's default components=("price", "crawl", "belief_update") requires ALL three to emit a fresh heartbeat (id > watermark and success time > candidate start) within the 12-minute window. If any of these jobs runs on a slower schedule than the others, a valid deployment could fail. Confirm the 12-minute/360-iteration budget comfortably exceeds the slowest job's cadence, or consider scoping the strict heartbeat gate separately from the source-weight publication gate.
• docker exec crypto-api python -m ... runtime (into the running candidate) vs docker run --rm ... crypto-sentiment-api:current python -m ... (fresh container) are both used for different checks — confirm deployment_checks is importable/entry-point-consistent in both the crawler (via uv run) and api (via plain python) images, since the two use different invocation styles.
• The backup step copies orchestrator_state.json with a plain cp (non-atomic); rely on the subsequent publication coherence check + retry loop to catch a torn copy, which is sufficient but means the retry loop is doing double duty as both race detection and atomicity guard — acceptable, just be aware a torn JSON read will surface as 'Invalid orchestrator state' and retry.

---

Reviewed by Sky — Unchained Sky engineering agent

Inline Comments (could not attach to lines)

.github/workflows/deploy.yml:707 — VERSION="${{ needs.build-and-push.outputs.version }}" references a job output that is not declared anywhere in this diff. The build-and-push job uses steps.meta.outputs.version for its tags but I don't see a job-level outputs: { version: ${{ steps.meta.outputs.version }} }. If that output isn't declared, $VERSION will be empty and docker buildx imagetools create --tag "$IMAGE:latest" "$IMAGE:$VERSION" will try to promote an empty/$IMAGE: tag and fail (or tag the wrong ref). Confirm the build job declares this output; otherwise the promote step is broken in production.

.github/workflows/deploy.yml:410 — return "$exit_code" in the rollback_on_exit trap handler references $exit_code, but I don't see it ever assigned anywhere in the visible script. With the EXIT trap firing on set -e failure, the actual failing status is in $? at trap entry, not $exit_code. An unset $exit_code makes return "$exit_code" return the status of the preceding echo (0), masking a failed deployment as exit 0. Capture exit_code=$? at the top of the handler (or use an ERR trap) so the workflow fails correctly.

.github/workflows/deploy.yml:630 — docker image prune -af runs while ROLLBACK_ACTIVE is still 1 and the *-previous rollback containers still exist (stopped). -a removes every image not referenced by a container; the stopped rollback containers still hold their old images so those survive, but this is still aggressive and can remove dangling/other images (e.g. base/cache images) before rollback is final. Consider moving prune after ROLLBACK_ACTIVE=0 + rollback-container removal, or drop -a.

crypto_sentiment_crawler/maintenance/deployment_checks.py:244 — OpenRouterEmbeddingProvider(..., cache_path=None, ...) assumes cache_path accepts None and that passing None actually disables caching (a real 'uncached' canary is the stated goal). Verify the provider treats None as 'no cache'; if it falls back to a default on-disk cache, the canary is not genuinely uncached and could pass on a stale/cached embedding.

[protostatis]

Second review follow-up:

• The job output is present at lines 23–24 (version: ${{ steps.meta.outputs.version }}), with id: meta at line 41.
• rollback_on_exit captures the original status immediately at line 395 (exit_code=$?) before any other command and returns it at line 448.
• The scheduler runs price, crawl, and belief_update as immediate startup jobs; the 12-minute gate is not waiting for their recurring intervals.
• The API target was built locally and the CLI was executed successfully via plain python; CI also built both API and crawler targets.
• cache_path=None disables both cache load and save (if not self.cache_path guards in the provider).
• Pruning remains while stopped rollback containers still reference and protect known-good images; moving prune -a after their removal would make those rollback images eligible for deletion.