Refactor worker recovery and harden candidate verification

- Replace the free-form manual recovery reason with structured
  ManualRecoveryKind holds and a unified /operator/recovery/action
  endpoint (removing the legacy continue endpoint)
- Make TargetRecoveryPlan non-optional and skip cleanup that already
  passed, automatically or via operator confirmation
- Heartbeat through restart cleanup so long reinstalls are not mistaken
  for a lost worker; guard coordinator_ready against locally-held jobs
- Treat a candidate update that reports a stale version as an
  infrastructure error instead of a pass
- Bump Rust toolchain to 1.99.1

Author: 3alpha

.github/workflows/main.yml

@@ -17,7 +17,7 @@ jobs:
     steps:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
       - name: Install pinned Rust toolchain
-        run: rustup toolchain install 1.96.1 --profile minimal --component clippy,rustfmt
+        run: rustup toolchain install 1.99.1 --profile minimal --component clippy,rustfmt
       - name: Check formatting
         run: cargo fmt --all -- --check
       - name: Lint

Dockerfile

@@ -1,4 +1,4 @@
-FROM rust:1.96.1-bookworm AS builder
+FROM rust:1.99.1-bookworm AS builder
 
 WORKDIR /usr/src/harness
 COPY Cargo.toml Cargo.lock rust-toolchain.toml ./

README.md

@@ -23,7 +23,7 @@ For every accepted job, the deterministic controller:
 5. captures normalized details and bounded, redacted log tails;
 6. upgrades the baseline in place to the candidate;
 7. repeats stabilization and capture, compares both observations, and performs optional Nexus analysis;
-8. restores a pre-existing target by removing it without deleting volumes and freshly installing the latest published release (`*`), or removes a target that was initially absent with `deleteVolumes: true`; cleanup fails when final inventory cannot be verified or contains unexpected dependencies.
+8. restores a pre-existing target by removing it without deleting volumes and freshly installing the latest published release (`*`), or removes a target that was initially absent with `deleteVolumes: true`; cleanup fails when final inventory cannot be verified or contains unexpected dependencies. A cleanup that already passed, automatically or through an operator, is never repeated.
 
 Nexus is advisory. With no key, the harness uses the heuristic analyzer only. With a key, it sends one bounded redacted request with no tools; Nexus failures fall back safely, and Nexus can never override a deterministic failure.
 
@@ -45,7 +45,7 @@ Use [`.env.example`](.env.example) for all timeout, Dappmanager, cleanup, and op
 
 ## Local fake mode
 
-Rust 1.96.1 is pinned by `rust-toolchain.toml`. The process reads a local `.env` if it exists; existing process variables take precedence.
+Rust 1.99.1 is pinned by `rust-toolchain.toml`. The process reads a local `.env` if it exists; existing process variables take precedence.
 
 ```bash
 mkdir -p .data
@@ -80,7 +80,7 @@ Every request carries `Authorization: Bearer <PACKAGE_HARNESS_WORKER_TOKEN>`, `C
 
 Claim input is validated before persistence or execution: `runId` is a bounded safe identifier, `source.repository` is an ASCII `owner/repository` name, `source.headSha` is a 7–64 character hexadecimal commit ID, and package names and references use the shared Dappnode parsers.
 
-Before mutating, the worker atomically persists the full claim and opaque claim token under `DATA_DIR`. It persists every recovery-relevant phase and, before network delivery, the exact serialized completion JSON. Transient failures and coordinator compatibility rejections retry the same bytes with capped backoff and worker-specific jitter, so a coordinator upgrade heals delivery without a worker restart. Successful delivery clears legacy delivery-recovery markers. The worker never claims another job until completion is acknowledged as `recorded` or `duplicate`.
+Before mutating, the worker atomically persists the full claim and opaque claim token under `DATA_DIR`. It persists every recovery-relevant phase and, before network delivery, the exact serialized completion JSON. Transient failures and coordinator compatibility rejections retry the same bytes with capped backoff and worker-specific jitter, so a coordinator upgrade heals delivery without a worker restart. An acknowledged delivery also clears a completion-conflict hold, so a restart that redelivers successfully needs no operator. The worker never claims another job until completion is acknowledged as `recorded` or `duplicate`.
 
 On restart, a pending completion is retried first. An interrupted job is never rerun: if it may have mutated the target, the worker inspects Dappmanager, performs bounded cleanup, then sends an `interrupted` worker-error completion. A lost claim is released locally after cleanup succeeds; if Tropibot still holds unresolved coordinator state, the next claim receives a conflict and polling stops without inventing new local recovery work. Manual recovery is reserved for failed cleanup, ambiguous local records, or a conflicting completion that proves Tropibot already holds different state for the claim.
 
