fix(backup): drop local copies that are already off-site, and say why one stays

bin/hyperion-web/src/handlers/hostings.rs

@@ -2757,6 +2757,14 @@ async fn fetch_expiry(
     }
 }
 
+/// Does any listed backup keep its local copy only because the owning node
+/// has "Delete the local copy once it is verified off-site" switched off? The
+/// Archives card then says so once, above the table — "off-site ✓" next to a
+/// local path otherwise reads as a delete that silently failed.
+pub(crate) fn backups_kept_by_setting(backups: &[hyperion_types::BackupRunWire]) -> bool {
+    backups.iter().any(|b| b.local_kept == "setting_off")
+}
+
 async fn fetch_backup_list(
     state: &SharedState,
     target: Option<&str>,
@@ -4895,13 +4903,35 @@ pub(crate) async fn run_offsite_drop_job(
                 r.kept_local,
                 if r.kept_local == 1 { "y" } else { "ies" },
             ));
+            if r.already_offsite > 0 {
+                log.push_str(&format!(
+                    "            ({} of those were already off-site: re-checked there and \
+                     dropped without being uploaded again)\n",
+                    r.already_offsite
+                ));
+            }
             // Red when anything the operator selected FAILED — including ids
             // that no longer exist (a concurrent delete or a stale page): those
             // land in `failed` without touching `considered`, so a
             // `considered == 0` success clause would report green when every
             // selected item failed. Mirror the estate-wide sweep's predicate.
+            //
+            // And red when a copy was KEPT: the operator asked for the local
+            // copy to go, and a green job next to a backup that is still on
+            // disk reads as "done" when it is not.
             reporter.step("Finished.", 100, &log).await;
