refactor(server): make background jobs restartable (1/2)

[ankitgoswami]

Reviewable diff: +921/-300 across 11 files (excludes generated, test, and story files).

Summary

Fleet's long-running server work now shares one restartable lifecycle contract. This gives the upcoming HA supervisor a safe boundary for stopping active work on demotion and starting it again on activation without rebuilding the process. Standalone fleetd behavior remains intact while the jobs are prepared for catalog-driven startup.

Stack:

1. This PR (1/2): runtime lifecycle foundation and service hardening
2. Follow-up PR B: declare the 12-job catalog and cut fleetd startup/shutdown over to the runtime group

Job selection, configuration filtering, passive-mode coordinator wiring, and epoch fencing are intentionally outside this PR. Catalog policy and construction land in PR B; coordinator behavior follows the catalog cutover.

How it works

Restartable background work implements Lifecycle: Start(context.Context) error activates it, while Stop(context.Context) error cancels and drains that activation within the caller's budget. A Job adds the validated runtime name, delegates that lifecycle, and itself implements Lifecycle; services do not need a second "named job" abstraction.

The runtime group receives an ordered list of jobs, validates unique names, and starts them in order. If startup fails partway through, it cancels the activation and rolls back the successfully started prefix in reverse order. Normal shutdown follows the same cancel-then-reverse-stop path. Rollback and shutdown each use one group-wide cleanup deadline, capped by any earlier caller deadline; incomplete cleanup permanently blocks reactivation so a new owner cannot overlap surviving work.

Command execution, telemetry, scheduling, IP scanning, diagnostics, MQTT ingest, alert metrics, and curtailment reconciliation now expose the same lifecycle shape and keep fresh cancellation/drain state per activation. Their stop paths prevent new work, wait for admitted goroutines, honor cancellation, and permit Start -> Stop -> Start after a clean drain. The diagnostics constructor is side-effect free, but fleetd explicitly starts the closer in this PR so stale errors continue to close before PR B lands.

Standalone shutdown gives jobs the existing 10-second graceful budget followed, when needed, by one additional 10-second drain window after forced cancellation. Teardown continues if a dependency still does not drain, so one stuck job cannot block process shutdown indefinitely; MQTT and diagnostics retain their already-bounded teardown behavior.

flowchart TB
    C["Catalog and scope selection (PR B)"] --> J["Validated named Job"]
    L["Concrete Lifecycle"] --> J
    J --> G["Runtime Group"]
    G --> S["Start jobs in order"]
    S --> A["Activation-scoped workers"]
    A --> X["Cancel activation"]
    X --> R["Stop jobs in reverse order"]
    R --> D["One group-wide cleanup deadline"]
    D --> OK["Stopped and restartable"]
    D --> F["Cleanup failure blocks reactivation"]

Loading

stateDiagram-v2
    [*] --> Stopped
    Stopped --> Starting: Start
    Starting --> Running: all jobs started
    Starting --> Stopped: rollback completed
    Starting --> Failed: rollback incomplete
    Running --> Stopped: stop completed
    Running --> Failed: stop incomplete
    Failed --> Failed: Start rejected with cleanup cause

Loading

Areas of the code involved

Area / package / file	What changed	Why it matters for review
server/internal/runtimejobs/ (new)	Shared Lifecycle, validated named Job, and ordered Group	Review lifecycle delegation, rollback, reverse stop, shared cleanup budget, and terminal failure behavior
server/internal/domain/command/, server/internal/testutil/	Construction is context-free; processor, reaper, and workers share activation cancellation and drain tracking	Prevents command work from overlapping a later activation
server/internal/domain/telemetry/	Pollers and writers restart cleanly while per-organization broadcasters remain process-owned	Separates activation shutdown from process teardown
server/internal/domain/schedule/	Cron state is rebuilt per activation and startup recovery honors cancellation	Ensures demotion can stop recovery and scheduled callbacks safely
server/internal/domain/ipscanner/	Per-run queues, cancellation-aware sends, and bounded worker drain	Prevents blocked scan results from hanging lifecycle transitions
server/internal/domain/curtailment/	Reconciler, MQTT ingest, and alert metrics implement the shared lifecycle	Makes control loops quiesce before another activation begins
server/internal/domain/diagnostics/	Constructor is side-effect free; the closer implements the shared lifecycle	Allows passive construction without silently starting work
server/cmd/fleetd/main.go	Explicit standalone lifecycle startup and shutdown	Preserves current operation until the catalog cutover

Key technical decisions & trade-offs

