HA: make command recovery restart-safe

[ankitgoswami]

Reviewable diff: +649/-505 across 18 files (excludes generated, test, and story files).

Summary

Fleet now recovers command processing safely after an active-instance restart. Commands that never left the queue remain eligible for execution, while commands interrupted after dispatch began are failed because their device outcome is unknown. Ownership loss ends the active runtime instead of attempting an in-process demotion and reacquisition.

Standalone Fleet keeps its existing admission and graceful-shutdown behavior.

Stack

• #874 - make command recovery restart-safe (current)
• #875 - wire the opt-in production runtime

#875 is stacked directly on this PR and adds the production configuration that selects HA or standalone mode. This PR defines command recovery and active-runtime lifecycle behavior without changing how deployments enable HA.

How it works

Patroni selects the writable PostgreSQL server and publishes it through etcd. Fleet separately verifies that writer and acquires its database lease before it starts active-only work.

On startup or failover:

1. The coordinator verifies the current database writer and acquires the Fleet lease.
2. It renews the lease once more after the closing writer proof, then publishes the active lifetime.
3. Command execution starts by running the existing reaper once before workers start.
4. The reaper preserves PENDING rows because no device attempt has started. It changes interrupted PROCESSING rows to FAILED with an unknown-outcome reason, then finishes affected terminal batches in bounded pages.
5. Active jobs start and the request gate opens.
6. The normal periodic reaper continues handling stale PROCESSING work.

On ownership loss, the coordinator cancels the active lifetime and returns a fatal error. The runtime closes admission, aborts active jobs, and returns that error to fleetd. The deployment supervisor can then restart Fleet as a fresh passive candidate.

Command writes use expected-state updates such as PROCESSING to SUCCESS. A late result therefore affects zero rows after recovery has already failed that command and is treated as a benign lost race.

Command enqueue uses one bounded transaction and a bulk insert. If the client sees an ambiguous enqueue error, Fleet checks whether the transaction committed and only fails the batch when it can prove no queue rows exist.

Recovered DownloadLogs batches no longer depend on the callback from the old process. Once the caller's organization-scoped batch is finished, the download endpoint lazily creates the missing log bundle from persisted results. First-time creation is serialized so concurrent requests reuse one complete bundle.

flowchart LR
    DCS["etcd publishes Patroni primary"] --> OBS["Fleet verifies writer"]
    PG["Connected PostgreSQL identity"] --> OBS
    OBS --> LEASE["Acquire and renew Fleet lease"]
    LEASE --> REAP["Run startup reaper"]
    REAP --> PENDING["Keep PENDING queued"]
    REAP --> PROCESSING["Fail interrupted PROCESSING"]
    PENDING --> JOBS["Start active jobs"]
    PROCESSING --> JOBS
    JOBS --> GATE["Open request gate"]

Loading

sequenceDiagram
    participant Old as Former active Fleet
    participant Lease as Fleet lease
    participant New as Replacement Fleet
    participant DB as Command tables
    participant Supervisor as Service supervisor

    Old->>Lease: Renewal fails
    Old->>Old: Close admission and abort active jobs
    Old->>Supervisor: Return fatal ownership error
    New->>Lease: Acquire ownership
    New->>DB: Fail PROCESSING and preserve PENDING
    New->>New: Start jobs and open admission
    Supervisor->>Old: Restart as passive candidate

Loading

Areas of the code involved

Area / package / file	What changed	Why it matters for review
server/internal/ha/	Makes active ownership loss terminal, simplifies activation to a final lease renewal, and keeps standalone shutdown graceful	Review ownership lifecycle, cancellation, and the activation race handling
server/internal/domain/command/	Runs bounded recovery through the existing reaper before workers start and treats zero-row state transitions as lost races	Review the PENDING versus PROCESSING policy and result consistency
server/internal/infrastructure/queue/	Bulk-enqueues messages in one bounded transaction and reconciles ambiguous failures	Review atomicity and error classification
server/internal/infrastructure/files/, command service and handler	Authorizes bundles by organization and command type, then serializes lazy creation after process restart	Review tenant isolation, idempotency, and the finished-batch boundary
server/sqlc/queries/	Adds bounded reaper queries and expected-state transition predicates	Review row locking, page limits, and status guards
server/internal/infrastructure/db/, runtimepolicy/	Applies the shared five-second transaction bound to command writes	Review timeout scope and rollback behavior
server/generated/sqlc/ and generated mocks	Regenerated from SQL and interface sources	Generated, skip
HA, command, queue, and file tests	Covers lifecycle exit, restart recovery, enqueue ambiguity, state races, and lazy log bundles	Focus on recovery boundaries rather than helper-level assertions

Key technical decisions & trade-offs