@@ -92,15 +92,15 @@ After manually completing a failed target cleanup, acknowledge that exact run th
 curl --fail-with-body \
   --request POST \
   --header 'content-type: application/json' \
-  --data '{"runId":"gh-pr-177-f33e0a37375d-f0181f51d12f8734"}' \
-  http://HARNESS_HOST:8080/operator/recovery/continue
+  --data '{"runId":"gh-pr-177-f33e0a37375d-f0181f51d12f8734","action":"continueAfterCleanup"}' \
+  http://HARNESS_HOST:8080/operator/recovery/action
 ```
 
-The endpoint accepts only cleanup-related manual holds. It preserves the claim token and stored result, records the operator-verified cleanup, clears the persisted hold, and wakes the paused worker without a container restart.
+This action accepts only cleanup holds; the same endpoint takes `confirmCleanup`, `retryCompletion` and `acceptCoordinatorResult` for completion conflicts, as the dashboard does. It preserves the claim token and stored result, records the operator-verified cleanup, clears the persisted hold, and wakes the paused worker without a container restart.
 
 The dashboard's local worker control remains visible for completion conflicts. It shows whether cleanup is confirmed, a completion payload is persisted, the local claim is held, and Tropibot acknowledged a result. Once cleanup is confirmed, the operator can either retry the exact persisted completion or explicitly accept Tropibot's existing result and release the local claim. The latter is destructive to the saved delivery obligation and requires typing the exact run ID. The worker remains alive and paused while waiting for either action, so no package restart is needed.
 
-For planned package updates or host maintenance, choose **Drain worker** in the dashboard first. The lifecycle moves from `accepting` to `draining`; no new claim request is started, while an already claimed job is allowed to finish mutation, mandatory cleanup, and persisted completion delivery. Restart only after the dashboard reports `drained` and **Safe to restart**. **Resume accepting jobs** cancels or exits a drain. A claim response that crosses the drain boundary is completed safely because the v1 coordinator protocol has no claim-release operation. `POST /operator/worker/drain` and `POST /operator/worker/resume` expose the same controls and require an `application/json` body (`{}`). `/readyz` returns unavailable while draining or drained, but `/healthz` remains a process liveness check. SIGTERM also requests a drain, and the Compose service grants up to two hours for active safe work to finish.
+For planned package updates or host maintenance, choose **Drain worker** in the dashboard first. The lifecycle moves from `accepting` to `draining`; no new claim request is started, while an already claimed job is allowed to finish mutation, mandatory cleanup, and persisted completion delivery. Restart only after the dashboard reports `drained` and **Safe to restart**. **Resume accepting jobs** cancels or exits a drain. A claim response that crosses the drain boundary is completed safely because the v1 coordinator protocol has no claim-release operation. `POST /operator/worker/drain` and `POST /operator/worker/resume` expose the same controls and require an `application/json` body (`{}`). `/readyz` returns unavailable while draining or drained, but `/healthz` remains a process liveness check. SIGTERM also requests a drain, but Docker's default stop timeout of 10 seconds is far shorter than a package mutation, so drain first. A stop that interrupts a job is reconciled on the next start: the worker cleans up the target and reports the job as `interrupted` without rerunning it.
 
 Heartbeats run independently of package execution. A cancellation request is observed at safe phase boundaries and inside stabilization polling. It prevents a new mutating phase but never bypasses required cleanup. If Tropibot no longer recognizes the claim, the worker finishes the current operation, reconciles cleanup, and releases the obsolete local claim when the target is safe.
 

docker-compose.yml

@@ -6,8 +6,6 @@ services:
       dockerfile: Dockerfile
     image: package-harness.dnp.dappnode.eth:0.1.1
     restart: unless-stopped
-    # Planned drains normally finish before shutdown. If SIGTERM arrives first,
-    # keep Docker from killing an in-flight mutation or mandatory cleanup.
     environment:
       PORT: "8080"
       DATA_DIR: /data

docs/architecture.md

@@ -14,17 +14,17 @@ Tropibot  <-------------------------------------------->  Package harness
 
 `CoordinatorClient` owns the three normative worker endpoints and their DTOs. It adds bearer authentication and the package version user-agent, bounds request timeouts and response bytes, previews errors, and classifies retryable failures. Golden fixtures make the camelCase JSON contract explicit.
 
-Planned restart control is an in-process `accepting → draining → drained` state machine. Draining wakes an idle poll immediately and prevents another claim request. Work already claimed—including a response crossing the drain boundary—continues through cleanup and durable completion before the worker publishes `drained`. The supervision server stays available for status and resume operations; readiness is false while draining or drained. SIGTERM requests the same drain and stops future loop iterations, while Docker's stop grace period leaves enough time for long Dappmanager mutations and mandatory cleanup.
+Planned restart control is an in-process `accepting → draining → drained` state machine. Draining wakes an idle poll immediately and prevents another claim request. Work already claimed—including a response crossing the drain boundary—continues through cleanup and durable completion before the worker publishes `drained`. The supervision server stays available for status and resume operations; readiness is false while draining or drained. SIGTERM requests the same drain and stops future loop iterations. Docker's default 10-second stop timeout does not cover a long Dappmanager mutation, so a stop during a job relies on restart recovery; operators drain before restarting.
 
 `PackageHarnessWorker` persists a full claim before execution, then starts a heartbeat task beside the controller. The controller depends only on the narrow `RunProgress` port: it publishes its already-persisted phase and reads cancellation/claim-loss signals. No coordinator HTTP call runs in the controller or can block cleanup.
 
-`RunRecord.worker` holds the opaque claim token, mutation/cleanup flag, final worker error when no normal result exists, exact pending completion JSON, acknowledgement status, and a manual-recovery reason. The atomic file store writes a temporary file in `/data` and persists it atomically. Only one unacknowledged claimed record is allowed to proceed. Pending completion bytes take precedence over legacy delivery-related recovery markers during restart. Compatibility rejections retry automatically with capped backoff, and an acknowledgement clears any stale marker.
+`RunRecord.worker` holds the opaque claim token, mutation/cleanup flag, final worker error when no normal result exists, exact pending completion JSON, acknowledgement status, and any manual-recovery hold (its kind and reason). The atomic file store writes a temporary file in `/data` and persists it atomically. Only one unacknowledged claimed record is allowed to proceed. On restart, pending completion bytes are redelivered even under a completion-conflict hold. Compatibility rejections retry automatically with capped backoff, and an acknowledgement clears the hold.
 
-On startup the worker finds an unresolved record before it polls. It retries a pending completion first. If an active job might have changed Dappmanager, it checks the target, refuses the harness and core packages, and executes the persisted recovery plan: restore the exact test baseline when the target existed before the run, or remove a target that was absent before the run. It verifies the final inventory, then sends an `interrupted` worker error. It never reruns the interrupted test. Successfully reconciled claim loss releases the obsolete local claim and lets Tropibot decide whether another claim is allowed. Cleanup and completion-conflict holds keep the worker task alive but not ready. `POST /operator/recovery/continue` remains the compatible cleanup-only endpoint; the dashboard uses `POST /operator/recovery/action` to record verified cleanup, retry exact persisted completion bytes, or explicitly accept the coordinator's existing result and release local delivery state. Every action is scoped to an exact run, and conflict release is rejected until cleanup is confirmed. The claim loop also remains paused, rather than exiting, when Tropibot reports an unresolved lost job. The UI separately proxies Tropibot's worker-authenticated lost-job lookup and exact-job ready command through `GET /api/coordinator/lost-job` and `POST /operator/coordinator/ready`; the browser never receives the coordinator token. The latter always injects `cleanupConfirmed: true`, lets the operator choose whether Tropibot should requeue the package, and wakes polling only after Tropibot accepts the recovery. Ordinary delivery compatibility failures self-heal when Tropibot becomes compatible.
+On startup the worker finds an unresolved record before it polls. It retries a pending completion first. If an active job might have changed Dappmanager, it checks the target, refuses the harness and core packages, and executes the persisted recovery plan: reinstall the latest published release with its volumes preserved when the target existed before the run, or remove a target that was absent before the run. A cleanup that already passed, in the controller or by operator confirmation, is not repeated. Heartbeats run throughout restart cleanup and delivery, so a long reinstall is not mistaken for a lost worker. It verifies the final inventory, then sends an `interrupted` worker error. It never reruns the interrupted test. Successfully reconciled claim loss releases the obsolete local claim and lets Tropibot decide whether another claim is allowed. Cleanup and completion-conflict holds keep the worker task alive but not ready. `POST /operator/recovery/action` continues after verified cleanup, records verified cleanup, retry exact persisted completion bytes, or explicitly accept the coordinator's existing result and release local delivery state. Every action is scoped to an exact run, and conflict release is rejected until cleanup is confirmed. The claim loop also remains paused, rather than exiting, when Tropibot reports an unresolved lost job. The UI separately proxies Tropibot's worker-authenticated lost-job lookup and exact-job ready command through `GET /api/coordinator/lost-job` and `POST /operator/coordinator/ready`; the browser never receives the coordinator token. The latter always injects `cleanupConfirmed: true`, lets the operator choose whether Tropibot should requeue the package, and wakes polling only after Tropibot accepts the recovery. It refuses a job this worker still holds locally, because that worker is recovering rather than lost. Ordinary delivery compatibility failures self-heal when Tropibot becomes compatible.
 
-The controller’s baseline and candidate paths share stabilization and capture. It always resolves the requested baseline first; an omitted `baselineRef` means the latest published release and never means “whatever is installed.” An existing target is reused only when its observed version matches the resolved baseline, and cleanup restores that resolved baseline after candidate testing regardless of the pre-run version. If the target was absent before the run, cleanup removes it. Stabilization requires a non-empty, all-running container list with the same sorted names across consecutive samples. Failed detail calls, empty lists, non-running states, and container-set changes reset the streak. Evidence history, log collection, redaction, and analysis input are bounded.
+The controller’s baseline and candidate paths share stabilization and capture. It always resolves the requested baseline first; an omitted `baselineRef` means the latest published release and never means “whatever is installed.” An existing target is reused only when its observed version matches the resolved baseline, and cleanup reinstalls the latest published release after candidate testing regardless of the pre-run version. After each install or update, the package must report the version its preview resolved; a reported success that left another version running is an infrastructure error, never a pass. If the target was absent before the run, cleanup removes it. Stabilization requires a non-empty, all-running container list with the same sorted names across consecutive samples. Failed detail calls, empty lists, non-running states, and container-set changes reset the streak. Evidence history, log collection, redaction, and analysis input are bounded.
 
-The Dappmanager adapter uses separate read and mutation timeouts. Install, update, and removal calls retry only failures classified as transient transport or package-download errors, with bounded exponential backoff. Before reissuing a timed-out operation, it reconciles observable state so an installed package, reached version, or completed removal is accepted without a duplicate mutation. Authentication, validation, required-setup, and malformed-response failures are terminal. The baseline preview alone determines the version used for testing and recovery; the pre-run installed version is ignored. Cleanup verifies the final package inventory; failed verification or unexpected packages introduced by the run make cleanup fail rather than producing a clean result.
+The Dappmanager adapter uses separate read and mutation timeouts. Install, update, and removal calls retry only failures classified as transient transport or package-download errors, with bounded exponential backoff. Before reissuing a timed-out operation, it reconciles observable state so an installed package, reached version (compared with the version the preview resolves, since candidate refs are IPFS hashes), or completed removal is accepted without a duplicate mutation. When Dappmanager reports that an earlier attempt is still running, the adapter waits up to the mutation timeout for the requested state instead of assuming success. Authentication, validation, required-setup, and malformed-response failures are terminal. The baseline preview alone determines the version used for testing and recovery; the pre-run installed version is ignored. Cleanup verifies the final package inventory; failed verification or unexpected packages introduced by the run make cleanup fail rather than producing a clean result.
 
 Nexus receives only a single redacted, bounded request with no tools or conversation history. Its typed result is advisory; a failure falls back to the heuristic analyzer and no Nexus result can turn a deterministic failure into a pass.
 

rust-toolchain.toml

@@ -1,5 +1,5 @@
 [toolchain]
-channel = "1.96.1"
+channel = "1.99.1"
 components = ["clippy", "rustfmt"]
 profile = "minimal"