• Job owns its name and validation while implementing and delegating Lifecycle; concrete services remain unaware of runtime catalog naming.
• The group manages lifecycle only; construction, enablement, scope, and catalog state stay outside until PR B has a concrete consumer.
• One cleanup budget, capped by the caller's deadline, covers the whole group rather than resetting a timeout per job and making total shutdown grow with catalog size.
• Incomplete rollback or shutdown is terminal rather than risking two activations that own the same work.
• Telemetry broadcasters remain process-owned across activation stops and close only during process teardown.
• Standalone wiring keeps existing shutdown guarantees; the catalog and supervisor become the owner only in later PRs.

Related

Related: #740

Testing & validation

• go test -short -race -count=1 across runtimejobs, every hardened domain package, and cmd/fleetd
• go test -short -run '^$' ./... from server/ to compile every server package
• just _lint-server (golangci-lint: 0 issues)
• Coverage includes ordered start/reverse stop, cancellation-before-stop, partial-start rollback, caller cancellation and shorter deadlines, a shared cleanup deadline, terminal cleanup failure, stop timeout recovery, restart after clean drain, and concrete lifecycle conformance

Coordinator transitions, epoch fencing, passive request gating, and the final 12-job catalog are not exercised here; those remain PR B and HA follow-up work.

Post-Deploy Monitoring & Validation

No migration or configuration change is required. After deployment, verify that command execution, telemetry, scheduling, scanning, diagnostics, MQTT ingest, and curtailment loops start normally and that shutdown logs contain no runtime job exceeded shutdown timeout or failed to drain runtime job errors. Rollback is the previous server artifact; this PR introduces no persisted state.

