Fix JSON script blocking behavior + tests (#862)

Fixes #859

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):

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())

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"