From 58eda410571786385ddd0784d4f32060f22a518e Mon Sep 17 00:00:00 2001
From: ai <a@b>
Date: Mon, 1 Jun 2026 08:20:54 +0000
Subject: [PATCH] salvage: log and test stale payload cleanup

---
 AI.md                                |  2 +
 TODO.md                              |  9 ++---
 src/nwsalvage.c                      |  6 ++-
 tests/salvage/README.md              |  3 ++
 tests/salvage/salvage_smoke_suite.sh | 56 ++++++++++++++++++++++++++++
 5 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/AI.md b/AI.md
index 64e74a0..710042c 100644
--- a/AI.md
+++ b/AI.md
@@ -81,6 +81,8 @@ use, the current project status that the user pasted into the chat.
 - Scan must treat `.salvage` JSON as a sidecar for the matching `.recycle`
   payload.  If an external tool such as Samba or an administrator removes the
   payload, `87/16` must not return the stale sidecar and should remove the JSON.
+  The server log should contain a greppable line like
+  `WARN SALVAGE 87/16 STALE ...` for this cleanup.
 - Scan, recover, and purge should share the same scan/sequence/basehandle view
   so that a sequence returned by scan identifies the exact sidecar used later.
 - Append salvage endpoint tests to `tests/salvage/salvage_smoke_suite.sh` rather
diff --git a/TODO.md b/TODO.md
index 62738e1..e63e64d 100644
--- a/TODO.md
+++ b/TODO.md
@@ -256,12 +256,9 @@ Current status:
 - `NCP 0x2222 / 87 / 16` has a backend-backed scanner that reads `.salvage`
   JSON entries and returns one salvageable entry per call.  Stale JSON sidecars
   whose `.recycle` payload has disappeared are not returned and are removed by
-  the scan path.
-
-Follow-up:
-
-- Add explicit endpoint coverage for stale `.salvage` cleanup after manually or
-  externally removing the matching `.recycle` payload.
+  the scan path.  This cleanup is logged as `WARN SALVAGE 87/16 STALE ...` and
+  covered by the salvage smoke suite with a manual payload-removal pause plus
+  a grep of `/var/log/mars_nwe/nw.log`.
 
 ### AFP / Mac namespace backend
 
diff --git a/src/nwsalvage.c b/src/nwsalvage.c
index f276580..d40fcc6 100644
--- a/src/nwsalvage.c
+++ b/src/nwsalvage.c
@@ -1887,8 +1887,12 @@ static int nwsalvage_scan_metadata_dir(const char *metadata_dir,
      * NCP 87/16 scans; remove the orphaned sidecar and keep scanning.
      */
     if (stat(recycle_path, &st) < 0) {
-      if (errno == ENOENT)
+      if (errno == ENOENT) {
+        XDPRINTF((1, 0,
+                  "WARN SALVAGE 87/16 STALE msg=\"payload missing; removing sidecar\" sidecar=\"%s\" recycle=\"%s\" name=\"%s\"",
+                  metadata_path, recycle_path, entry.original_name));
         (void)unlink(metadata_path);
+      }
       continue;
     }
     if (!S_ISREG(st.st_mode))
diff --git a/tests/salvage/README.md b/tests/salvage/README.md
index 07e2166..818cc79 100644
--- a/tests/salvage/README.md
+++ b/tests/salvage/README.md
@@ -103,6 +103,9 @@ It verifies the basic `.recycle`/`.salvage` directory contract and stale JSON
 detection.  Runtime scans also treat `.salvage` JSON as a sidecar for the
 matching `.recycle` payload: if the payload is removed externally, the entry
 must disappear from `87/16` scan results and the stale JSON should be cleaned.
+The combined smoke suite includes a manual pause that asks the tester to remove
+the backend `.recycle` payload, then scans again and greps the server log for
+`WARN SALVAGE 87/16 STALE`.
 
 ## NCP path visibility
 
diff --git a/tests/salvage/salvage_smoke_suite.sh b/tests/salvage/salvage_smoke_suite.sh
index 84f30ae..f24909e 100755
--- a/tests/salvage/salvage_smoke_suite.sh
+++ b/tests/salvage/salvage_smoke_suite.sh
@@ -26,6 +26,7 @@ OUT_FILE=""
 KEEP_GOING=1
 CLEAN_ARTIFACTS=1
 CYCLE_COUNT=3
+SERVER_LOG_FILE="/var/log/mars_nwe/nw.log"
 
 usage() {
     cat <<USAGE
@@ -40,6 +41,7 @@ Options:
   --salvage-name NAME       Salvage metadata repository name (default: $SALVAGE_REPOSITORY)
   --out FILE                Write the complete report to FILE as well as stdout
   --cycles N                Number of NCP write/delete versions to create, 1..3 (default: 3)
+  --server-log FILE         Server log to grep for stale sidecar cleanup (default: $SERVER_LOG_FILE)
   --no-clean-artifacts      Accepted for compatibility; cleanup is done through NCP purge
   --stop-on-failure         Stop after the first failing check
   -h, --help                Show this help
@@ -66,6 +68,7 @@ while [ $# -gt 0 ]; do
         --salvage-name) SALVAGE_REPOSITORY=$2; shift 2 ;;
         --out) OUT_FILE=$2; shift 2 ;;
         --cycles) CYCLE_COUNT=$2; shift 2 ;;