[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 (fdeb6854a2cbda30e05c3744dcedeba8dd8a9dc2...50ec2128ec6b4e144068c87464e2a88afd012f94, exact PR three-dot diff)
• Model: gpt-5.5

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

---

Review Summary

Overall Risk: MEDIUM

Findings

[MEDIUM] Telemetry restart can permanently strand devices in in-flight state

• Category: Reliability
• Location: server/internal/domain/telemetry/service.go:587
• Description: The new restartable telemetry lifecycle clears only runCancel, runDone, and stopping when an activation drains. It leaves s.inFlight untouched across Stop/Start. Background status polling claims devices in s.inFlight before enqueueing them, and workers release those claims only after consuming the task. If Stop cancels the activation after a status task is queued but before a worker consumes it, the worker can exit through ctx.Done() and never delete the claim. After Start, status polling skips the device forever as already in flight, full telemetry workers requeue and skip it, and RefreshDevice can wait until timeout.
• Impact: Devices can stop receiving telemetry/status collection after a runtime Stop/Start cycle, especially during runtime demotion or any future use of the new restartable lifecycle. Failed/offline devices may never recover automatically because the recovery status poller keeps skipping the stale claim.
• Recommendation: Track ownership for in-flight claims by activation, or explicitly clear activation-owned claims when finishRun completes. Preserve request-owned RefreshDevice claims separately, for example by storing {kind, ownerRunID} instead of only inFlightKind, and on activation drain delete entries whose owner matches that run.

Notes

The reviewed diff is primarily lifecycle/shutdown work for background jobs. I did not find auth, SQL injection, command injection, protobuf wire-format, or cryptostealing/pool-hijack issues in the changed hunks.

---

_{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: 82ffdf3e04

ℹ️ 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".

[Copilot]

Pull request overview

This PR introduces a shared restartable lifecycle contract for long-running Fleet server background work, and migrates several domain services to implement that contract so they can be started/stopped safely across active/passive transitions (while preserving current standalone fleetd behavior).

Changes:

• Adds server/internal/runtimejobs with Lifecycle, validated named Job, and ordered Group start/stop orchestration (including rollback-on-partial-start and terminal failure semantics).
• Refactors multiple background services (command execution, telemetry, scheduling, IP scanning, diagnostics, curtailment loops) to support Start -> Stop -> Start with activation-scoped cancellation/drain tracking.
• Hardens fleetd shutdown behavior with a consistent “graceful budget then bounded drain” stop helper while keeping current startup wiring intact ahead of the catalog cutover in PR B.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
server/internal/testutil/service_provider.go	Updates execution service construction to new restartable lifecycle API.
server/internal/runtimejobs/group.go	Adds lifecycle orchestration primitives (Job/Group) for restartable background work.
server/internal/runtimejobs/group_test.go	Adds coverage for ordered start/reverse stop, rollback, deadlines, and terminal failure behavior.
server/internal/handlers/errorquery/handler_test.go	Updates diagnostics service construction API usage.
server/internal/domain/telemetry/service.go	Refactors telemetry service to implement restartable lifecycle with activation-scoped cancellation/drain.
server/internal/domain/telemetry/service_test.go	Adds/updates tests for telemetry restart behavior and removes timing-based flakiness in channel reads.
server/internal/domain/schedule/processor.go	Refactors scheduler processor lifecycle to support restartable start/stop with bounded drain semantics.
server/internal/domain/schedule/processor_test.go	Updates stop signature usage and adds tests for restartability and deadline-bounded stop.
server/internal/domain/ipscanner/service.go	Refactors IP scanner to use per-activation state and implement restartable lifecycle.
server/internal/domain/ipscanner/service_test.go	Adds tests for stop/start behavior, deadline handling, and cancellation-unblocking for worker drains.
server/internal/domain/ipscanner/integration_test.go	Updates stop signature usage for the new lifecycle.
server/internal/domain/diagnostics/watcher_test.go	Updates diagnostics service construction API usage.
server/internal/domain/diagnostics/service.go	Makes diagnostics construction side-effect free and adds explicit lifecycle start/stop for the closer goroutine.
server/internal/domain/diagnostics/service_test.go	Updates diagnostics service construction API usage.
server/internal/domain/diagnostics/closer_test.go	Updates closer tests for explicit Start/Stop and adds restart test coverage.
server/internal/domain/curtailment/reconciler/reconciler.go	Refactors reconciler to implement restartable lifecycle with activation ownership and deadline-bounded stop.
server/internal/domain/curtailment/reconciler/reconciler_test.go	Updates tests for new lifecycle and adds restart/timeout ownership scenarios.
server/internal/domain/curtailment/mqttingest/subscriber.go	Refactors subscriber to implement restartable lifecycle with “timed-out stop retains ownership” semantics.
server/internal/domain/curtailment/mqttingest/subscriber_test.go	Updates tests for new Stop signature and adds timeout ownership coverage.
server/internal/domain/curtailment/alert_metrics.go	Refactors alert metrics loop to implement restartable lifecycle with deadline-bounded stop.
server/internal/domain/curtailment/alert_metrics_test.go	Updates tests and adds coverage for activation cancellation + stop-timeout ownership.
server/internal/domain/command/zero_target_integration_test.go	Updates execution service construction API usage.
server/internal/domain/command/reaper_integration_test.go	Updates execution service construction API usage.
server/internal/domain/command/execution_service.go	Refactors command execution service to implement restartable lifecycle with activation-scoped cancellation/drain.
server/internal/domain/command/execution_service_test.go	Updates tests for new construction and adds restart/timeout/drain ownership coverage.
server/internal/domain/command/execution_service_credentials_test.go	Updates execution service construction API usage.
server/cmd/fleetd/main.go	Updates wiring for explicit diagnostics closer lifecycle and standardized shutdown for restartable jobs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b16a5f21c6

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50ec2128ec

ℹ️ 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".

[chatgpt-codex-connector]

Keep telemetry writers alive until workers drain

In the current per-activation implementation, these writer goroutines are canceled at the same time as the worker goroutines registered above, so during Stop a writer can take the ctx.Done() branch and exit while a miner RPC worker is still finishing. That worker can then enqueue its final status/metrics into the buffered per-run channel, but no writer remains and Stop still returns nil after the worker exits, silently dropping a completed poll during HA demotion/restart or shutdown. Keep the writers running until workers are done, or close/drain the per-run result channels after worker completion.

Useful? React with 👍 / 👎.

[ankitgoswami]

🤖 Superseded by the smaller runtime-jobs PR tree:

• refactor(server): define runtime job lifecycle (1/9) #780 lifecycle foundation
• refactor(diagnostics): make error closer restartable (2/9) #781 diagnostics
• refactor(ipscanner): isolate scanner activations (3/9) #782 IP scanner
• refactor(schedule): make processor restartable (4/9) #783 scheduler
• refactor(curtailment): make control loops restartable (5/9) #785 curtailment
• refactor(command): make execution restartable (6/9) #787 command execution
• refactor(telemetry): isolate activation results (7/9) #786 telemetry
• refactor(server): orchestrate runtime job groups (8/9) #784 Job and Group orchestration
• refactor(fleetd): start jobs from runtime catalog (9/9) #788 catalog and fleetd cutover

#788 is temporarily based on ankitg/runtimejobs-integration, which combines the parallel domain branches for the final cutover diff. The original branch is retained for reference.