• Preserve PENDING work instead of failing all queued work, because no device attempt has started.
• Fail PROCESSING work instead of replaying it, because Fleet cannot prove whether the device already acted.
• Reuse the command reaper at startup instead of adding takeover-only cleanup and coordinator sequencing.
• Exit after ownership loss instead of implementing graceful in-process demotion and reacquisition.
• Use local expected-state SQL guards instead of advisory locks or ownership columns on command rows.
• Authorize log bundles through the persisted organization and DownloadLogs type before accessing local files.
• Accept a failover edge case in per-device ordering. Failing an interrupted PROCESSING row can release a later command before an already-issued request from the former process is provably quiesced. Device-side fencing or a quarantine and reconciliation workflow is out of scope for this PR.
• Accept the rare crash window between batch creation and enqueue. A batch with no queue rows can remain pending after a hard process crash; closing it would require another recovery protocol for little operational value.
• Do not reconstruct process-local completion activity after restart. Persisted command history remains authoritative.

Testing & validation

• Focused HA, command, queue, file, runtime-job, handler, and fleetd tests passed.
• Hermit-backed server lint passed with zero issues.
• sqlc output and interface mocks were regenerated from their sources.
• Database-backed integration tests cover restart recovery, enqueue ambiguity, guarded transitions, cleanup paging, and cross-organization or wrong-type bundle rejection. They were not run locally because the available PostgreSQL credentials do not match the test harness; CI is the verification gate.
• A live multi-process Patroni and etcd failover is not covered here.

Post-Deploy Monitoring & Validation

• Logs: watch for active Fleet ownership ended, Interrupted by Fleet restart, command-service startup failures, and repeated Fleet process exits.
• Healthy signals: exactly one Fleet instance is active; PENDING counts fall after failover; interrupted PROCESSING rows fail once; replacement workers resume dispatch.
• Failure signals: both instances remain passive, restart loops continue, or command backlogs grow after failover.
• Mitigation trigger: if a replacement cannot become active or command processing does not resume, disable HA and run one known-good standalone instance while lease and writer state are inspected.
• Window and owner: the deploying operator should observe startup, one controlled failover, and the following 24 hours.

Follow-ups

• Add a chaos E2E that kills the primary mid-curtailment batch and asserts total time to shed stays under 180 seconds.
• Verify that the curtailment caller re-issues failed batches promptly rather than waiting on a slow poll.

[github-actions]

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

• Reviewed pull request diff only (6ccc451168b092bd97939179816f4d5316fda562...0911bf99bfb81197339cd0a1e90929775528e2d8, exact PR three-dot diff)
• Model: gpt-5.6-sol

💡 Click "edited" above to see previous reviews for this PR.

---

Review Summary

Overall Risk: HIGH

The restart recovery path can release per-device queue serialization while the prior device operation may still be running. Recovery also bypasses durable command-completion finalizers, leaving audit records incomplete after failover.

Findings

[HIGH] Restart reaping permits overlapping commands with unknown device operations

• Category: Concurrency
• Location: server/sqlc/queries/queue.sql:114
• Description: Startup sets include_fresh, immediately changing every PROCESSING message to FAILED without waiting for its command-specific timeout. Queue ordering only blocks on PENDING or PROCESSING, so subsequent commands for that device can then execute even though the previous plugin request or autonomous miner operation has an unknown outcome. This is especially unsafe for firmware installation, whose device-side work can continue after Fleet exits; the same transaction also resets UPDATING or REBOOT_REQUIRED to ACTIVE.
• Impact: Commands can execute concurrently or out of order after failover. A delayed pool or curtailment update can overwrite a newer setting, while another command issued during an ongoing firmware installation can leave a miner unavailable or bricked. The database state guard prevents the old worker from overwriting the queue result, but it does not fence external miner side effects.
• Recommendation: Keep unknown in-flight devices quarantined so later queue claims remain blocked until the old process is definitively fenced or command-specific recovery verifies the miner state. Add ownership/claim tokens and receiver-side idempotency where supported, and never reset firmware status to ACTIVE without probing the device outcome.

[MEDIUM] Recovery marks batches finished without running completion finalizers

• Category: Reliability
• Location: server/internal/domain/command/execution_service.go:355
• Description: Startup directly marks terminal batches FINISHED, but the activity completion callback exists only in the originating process's in-memory status goroutine. After a restart that goroutine is gone, and no recovery path recreates the idempotent *.completed activity event. The SQL finalizer can also move a still-PENDING batch to FINISHED without setting started_at.
• Impact: Failover can permanently omit completion outcomes for password, pool, firmware, and other commands from the audit trail, and recovered batch timestamps become inconsistent.
• Recommendation: Persist completion work in an outbox or run an idempotent startup finalizer that reconstructs completion activity before marking the batch finished. Set started_at = COALESCE(started_at, CURRENT_TIMESTAMP) for recovered pending batches.

Notes

The authoritative diff matched the stated commit range. No concrete authentication bypass, SQL injection, credential exposure, protobuf incompatibility, or pool-address substitution was found in the changed hunks. The added tests do not cover continued device-side execution during takeover or restoration of completion audit events after restart.

---

_{Generated by Codex Security Review |
Triggered by: @ankitgoswami |
Review workflow run}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff3e92f1f8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[mcharles-square]

Tradeoffs seems reasonable