+        --server-log) SERVER_LOG_FILE=$2; shift 2 ;;
         --no-clean-artifacts) CLEAN_ARTIFACTS=0; shift ;;
         --stop-on-failure) KEEP_GOING=0; shift ;;
         -h|--help) usage; exit 0 ;;
@@ -453,6 +456,56 @@ run_delete_capture_cycle() {
     emit "note=.recycle/.salvage are backend repositories; normal NCP reads are expected to fail for dot-directory paths"
 }
 
+
+run_stale_payload_cleanup_test() {
+    local before_count=0 after_count=0 status grep_status
+    local content="mars_nwe salvage stale payload cleanup"
+
+    section "stale payload cleanup: create salvage entry"
+    emit "payload=$content"
+    run_ncp_create_with_payload "stale payload cleanup: NCP create/write live file" "$content" || return 1
+    run_ncp_delete_only "stale payload cleanup: NCP delete live file" || return 1
+
+    count_salvage_entries "stale payload cleanup: NCP salvage scan before manual payload removal" before_count || return 1
+    if [ "$before_count" -lt 1 ]; then
+        fail_check "stale payload cleanup: expected one salvage entry before manual payload removal"
+        return 1
+    fi
+
+    section "stale payload cleanup: manual pause"
+    emit "Remove the recycle payload in another shell, then press Enter here."
+    emit "command=sudo rm -f '$FIRST_RECYCLE'"
+    emit "payload_backend_path=$FIRST_RECYCLE"
+    emit "metadata_backend_path=$FIRST_META"
+    if [ -r /dev/tty ]; then
+        printf 'Press Enter after removing %s ... ' "$FIRST_RECYCLE" > /dev/tty
+        read -r _mars_salvage_pause < /dev/tty
+    else
+        emit "warning=/dev/tty not readable; waiting on stdin for manual stale-payload confirmation"
+        read -r _mars_salvage_pause
+    fi
+
+    count_salvage_entries "stale payload cleanup: NCP salvage scan after manual payload removal" after_count || return 1
+    if [ "$after_count" -ne 0 ]; then
+        fail_check "stale payload cleanup: expected stale sidecar to be dropped, remaining=$after_count"
+        return 1
+    fi
+
+    section "stale payload cleanup: grep server log"
+    emit "log_file=$SERVER_LOG_FILE"
+    emit "grep_pattern=WARN SALVAGE 87/16 STALE"
+    emit "grep_sidecar=$FIRST_META"
+    emit "\$ grep -F 'WARN SALVAGE 87/16 STALE' '$SERVER_LOG_FILE' | grep -F '$FIRST_META' | tail -5"
+    grep -F 'WARN SALVAGE 87/16 STALE' "$SERVER_LOG_FILE" 2>&1 | grep -F "$FIRST_META" | tail -5 | report_tee
+    grep_status=${PIPESTATUS[1]}
+    emit "[exit=$grep_status]"
+    if [ "$grep_status" -ne 0 ]; then
+        fail_check "stale payload cleanup: expected WARN SALVAGE 87/16 STALE for $FIRST_META in $SERVER_LOG_FILE"
+        return 1
+    fi
+    return 0
+}
+
 run_version_capture_cycles() {
     local i label content count salvage_count=0
     i=0
@@ -490,6 +543,7 @@ emit "recycle_repository=$RECYCLE_REPOSITORY"
 emit "salvage_repository=$SALVAGE_REPOSITORY"
 emit "clean_artifacts=$CLEAN_ARTIFACTS"
 emit "cycles=$CYCLE_COUNT"
+emit "server_log=$SERVER_LOG_FILE"
 
 if prepare_paths; then
     emit "volume_root=$VOLUME_ROOT"
@@ -510,6 +564,8 @@ fi
 
 cleanup_live_file_if_present
 run_ncp_salvage_purge_all
+run_stale_payload_cleanup_test
+run_ncp_salvage_purge_all
 
 run_version_capture_cycles
 recover_first_salvage_entry "recover first version after $CYCLE_COUNT captures" "mars_nwe salvage payload cycle 1"