Reject invalid chat transcripts and propagate browser cleanup failures

Address the two final whole-PR findings from the consolidated movie repair
brief. A fresh openai-chat response must provide a speech-bearing string
transcript even when local ASR is off; null must not reuse the no-transcript
sentinel belonging to deterministic engines or cached accepted audio.
Preserve the bounded candidate loop and rejected-byte evidence.

Report failed Windows tree termination as OSError so the recorder owner's
existing per-child handler continues all cleanup and retains failure metadata.
Card rendering checks its acquired browser handle before acting on the PID,
propagates wait failure, and reports locked-profile removal instead of
allowing a pending successful return to hide incomplete cleanup.

Add fake HTTP/process/filesystem boundary regressions for both findings,
including real adapter invocation, accepted chat cache reuse, ASR modes,
normal completed cards, and serve cleanup after a failed leader later exits.
Correct the rejected-chat fixture to use the chat engine. No media or native
browser execution was performed; Drew retains personal video acceptance.

Validation: expected RED failures retained; 40 focused tests and the single
75-test contracts entrypoint pass. Full evidence and self-review are in
.superpowers/sdd/2026-09-11-movie-committee-repairs/final-fix-report.md.

skills/proving-it-works-with-a-movie/scripts/browser_tools.py

@@ -71,7 +71,10 @@ def kill_process_tree(pid: int) -> None:
     or ttyd leaves helpers behind otherwise, and on Unix a pty child starts
     its own session, so a process group is not enough."""
     if sys.platform == "win32":
-        subprocess.run(["taskkill", "/T", "/F", "/PID", str(pid)], capture_output=True)
+        result = subprocess.run(["taskkill", "/T", "/F", "/PID", str(pid)], capture_output=True)
+        if result.returncode != 0:
+            detail = result.stderr.decode(errors="replace").strip()
+            raise OSError(f"taskkill failed for child {pid} (status {result.returncode}): {detail}")
         return
     for victim in reversed(_descendants(pid)):
         try:
@@ -89,7 +92,7 @@ def render_card(html: Path, png: Path, *, browser: str, width: int,
         raise FileNotFoundError(f"card HTML does not exist: {html}")
     png.parent.mkdir(parents=True, exist_ok=True)
     png.unlink(missing_ok=True)
-    with tempfile.TemporaryDirectory(prefix="movie-browser-", ignore_cleanup_errors=True) as profile:
+    with tempfile.TemporaryDirectory(prefix="movie-browser-") as profile:
         profile_path = Path(profile)
         log = profile_path / "browser.log"
         argv = [
@@ -120,8 +123,7 @@ def render_card(html: Path, png: Path, *, browser: str, width: int,
                 time.sleep(0.05)
             raise TimeoutError(f"Browser exceeded {timeout:g}s")
         finally:
-            kill_process_tree(process.pid)
-            try:
-                process.wait(timeout=5)
-            except subprocess.TimeoutExpired:
-                pass
+            # One-shot screenshot commands may exit normally once output is ready.
+            if process.poll() is None:
+                kill_process_tree(process.pid)
+            process.wait(timeout=5)

skills/proving-it-works-with-a-movie/scripts/narrate

@@ -345,8 +345,8 @@ def main():
                     continue
 
             # Preserve the engine transcript gate and the ASR drift thresholds.
-            if claimed is not None:
-                if not norm(claimed):
+            if engine == "openai-chat" and not cached:
+                if not isinstance(claimed, str) or not norm(claimed):
                     print(f"{sid}: chat transcript contains no speech", file=sys.stderr)
                     continue
                 drift_result = structural_drift(text, claimed)

tests/proving-it-works-with-a-movie/test_browser_contract.py

@@ -0,0 +1,112 @@
+"""Browser cleanup decisions with fake processes and a completed-output token."""
+import contextlib
+import os
+import subprocess
+import tempfile
+import unittest
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import patch
+
+import fixtures
+
+
+class CompletedOutput:
+    """Stand in for the completed-output observation without creating an image."""
+    def startswith(self, prefix):
+        return True
+
+    def endswith(self, suffix):
+        return True
+
+
+@contextlib.contextmanager
+def rendering(*, exited=False, taskkill_status=0, wait_timeout=False, locked_profile=False):
+    module = fixtures.load_script("browser_tools")
+    with tempfile.TemporaryDirectory() as temp, contextlib.ExitStack() as stack:
+        html, png = Path(temp).resolve() / "card.html", Path(temp).resolve() / "card.png"
+        html.write_text("<p>card</p>")
+        process = SimpleNamespace(pid=1100, returncode=0 if exited else None)
+        process.poll = lambda: process.returncode
+        calls, profiles = [], []
+        original_is_file, original_unlink = Path.is_file, os.unlink
+
+        def popen(argv, **kwargs):
+            profiles.append(Path(kwargs["cwd"]))
+            return process
+
+        def taskkill(argv, **kwargs):
+            calls.append(argv)
+            if taskkill_status == 0 and not wait_timeout:
+                process.returncode = -9
+            return subprocess.CompletedProcess(argv, taskkill_status, b"", b"termination failed")
+
+        def wait(timeout):
+            if process.returncode is None:
+                raise subprocess.TimeoutExpired("fake browser", timeout)
+            return process.returncode
+
+        def unlink(path, *args, **kwargs):
+            if locked_profile and Path(path).name == "browser.log":
+                raise PermissionError("locked browser profile")
+            return original_unlink(path, *args, **kwargs)
+
+        process.wait = wait
+        for obj, name, value in (
+            (module.sys, "platform", "win32"),
+            (module.subprocess, "Popen", popen),
+            (module.subprocess, "run", taskkill),
+            (Path, "is_file", lambda path: True if path == png else original_is_file(path)),
+            (Path, "read_bytes", lambda path: CompletedOutput()),
+            (os, "unlink", unlink),
+        ):
+            stack.enter_context(patch.object(obj, name, value))
+        try:
+            yield SimpleNamespace(render=lambda: module.render_card(html, png, browser="fake-browser", width=640, height=360),
+                                  process=process, calls=calls, profiles=profiles)
+        finally:
+            stack.close()
+            for profile in profiles:
+                if profile.exists():
+                    module.shutil.rmtree(profile)
+
+
+class BrowserCleanupContract(unittest.TestCase):
+    def test_windows_tree_termination_failure_reaches_caller_as_oserror(self):
+        module = fixtures.load_script("browser_tools")
+        with patch.object(module.sys, "platform", "win32"), \
+             patch.object(module.subprocess, "run", return_value=subprocess.CompletedProcess([], 1, b"", b"access denied")):
+            with self.assertRaises(OSError):
+                module.kill_process_tree(1100)
+
+    def test_completed_output_does_not_hide_tree_termination_failure(self):
+        with rendering(taskkill_status=1) as rig:
+            with self.assertRaises(OSError):
+                rig.render()
+
+    def test_completed_output_does_not_hide_owned_child_wait_timeout(self):
+        with rendering(wait_timeout=True) as rig:
+            with self.assertRaises(subprocess.TimeoutExpired):
+                rig.render()
+
+    def test_completed_output_does_not_hide_locked_profile(self):
+        with rendering(locked_profile=True) as rig:
+            with self.assertRaises(PermissionError):
+                rig.render()
+
+    def test_normally_exited_completed_card_succeeds_without_numeric_pid_cleanup(self):
+        with rendering(exited=True, taskkill_status=1) as rig:
+            self.assertIsNone(rig.render())
+            self.assertEqual(rig.calls, [])
+            self.assertFalse(rig.profiles[0].exists())
+
+    def test_completed_card_releases_live_browser_and_profile(self):
+        with rendering() as rig:
+            self.assertIsNone(rig.render())
+            self.assertEqual(rig.process.poll(), -9)
+            self.assertEqual(len(rig.calls), 1)
+            self.assertFalse(rig.profiles[0].exists())
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/proving-it-works-with-a-movie/test_narration_contract.py

@@ -1,3 +1,4 @@
+import base64
 import io
 import json
 import subprocess
@@ -11,6 +12,75 @@ from unittest.mock import patch
 import fixtures
 
 
+class ChatResponseContract(unittest.TestCase):
+    def test_malformed_transcripts_cannot_publish_candidates_in_any_asr_mode(self):
+        cases = ({}, {"transcript": None}, {"transcript": 42},
+                 {"transcript": ["Two", "words"]}, {"transcript": {}},
+                 {"transcript": False}, {"transcript": ""},
+                 {"transcript": "   "}, {"transcript": "...!?"})
+        for response in cases:
+            for mode in ("off", "auto", "on"):
+                with self.subTest(response=response, mode=mode), tempfile.TemporaryDirectory() as temp:
+                    module = fixtures.load_script("narrate")
+                    root = Path(temp)
+                    scenes, output = root / "scenes.json", root / "narration"
+                    scenes.write_text(json.dumps({"scenes": [{"id": "clip", "narration": "Two words"}]}))
+                    sentinels = []
+
+                    def post(*args, **kwargs):
+                        sentinel = f"NOT MEDIA: response {len(sentinels) + 1}".encode()
+                        sentinels.append(sentinel)
+                        audio = dict(response, data=base64.b64encode(sentinel).decode())
+                        return {"choices": [{"message": {"audio": audio}}]}
+
+                    argv = ["narrate", str(scenes), str(output), "--engine", "openai-chat", "--verify", mode]
+                    with patch.object(sys, "argv", argv), \
+                         patch.object(module.shutil, "which", return_value="fake-ffprobe"), \
+                         patch.object(module, "openai_key", return_value="fake-key"), \
+                         patch.object(module, "post", post), \
+                         patch.object(module, "duration", return_value=1.25), \
+                         patch.object(module, "transcribe_local", side_effect=AssertionError("invalid transcript reached ASR")), \
+                         redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()):
+                        try:
+                            result = module.main()
+                        except Exception as error:
+                            result = error
+                    self.assertEqual(result, 1)
+                    self.assertEqual(json.loads((output / "manifest.json").read_text()), [])
+                    self.assertEqual(len(sentinels), 2)
+                    self.assertEqual({path.read_bytes() for path in output.glob(".clip.attempt-*.wav")}, set(sentinels))
+                    self.assertFalse((output / "clip.wav").exists())
+
+    def test_valid_chat_and_cached_reuse_obey_independent_asr_modes(self):
+        for mode in ("off", "auto", "on"):
+            for heard in (None, "Two words"):
+                with self.subTest(mode=mode, heard=heard), tempfile.TemporaryDirectory() as temp:
+                    module = fixtures.load_script("narrate")
+                    root = Path(temp)
+                    scenes, output = root / "scenes.json", root / "narration"
+                    scenes.write_text(json.dumps({"scenes": [{"id": "clip", "narration": "Two words"}]}))
+                    sentinel = b"NOT MEDIA: accepted response"
+                    response = {"choices": [{"message": {"audio": {
+                        "data": base64.b64encode(sentinel).decode(), "transcript": "Two words",
+                    }}}]}
+                    argv = ["narrate", str(scenes), str(output), "--engine", "openai-chat", "--verify", "off"]
+                    with patch.object(sys, "argv", argv), \
+                         patch.object(module.shutil, "which", return_value="fake-ffprobe"), \
+                         patch.object(module, "openai_key", return_value="fake-key"), \
+                         patch.object(module, "post", return_value=response) as post, \
+                         patch.object(module, "duration", return_value=1.25), \
+                         patch.object(module, "transcribe_local", return_value=heard) as asr, \
+                         redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()):
+                        self.assertEqual(module.main(), 0)
+                        argv[-1] = mode
+                        expected = 1 if mode == "on" and heard is None else 0
+                        self.assertEqual(module.main(), expected)
+                    self.assertEqual(post.call_count, 1, "accepted cache must not synthesize again")
+                    self.assertEqual(asr.call_count, int(mode != "off"))
+                    self.assertEqual(bool(json.loads((output / "manifest.json").read_text())), expected == 0)
+                    self.assertEqual((output / "clip.wav").read_bytes(), sentinel)
+
+
 class NarrationPublicationContract(unittest.TestCase):
     def setUp(self):
         self.module = fixtures.load_script("narrate")
@@ -81,11 +151,12 @@ class NarrationPublicationContract(unittest.TestCase):
                   {"id": "raise", "narration": "Raise here"}]
         def accepted(text, wav, voice):
             wav.write_bytes(b"accepted")
+            return text
 
-        self.run_narrate(scenes, accepted)
-        first = self.run_narrate(scenes, synthesize, expected=1, extra_options=("--force",))
+        self.run_narrate(scenes, accepted, engine="openai-chat")
+        first = self.run_narrate(scenes, synthesize, engine="openai-chat", expected=1, extra_options=("--force",))
         attempts_after_rejection = len(attempts)
-        second = self.run_narrate(scenes, synthesize, expected=1)
+        second = self.run_narrate(scenes, synthesize, engine="openai-chat", expected=1)
 
         self.assertEqual(first, [])
         self.assertEqual(second, [])

tests/proving-it-works-with-a-movie/test_recorder_contract.py

@@ -10,6 +10,8 @@ from pathlib import Path
 from types import SimpleNamespace
 from unittest.mock import patch
 
+import fixtures
+
 SCRIPT = Path(__file__).resolve().parents[2] / "skills/proving-it-works-with-a-movie/examples/film-terminal.py"
 
 
@@ -179,6 +181,33 @@ def serving(failure=None, stop_at=None, relative=False):
 
 
 class ServeLifecycleTests(unittest.TestCase):
+    def test_tree_cleanup_failure_keeps_failure_after_leader_exits_and_cleans_other_resources(self):
+        with serving(stop_at="ready") as rig:
+            browser = fixtures.load_script("browser_tools")
+            terminated = []
+
+            def taskkill(argv, **kwargs):
+                pid = int(argv[-1])
+                terminated.append(pid)
+                child = next(child for child in rig.children if child.pid == pid)
+                child.returncode = 0 if pid == 1100 else -9
+                return subprocess.CompletedProcess(argv, 1 if pid == 1100 else 0, b"", b"tree termination failed")
+
+            with patch.object(rig.module, "kill_process_tree", browser.kill_process_tree), \
+                 patch.object(browser.sys, "platform", "win32"), \
+                 patch.object(browser.subprocess, "run", taskkill):
+                self.assertEqual(rig.module.serve(rig.args), 1)
+            session = rig.module.read_json(rig.directory / "session.json")
+            self.assertEqual(session["pids"], [1100, 1101])
+            self.assertFalse(session.get("closed", False))
+            self.assertEqual(terminated, [1100, 1101])
+            self.assertTrue(all(child.poll() is not None for child in rig.children))
+            self.assertTrue(all(handle.closed for handle in rig.handles))
+            self.assertFalse((rig.directory / "profile").exists())
+            self.assertFalse((rig.directory / "ready.json").exists())
+            self.assertTrue(all((rig.directory / name).exists()
+                                for name in ("ttyd.log", "browser.log", "terminal.log")))
+
     def test_every_acquisition_failure_releases_owned_resources(self):
         for failure in ("ttyd.log", "browser.log", "ttyd launch", "browser launch",
                         "session metadata", "terminal.log", "connection", "later connection"):