From b3b8f34a133410aacf8924e87ac458a5b9f03f3a Mon Sep 17 00:00:00 2001
From: Alex <25013571+alexhb1@users.noreply.github.com>
Date: Fri, 10 Apr 2026 20:46:07 +0100
Subject: [PATCH] Fix JSON script blocking behavior + tests (#862)

Fixes #859
---
 docs/custom-scripts.md                        | 23 ++++++++++++-----
 .../download/postprocess/custom_script.py     | 25 ++++++++++++-------
 tests/core/test_download_processing.py        |  4 +++
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/docs/custom-scripts.md b/docs/custom-scripts.md
index b4c3e93..de78ad5 100644
--- a/docs/custom-scripts.md
+++ b/docs/custom-scripts.md
@@ -121,15 +121,26 @@ Example payload shape:
 }
 ```
 
-Example (bash + jq) (JSON payload must be enabled):
+Example (bash + python3) (JSON payload must be enabled):
 
 ```bash
 payload="$(cat)"
-mode="$(echo "$payload" | jq -r '.output.mode')"
-title="$(echo "$payload" | jq -r '.task.title')"
-final_paths="$(echo "$payload" | jq -r '.paths.final_paths[]')"
-echo "mode=$mode title=$title" >&2
-echo "$final_paths" >&2
+target="$1"
+PAYLOAD="$payload" TARGET="$target" python3 - <<'PY'
+import json
+import os
+import sys
+
+payload = json.loads(os.environ["PAYLOAD"])
+
+print(f"target={os.environ['TARGET']}", file=sys.stderr)
+print(
+    f"mode={payload['output']['mode']} title={payload['task']['title']}",
+    file=sys.stderr,
+)
+for path in payload["paths"]["final_paths"]:
+    print(path, file=sys.stderr)
+PY
 ```
 
 Example (Python) (works whether JSON payload is enabled or not):
diff --git a/shelfmark/download/postprocess/custom_script.py b/shelfmark/download/postprocess/custom_script.py
index bed80a6..673881f 100644
--- a/shelfmark/download/postprocess/custom_script.py
+++ b/shelfmark/download/postprocess/custom_script.py
@@ -124,19 +124,26 @@ def run_custom_script(
     )
 
     try:
+        run_kwargs: dict[str, Any] = {
+            "check": True,
+            "timeout": timeout_seconds,
+            "capture_output": True,
+            "text": True,
+            "cwd": cwd,
+        }
         # If we are not sending a JSON payload, close stdin so scripts that try
-        # to read it won't block indefinitely.
-        stdin = None if execution.payload_json is not None else subprocess.DEVNULL
+        # to read it won't block indefinitely. When we do send a payload, let
+        # subprocess.run manage stdin implicitly via `input=` to avoid passing
+        # both arguments at once.
+        if execution.payload_json is None:
+            run_kwargs["stdin"] = subprocess.DEVNULL
+        else:
+            run_kwargs["input"] = execution.payload_json
+
         result = run_blocking_io(
             subprocess.run,
             [execution.script_path, str(execution.target_arg)],
-            check=True,
-            timeout=timeout_seconds,
-            capture_output=True,
-            text=True,
-            cwd=cwd,
-            stdin=stdin,
-            input=execution.payload_json,
+            **run_kwargs,
         )
         if result.stdout:
             logger.debug("Task %s: custom script stdout: %s", task_id, result.stdout.strip())
diff --git a/tests/core/test_download_processing.py b/tests/core/test_download_processing.py
index b37a417..a0a7f1f 100644
--- a/tests/core/test_download_processing.py
+++ b/tests/core/test_download_processing.py
@@ -786,6 +786,8 @@ class TestCustomScriptExecution:
         call_args = mock_run.call_args
         result_path = Path(result)
         assert call_args[0][0] == ["/path/to/script.sh", str(result_path)]
+        assert call_args.kwargs["stdin"] is subprocess.DEVNULL
+        assert "input" not in call_args.kwargs
 
     def test_runs_custom_script_with_json_payload_on_stdin(self, temp_dirs, sample_direct_task):
         """Sends a JSON payload to the custom script via stdin when enabled."""
@@ -824,6 +826,7 @@ class TestCustomScriptExecution:
 
         payload_json = mock_run.call_args.kwargs.get("input")
         assert payload_json
+        assert "stdin" not in mock_run.call_args.kwargs
         payload = json.loads(payload_json)
         assert payload["version"] == 1
         assert payload["phase"] == "post_transfer"
@@ -879,6 +882,7 @@ class TestCustomScriptExecution:
 
         payload_json = mock_run.call_args.kwargs.get("input")
         assert payload_json
+        assert "stdin" not in mock_run.call_args.kwargs
         payload = json.loads(payload_json)
         assert payload["version"] == 1
         assert payload["phase"] == "post_upload"