From d1251bf238af48f3b890a8ec94714a097951db62 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 18 Jul 2026 18:11:19 +0000
Subject: [PATCH] Final polish: explicit LOCK_FD close, tar --list SIGPIPE
 comment, marker timing comment, _extract_bash_function docstring

---
 app/sovran_systemsos_web/scripts/sovran-hub-backup.sh | 10 ++++++++++
 app/tests/test_manual_backup_workflow.py              | 11 +++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/sovran_systemsos_web/scripts/sovran-hub-backup.sh b/app/sovran_systemsos_web/scripts/sovran-hub-backup.sh
index 24dbe6e..3e7c05c 100755
--- a/app/sovran_systemsos_web/scripts/sovran-hub-backup.sh
+++ b/app/sovran_systemsos_web/scripts/sovran-hub-backup.sh
@@ -68,6 +68,9 @@ cleanup() {
     done
   fi
 
+  # Release the concurrency lock file descriptor if it was opened
+  [[ -n "${LOCK_FD:-}" ]] && exec {LOCK_FD}>&- 2>/dev/null || true
+
   if [[ "$LND_STOPPED" -eq 1 ]]; then
     log "Restarting previously active LND-related services…"
     for (( idx=${#LND_UNITS_TO_RESTART[@]}-1 ; idx>=0 ; idx-- )); do
@@ -500,6 +503,9 @@ _create_archive_impl() {
     fail "Archive $archive_name is empty after creation."
   fi
 
+  # Fast readability check: read only the first tar entry header (~512 bytes).
+  # head -1 closes the pipe after one line, sending SIGPIPE to tar which then
+  # exits early — so this is O(1) regardless of archive size.
   local spot_entry
   spot_entry="$(LC_ALL=C tar --list --file "$partial_path" 2>/dev/null | head -1 || true)"
   if [[ -z "$spot_entry" ]]; then
@@ -860,6 +866,10 @@ CHECKSUM_FILE="$BACKUP_DIR/SHA256SUMS.txt"
   echo "(/run/media/Second_Drive) and are reconstructable/internal-backup data."
   echo ""
   echo "Artifact listing:"
+  # INCOMPLETE exists at this point (created at backup start); BACKUP_COMPLETE does not
+  # exist yet (written after checksums). Excluding both marker files here is intentional:
+  # INCOMPLETE is excluded so it doesn't appear as a data artifact, and BACKUP_COMPLETE
+  # is excluded defensively for consistency should the ordering ever change.
   find "$BACKUP_DIR" -mindepth 1 -maxdepth 2 -type f \
     ! -name 'INCOMPLETE' ! -name 'BACKUP_COMPLETE' -print0 | sort -z | tr '\0' '\n'
 } > "$MANIFEST_FILE"
diff --git a/app/tests/test_manual_backup_workflow.py b/app/tests/test_manual_backup_workflow.py
index f5c1dc8..cb3da74 100644
--- a/app/tests/test_manual_backup_workflow.py
+++ b/app/tests/test_manual_backup_workflow.py
@@ -17,8 +17,15 @@ NIX_HUB_FILE = REPO_ROOT / "modules" / "core" / "sovran-hub.nix"
 
 def _extract_bash_function(script_path: Path, func_name: str) -> str:
     """Extract a bash function definition from a shell script using Python regex.
-    Returns the complete function definition as a string that can be inlined into
-    test scripts, avoiding eval of awk output and the associated risks."""
+
+    The pattern matches from 'funcname() {' to the first line whose only content
+    is '}' (the function closing brace, at column 0). This works correctly for
+    functions whose control structures (case/if/while/for) use indented braces,
+    because only the function's own closing brace sits at column 0. It would not
+    correctly extract a function that contains a nested function definition.
+
+    Returns the complete function definition, safe to inline into a test bash
+    script without eval or awk."""
     source = script_path.read_text()
     pattern = r"^" + re.escape(func_name) + r"\(\) \{.*?^}"
     match = re.search(pattern, source, re.MULTILINE | re.DOTALL)