-            reporter.finish(r.failed == 0, None).await;
+            let kept = (r.kept_local > 0).then(|| {
+                format!(
+                    "{} local cop{} kept — not confirmed off-site; the Backups tab says why",
+                    r.kept_local,
+                    if r.kept_local == 1 {
+                        "y was"
+                    } else {
+                        "ies were"
+                    },
+                )
+            });
+            reporter.finish(r.failed == 0 && kept.is_none(), kept).await;
         }
         Ok(RpcResponse::Error(e)) => reporter.finish(false, Some(e.to_string())).await,
         Ok(_) => {

bin/hyperion-web/src/handlers/settings.rs

@@ -1833,12 +1833,46 @@ async fn propagate_notifications(
                 Some(node_id.as_str()),
                 Request::AgentConfigUpdate {
                     section: sect.to_string(),
-                    fields: f.into(),
+                    fields: f.clone().into(),
                 },
             )
             .await;
             match resp {
                 Ok(RpcResponse::AgentConfigUpdate) => Ok(()),
+                // An agent from before "keep the newest local copies" refuses
+                // the whole [backup] save over that one key — and with it the
+                // on/off switch it does understand. Resend without the key
+                // when leaving it out changes nothing on that node.
+                Ok(RpcResponse::Error(e))
+                    if backup_keep_unknown_to_old_node(&sect, &e.to_string()) =>
+                {
+                    match backup_fields_for_old_node(&f) {
+                        Some(f_old) => {
+                            match crate::dispatcher::dispatch_to_node(
+                                &st,
+                                Some(node_id.as_str()),
+                                Request::AgentConfigUpdate {
+                                    section: sect.to_string(),
+                                    fields: f_old.into(),
+                                },
+                            )
+                            .await
+                            {
+                                Ok(RpcResponse::AgentConfigUpdate) => Ok(()),
+                                Ok(RpcResponse::Error(e)) => {
+                                    Err(format!("{label} refused it ({e})"))
+                                }
+                                Ok(_) => Err(format!("{label} answered with something unexpected")),
+                                Err(e) => Err(format!("{label} could not be updated ({e})")),
+                            }
+                        }
+                        None => Err(format!(
+                            "{label} runs an older agent that cannot keep the newest local \
+                             copies, so it kept its previous setting — upgrade it, or set the \
+                             count to 0 to change dropping there"
+                        )),
+                    }
+                }
                 // An older agent that doesn't know these keys refuses the
                 // write; the operator needs to hear THAT, not "unreachable".
                 Ok(RpcResponse::Error(e)) => Err(format!("{label} refused it ({e})")),
@@ -1865,6 +1899,39 @@ async fn propagate_notifications(
     }
 }
 
+/// Did a node refuse a `[backup]` save only because it predates
+/// `keep_local_latest`? (Its config parser rejects unknown keys by name.)
+fn backup_keep_unknown_to_old_node(section: &str, error: &str) -> bool {
+    section == "backup" && error.contains("keep_local_latest")
+}
+
+/// The `[backup]` fields to send a node that predates `keep_local_latest`, or
+/// `None` when leaving the key out would change what that node does. An old
+/// node behaves as "keep 0", so the key can go when it says 0 (or nothing), or
+/// when dropping is being switched off. A keep count above 0 with dropping on
+/// must NOT be stripped: the old node would then drop the very copies the
+/// operator asked to keep.
+fn backup_fields_for_old_node(
+    fields: &std::collections::BTreeMap<String, String>,
+) -> Option<std::collections::BTreeMap<String, String>> {
+    let keep = fields
+        .get("keep_local_latest")
+        .map(|v| v.trim())
+        .unwrap_or("");
+    let drop_on = fields.get("drop_local_after_offsite").is_some_and(|v| {
+        matches!(
+            v.trim().to_ascii_lowercase().as_str(),
+            "true" | "on" | "1" | "yes"
+        )
+    });
+    if !(keep.is_empty() || keep == "0" || !drop_on) {
+        return None;
+    }
+    let mut f = fields.clone();
+    f.remove("keep_local_latest");
+    Some(f)
+}
+
 /// Operator-facing name for a config section.
 ///
 /// The redirect used to say "Section [backup_remote] saved", which is the name
@@ -1982,16 +2049,37 @@ pub async fn post_offsite_backfill(
                     "",
                 )
                 .await;
-            match hyperion_rpc_client::call(
+            // One blocking RPC that uploads and, with drop on, re-checks every
+            // site's local copies: heartbeat the job row so a long sweep is not
+            // reaped as stale (and its real result thrown away) mid-flight.
+            let hb = {
+                let reporter = reporter.clone();
+                tokio::spawn(async move {
+                    let mut pct = 5;
+                    loop {
+                        tokio::time::sleep(std::time::Duration::from_secs(120)).await;
+                        pct = (pct + 5).min(90);
+                        reporter
+                            .step(
+                                "Still working — copying off-site and re-checking local copies…",
+                                pct,
+                                "",
+                            )
+                            .await;
+                    }
+                })
+            };
+            let resp = hyperion_rpc_client::call(
                 &job_state.agent_socket,
                 Request::BackupOffsiteBackfill {
                     limit: BACKFILL_BATCH,
                     drop_local,
                     s3_targets,
                 },
             )
-            .await
-            {
+            .await;
+            hb.abort();
+            match resp {
                 Ok(RpcResponse::BackupOffsiteBackfill(r)) => {
                     let mut log = format!(
                         "considered: {}\npushed:     {}\nfailed:     {}\n",
@@ -2007,6 +2095,13 @@ pub async fn post_offsite_backfill(
                             r.kept_local,
                             if r.kept_local == 1 { "y" } else { "ies" },
                         ));
+                        if r.already_offsite > 0 {
+                            log.push_str(&format!(
+                                "            ({} of those were already off-site: re-checked \
+                                 there and dropped without being uploaded again)\n",
+                                r.already_offsite
+                            ));
+                        }
                     }
                     // Separated from `failed` because no retry fixes it: the
                     // archive was pruned off local disk before anything copied
@@ -2034,8 +2129,24 @@ pub async fn post_offsite_backfill(
                              continue.\n",
                         );
                     }
+                    if !r.note.is_empty() {
+                        log.push_str(&format!("\n{}\n", r.note));
+                    }
                     reporter.step("Finished.", 100, &log).await;
-                    reporter.finish(r.failed == 0, None).await;
+                    // A kept copy is not a failure of the sweep, but with
+                    // "delete local" asked for it is not done either — say so
+                    // rather than show green over copies still on disk.
+                    let kept =
+                        (drop_local && r.kept_local > 0).then(|| {
+                            format!(
+                            "{} local cop{} kept — not confirmed off-site; each site's Backups \
+                             tab says why",
+                            r.kept_local,
+                            if r.kept_local == 1 { "y was" } else { "ies were" },
+                        )
+                        });
+                    let kept = kept.or_else(|| (!r.note.is_empty()).then(|| r.note.clone()));
+                    reporter.finish(r.failed == 0 && kept.is_none(), kept).await;
                 }
                 Ok(RpcResponse::Error(e)) => reporter.finish(false, Some(e.to_string())).await,
                 Ok(_) => {
@@ -3321,6 +3432,48 @@ mod tests {
         flatten_toml_strings, letter_verdict, mask_secrets_in_toml,
         synthesize_unchecked_checkboxes, toml_key, toml_value,
     };
+
+    /// A node from before `keep_local_latest` gets the [backup] save without
+    /// it only when that cannot change what the node does.
+    #[test]
+    fn backup_fields_for_old_node_strips_only_when_harmless() {
+        use super::{backup_fields_for_old_node, backup_keep_unknown_to_old_node};
+        let m = |pairs: &[(&str, &str)]| -> std::collections::BTreeMap<String, String> {
+            pairs
+                .iter()
+                .map(|(k, v)| (k.to_string(), v.to_string()))
+                .collect()
+        };
+        assert!(backup_keep_unknown_to_old_node(
+            "backup",
+            "validation: field `keep_local_latest` is not editable in section `backup`"
+        ));
+        assert!(!backup_keep_unknown_to_old_node("backup", "something else"));
+        assert!(!backup_keep_unknown_to_old_node(
+            "snapshots",
+            "keep_local_latest"
+        ));
+
+        // keep 0 = what the old node does anyway → strip.
+        let f = backup_fields_for_old_node(&m(&[
+            ("drop_local_after_offsite", "true"),
+            ("keep_local_latest", "0"),
+        ]))
+        .expect("harmless");
+        assert_eq!(f, m(&[("drop_local_after_offsite", "true")]));
+        // Switching dropping OFF must reach it whatever the count says.
+        assert!(backup_fields_for_old_node(&m(&[
+            ("drop_local_after_offsite", "false"),
+            ("keep_local_latest", "3"),
+        ]))
+        .is_some());
+        // Dropping on with a keep count: stripping would drop the kept copies.
+        assert!(backup_fields_for_old_node(&m(&[
+            ("drop_local_after_offsite", "TRUE"),
+            ("keep_local_latest", "3"),
+        ]))
+        .is_none());
+    }
     use std::collections::BTreeMap;
 
     /// The fork guard must survive a `<textarea>`.

bin/hyperion-web/templates/hostings_detail.html

@@ -3283,7 +3283,8 @@ <h2>
       {% endif %}
     </div>
     <p class="note" style="margin-top:0.8rem">
-      How long archives and snapshots are kept, which kinds this node makes at all, and the FTP
+      How long archives and snapshots are kept, whether the local copy is deleted once it is
+      verified off-site, which kinds this node makes at all, and the FTP
       off-site target are set in <a href="/settings#backups">Settings → Backups &amp; trash</a>; S3 targets in
       <a href="/settings/backups">Settings → Backup targets</a>.
     </p>
@@ -3336,11 +3337,11 @@ <h2>
        delete form's. #}
     <button type="button" class="btn small" id="bulk-offsite-btn" disabled
             onclick="hyBulkOffsiteSubmit()"
-            title="Copy the selected backups off-site, then delete the local copies once verified">Off-site &amp; delete local</button>
+            title="Delete the local copies of the selected backups once each is confirmed off-site — uploading it first only if it is not there yet">Off-site &amp; delete local</button>
   </form>
   <form id="bulk-offsite-form" method="post" action="/hostings/backups/offsite-drop"
         data-confirm-title="Copy selected off-site, then delete local"
-        data-confirm-body="Each selected backup is uploaded to the configured off-site targets, then its local archive and database dump are deleted ONLY after an independent re-verify that the copy is really off-site. A copy that cannot be confirmed off-site is kept. The off-site copies remain and can still be restored. Runs as a background job."
+        data-confirm-body="A selected backup that is already off-site is re-checked there; any other is uploaded to the configured off-site targets first. Its local archive and database dump are then deleted ONLY after an independent re-verify that the copy is really off-site. A copy that cannot be confirmed off-site is kept. The off-site copies remain and can still be restored. Runs as a background job."
         data-confirm-confirm-label="Copy off-site &amp; delete local"
         data-confirm-variant="danger"
         style="display:none">
@@ -3419,6 +3420,16 @@ <h2>
      refresh re-renders only this region from the full page, and nothing in it
      is processed by htmx again afterwards: keep it to plain forms. #}
   <div id="backups-live" data-live-refresh="15">
+  {% if crate::handlers::hostings::backups_kept_by_setting(backups.as_slice()) %}
+    <p class="note" style="margin:0 0 0.6rem">
+      Backups already off-site (<em>uploaded</em> or <em>off-site ✓</em>) still keep their local copy
+      on this site's node, because <strong>Delete the local copy once it is verified off-site</strong> is
+      off there. Turn it on in
+      <a href="/settings#local-copies">Settings → Backups &amp; trash → Local backup copies</a> and this
+      site's copies are dropped after its next archive backup (scheduled, or <em>Back up now</em>) — or use
+      <em>Off-site &amp; drop</em> on a row now.
+    </p>
+  {% endif %}
   {% if backups.is_empty() %}
     <p class="note">No archives yet. <em>Back up now</em> makes the first one.</p>
   {% else %}
@@ -3448,10 +3459,16 @@ <h2>
                 back at the right size. An em dash means we have no record. -#}
             <td>
               <span class="{{ b.offsite_label().1 }}" style="font-size:0.6rem"
-                    title="{% if !b.remote_error.is_empty() %}{{ b.remote_error }}{% else if !b.remote_blob_key.is_empty() %}{{ b.remote_blob_key }}{% else %}No off-site copy is recorded for this backup{% endif %}">{{ b.offsite_label().0 }}</span>
+                    title="{% if !b.remote_error.is_empty() %}{{ b.remote_error }}{% else if !b.remote_blob_key.is_empty() %}{{ b.remote_blob_key }}{% else if !b.s3_ok_targets.is_empty() %}Pushed in full to S3: {{ b.s3_target_names() }}{% else %}No off-site copy is recorded for this backup{% endif %}">{{ b.offsite_label().0 }}</span>
               {%- if !b.sha256_hex.is_empty() %}
               <span class="cell-sub" style="font-size:0.6rem" title="Content digest recorded when the archive was made: {{ b.sha256_hex }}">fingerprinted</span>
               {%- endif %}
+              {#- Why the local copy is still here although it is off-site —
+                  so "off-site ✓" beside a local path never reads as a delete
+                  that silently failed. Reason from the OWNING node. -#}
+              {%- if !b.local_kept.is_empty() %}
+              <span class="cell-sub" style="font-size:0.6rem{% if b.local_kept == "unconfirmed" %};color:var(--warn){% endif %}" title="{{ b.local_note }}">{{ b.local_kept_label() }}</span>
+              {%- endif %}
             </td>
             <td class="right muted numeric">{{ crate::handlers::stats::fmt_bytes(b.bytes_total) }}</td>
             <td>
@@ -3496,13 +3513,13 @@ <h2>
                    leave the node. #}
                 <form method="post" action="/hostings/backups/offsite-drop" style="display:inline"
                       data-confirm-title="Copy off-site, then delete local"
-                      data-confirm-body="This backup is uploaded to the configured off-site targets, then its local archive{% if b.db_dump_path.is_some() %} and database dump{% endif %} are deleted ONLY after an independent re-verify that the copy is really off-site. If it cannot be confirmed off-site, the local copy is kept. Runs as a background job."
+                      data-confirm-body="{% if b.remote_state == "verified" || b.remote_state == "ok" || !b.s3_ok_targets.is_empty() %}This backup is already off-site: it is re-checked there (and uploaded again only if that fails){% else %}This backup is uploaded to the configured off-site targets{% endif %}, then its local archive{% if b.db_dump_path.is_some() %} and database dump{% endif %} are deleted ONLY after an independent re-verify that the copy is really off-site. If it cannot be confirmed off-site, the local copy is kept. Runs as a background job."
                       data-confirm-confirm-label="Copy off-site &amp; delete local"
                       data-confirm-variant="danger">
                   <input type="hidden" name="_csrf" value="{{ csrf_token }}">
                   <input type="hidden" name="selector" value="{{ detail.id.as_str() }}">
                   <input type="hidden" name="backup_ids" value="{{ b.id }}">
-                  <button class="btn small" type="submit" title="Copy off-site, then delete the local copy">Off-site &amp; drop</button>
+                  <button class="btn small" type="submit" title="Delete the local copy once it is confirmed off-site — uploading it first only if it is not there yet">Off-site &amp; drop</button>
                 </form>
                 {% endif %}
                 {% if b.state != "running" %}