refactor(diagnostics): make error closer restartable (2/9)

[ankitgoswami]

Reviewable diff: +81/-9 across 2 files (excludes generated, test, and story files).

Summary

Diagnostics error closing is now explicitly activatable and restartable, so a passive Fleet can construct diagnostics without silently starting background work. Standalone fleetd still starts the closer during boot and drains it during shutdown.

Stack:

• Foundation: #780 (merged)
• Parallel domain refactors: diagnostics #781, IP scanning #782, scheduling #783, curtailment #785, command execution #787, and telemetry #786
• Orchestration: #784
• Catalog and fleetd cutover: #788

This PR is the diagnostics lifecycle slice and now targets main. Merged #780 supplies the shared runtimejobs.Lifecycle contract; the remaining domain and orchestration refactors are independent sibling PRs and can merge in any order. #788 is temporarily based on the integration branch so its reviewable diff contains only catalog/cutover work; after the siblings merge it will be rebased and retargeted to main. Passive-mode coordinator wiring, epoch fencing, and request gating remain later HA work.

How it works

NewService only constructs dependencies. fleetd calls Start explicitly, which creates one activation context and closer loop. The run identity-safely clears its own state before signaling completion, whether it ends through Stop or independent activation-context cancellation. Stop snapshots the run under the mutex, waits unlocked, and may return at the caller deadline while cleanup continues; restart is admitted after the prior run actually exits.

flowchart LR
    C["Construct diagnostics service"] --> S["fleetd calls Start"]
    S --> L["Error closer loop"]
    X["Shutdown or demotion"] --> T["Stop with deadline"]
    T --> L
    T --> R["Safe to start again"]

Loading

Areas of the code involved

Area / package / file	What changed	Why it matters for review
server/internal/domain/diagnostics/service.go	Removes constructor side effects and adds restartable lifecycle state	Passive construction must not start active work
server/cmd/fleetd/main.go	Explicitly starts and boundedly stops diagnostics	Preserves current standalone behavior with no between-PR gap
Diagnostics and error-query tests	Update construction and cover restart/error behavior	Verifies the lifecycle boundary without changing query behavior

Key technical decisions & trade-offs

• Start diagnostics explicitly in this PR rather than waiting for catalog cutover, avoiding a period where stale miner errors would stop auto-closing.
• Keep lifecycle ownership on the existing Service; no diagnostics-specific job wrapper is introduced.

Related

Related: #740

Testing & validation

• go test -short -race -count=1 ./internal/domain/diagnostics ./internal/handlers/errorquery ./cmd/fleetd
• Covers side-effect-free construction, retry after a closer error, activation-context cancellation, timed-out stop cleanup, and Start -> Stop -> Start.

Post-Deploy Monitoring & Validation

Watch diagnostics closer errors and verify stale miner errors continue to transition closed. Roll back if the closer stops running after startup or shutdown logs report a diagnostics drain failure.

---

[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 (4bf7e7a0e3c126f3cf358badbdd50fc12a4a0860...33fab91753ae7f5463160c5d3148ed97bb0ffc2f, exact PR three-dot diff)
• Model: gpt-5.5

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

---

Review Summary

Overall Risk: NONE

Findings

No security, correctness, or reliability findings were identified in the reviewed diff.

Notes

Reviewed .git/codex-review.diff as the authoritative scope. The changes are limited to making the diagnostics stale-error closer explicitly lifecycle-managed, starting it from fleetd, and updating tests for the new construction semantics.

---

_{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: 4386873c75

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

Refactors the diagnostics error closer so constructing the diagnostics service is side-effect free, and the closer can be explicitly started/stopped (restartable) via the shared runtimejobs.Lifecycle contract. This aligns diagnostics with the broader “explicit activation” direction for Fleet background work and preserves standalone fleetd behavior by starting the closer during boot and stopping it during shutdown.

Changes:

• Convert diagnostics closer startup from constructor side effect to explicit Start/Stop lifecycle methods.
• Update fleetd startup/shutdown to start diagnostics explicitly and stop it via the bounded standalone stop helper.
• Adjust diagnostics consumers/tests to use the new constructor signature and add lifecycle-oriented closer tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/internal/handlers/errorquery/handler_test.go	Updates diagnostics service construction to match new side-effect-free constructor.
server/internal/domain/diagnostics/watcher_test.go	Updates diagnostics service construction in watcher tests to the new signature.
server/internal/domain/diagnostics/service.go	Introduces restartable lifecycle state and implements runtimejobs.Lifecycle via Start/Stop.
server/internal/domain/diagnostics/service_test.go	Updates test helper to construct diagnostics service without passing a context.
server/internal/domain/diagnostics/closer_test.go	Reworks closer tests to validate explicit start/stop and restart behavior.
server/cmd/fleetd/main.go	Starts diagnostics closer explicitly at boot and stops it via stopStandaloneJob during shutdown.