Просмотр исходного кода

docs(movie): make native Windows recorder recipes executable

Replace the Git Bash PowerShell shorthand with complete native commands,
explicit path conversion, a kept-alive serve task, bounded readiness, and
run/key/watch/close examples. Use native input and sleep commands so the
recipe works without a sample app. Explain empty take directories and
PowerShell 5.1 embedded-quote escaping observed in native trials.

The original PowerShell missing-cwd finding does not reproduce when the
session is nested under the working directory; retain the successful
baseline and describe explicit directory creation as setup clarity.

Fresh readers exercised the final recipes on native PowerShell 5.1,
PowerShell 7, and Git Bash. Preserve the failed first candidate and driver
setup failures, distinguish instruction trials from full skill evaluation,
and keep movie acceptance with Drew. Record Drew's approval of the normal
workflow dependencies and the bounded repair plan.
Drew Ritter 2 недель назад
Родитель
Сommit
9fe81b48a2

+ 60 - 0
docs/superpowers/plans/2026-09-11-movie-review-fixes.md

@@ -0,0 +1,60 @@
+# Movie review fixes implementation plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [x]`) syntax for tracking.
+
+**Goal:** Repair the reproduced findings on #2214/#2275 while retaining the existing movie workflow.
+
+**Architecture:** Keep the recorder and five media tools. Refuse reused take directories before sending input; publish only accepted narration in the manifest. Correct the Windows recipes and missing-capability test handling.
+
+**Tech Stack:** Python, unittest, uv, ttyd, Chromium, PowerShell and Git Bash.
+
+**Spec:** `docs/superpowers/specs/2026-09-09-proof-movie-windows-completion-design.md`
+
+## Global constraints
+
+- Native Windows 11 x64: PowerShell 5.1, PowerShell 7, Git Bash; invoking and recorded shells match.
+- Preserve the Unix recipe, existing CLI interfaces, media formats, and verification policy.
+- Drew approved the necessary third-party dependencies on 2026-09-11: "those deps are normal and fine". Remove only the unused `websockets` test dependency.
+- Drew personally watches the movies for final acceptance. Do not run movie checkers, audio transcription, or image inspection for this repair pass. Mock external media boundaries in code-contract tests.
+- Preserve old takes and failed audio as evidence. Do not delete user output directories.
+- No new framework, backwards compatibility layer, merge, or external review comment.
+
+### Task 1: Repair reusable output and missing-capability contracts
+
+**Files:**
+- Modify `skills/proving-it-works-with-a-movie/examples/film-terminal.py` and `scripts/narrate`.
+- Modify `tests/proving-it-works-with-a-movie/test_terminal.py`, `test_narration.py`, `test_subtitles.py`, and `run-tests.py`.
+
+**Interfaces:** Existing recorder `film`, `run`, `key`, `watch`, and `main`; existing narration manifest; existing unittest runner.
+
+- [x] Add regressions that execute real cache/filesystem behavior with fake external synthesis/capture. Two rejected `openai-chat --verify off` invocations must both return 1 and regenerate, with failed scenes absent from the manifest; an accepted scene still caches. A cached clip rejected by strict ASR must be removed from the manifest. No actual ASR or media inspection.
+- [x] Add a frame-directory regression with existing PNGs plus a sentinel file: recording must fail before capture and preserve every byte. Test CLI refusal before connecting or typing, for run/key/watch. Keep empty/new directories valid.
+- [x] Run focused tests to observe the known failures.
+- [x] Implement the smallest guards. Before manifest append: `if sid in failures: continue`. Share a small nonempty-directory guard between the CLI preflight and direct `film()` path; use `any(out.iterdir())` only after checking existence. Report a clear error instructing the caller to use a new take directory. Guard before run/key side effects, not only inside `film`.
+- [x] Mock `module.shutil.which` in subtitle unit tests. In the real subtitle integration test call `fixtures.missing_executables("uv", "ffmpeg")` and skip before invoking `has_libass` if missing. Preserve strict runner rejection of skipped capabilities.
+- [x] Remove only `websockets` from the runner's inline dependency list.
+- [x] Run narration and recorder unit tests. Run subtitle tests with an isolated PATH containing uv and no FFmpeg; ordinary mode must pass with a capability skip, strict mode must fail because of that skip. Do not run media integration tests with FFmpeg available.
+- [x] Self-review and commit only owned files. Report commands, results, commits, and any concerns.
+
+### Task 2: Make Windows recipes executable from a fresh directory
+
+**Files:** Modify `skills/proving-it-works-with-a-movie/recording-a-terminal.md`; record local before/after instruction evidence separately from shipped guidance.
+
+**Interfaces:** Existing serve/run/key/watch/close CLI, readiness JSON, exit codes 0/1/2.
+
+- [x] Preserve current docs as the baseline. Have fresh readers identify and exercise setup and invocation for each shell without consulting recorder source. Record outcomes against the unchanged recipe. The actual PowerShell recipe creates the cwd indirectly and passes; the missing-cwd review finding is not reproduced.
+- [x] Add `[System.IO.Directory]::CreateDirectory($work) | Out-Null` before PowerShell serve. Use literal-path checks for readiness because the sample directory contains brackets.
+- [x] Replace the Git Bash shorthand with a complete Bash block: `skill=$(cygpath -m /c/path/to/skills/proving-it-works-with-a-movie)`, `work=$(cygpath -m "$HOME/movie O'Brien λ & [take]")`, `mkdir -p "$work"`, `film="$skill/examples/film-terminal.py"`, then `uv run --script "$film" ...` with `--shell gitbash`. Explain the harness-owned background serve lifetime and readiness before commands.
+- [x] Explain that each take needs an empty/new directory; return code 2 means the command remains active and should continue through key/watch. Include cleanup and keep the Unix section untouched.
+- [x] Use fresh readers for corrected-doc trials on native Windows. Verify shell startup, cwd, persistent state, status handling, and close; do not grade media. Record exact commands, docs revision, and limitations. Cover PowerShell 5.1 and 7 plus Git Bash without a cross-product matrix.
+- [x] Commit the corrected recipe. Review the complete repair diff against the reproduced comments; keep final media acceptance with Drew.
+
+## Results
+
+- Code repair: `4d4ede29`; 10 narration tests, 8 recorder unit tests, and 4 subtitle tests passed. The FFmpeg-dependent integration test was skipped; strict mode rejected that skip.
+- Fresh-reader baseline: Git Bash failed on the copied PowerShell call operator; PowerShell 7 passed the nested session/cwd recipe.
+- Corrected recipes passed on native Windows PowerShell 5.1, PowerShell 7, and Git Bash: echo, persistent state, interactive key, long-command watch, and close. PowerShell quoted-command arguments were checked in both versions.
+- An initial PowerShell 5.1 candidate exposed native argument quote loss. Final examples use Read-Host/Start-Sleep, with verified version-specific quoting guidance. PowerShell can retain a true success flag after a parse error; the guidance now states that limitation.
+- These are bounded instruction trials, not proof of automatic skill discovery or a full adversarial evaluation of the imported skill. Drew retains movie acceptance.
+- The worker accidentally ran one integration test that checked temporary test frames before restricting subsequent execution to unit tests. Drew was informed; that run is excluded from acceptance evidence.
+- Detailed local reports, command logs, and preserved failures are under `.superpowers/sdd/2026-09-11-movie-review-fixes/`.

+ 91 - 11
skills/proving-it-works-with-a-movie/recording-a-terminal.md

@@ -16,33 +16,113 @@ on return ends the session.
 
 It needs uv, ttyd, and Chrome or Edge on PATH, or `--ttyd` and `--browser`.
 `--shell powershell51|powershell7|gitbash` picks the filmed shell; the shell
-you type these commands into is a separate choice. From PowerShell:
+you type these commands into is a separate choice. Replace the sample
+commands below with the software you are proving.
+
+### PowerShell
+
+Run this block in a background terminal/task your harness keeps alive.
+For PowerShell 5.1 use `--shell powershell51`; for PowerShell 7 use
+`--shell powershell7`.
 
 ```powershell
 $skill = 'C:/path/to/skills/proving-it-works-with-a-movie'
 $work = "$HOME/movie O'Brien λ & [take]"
 $film = "$skill/examples/film-terminal.py"
-# in a background task, kept alive until close:
+[System.IO.Directory]::CreateDirectory($work) | Out-Null
 & uv run --script $film serve "$work/session" --shell powershell7 --cwd $work
-# then one call each; wait for "$work/session/ready.json" first:
-& uv run --script $film run "$work/session" 'pytest -q' --record "$work/take-one"
-& uv run --script $film run "$work/session" 'python app.py' --record "$work/take-two" --seconds 8  # a TUI: exits 2, still running
-& uv run --script $film key "$work/session" q --record "$work/take-three"
+```
+
+In a second terminal/task, define the same paths and wait for readiness.
+Repeat these three variable definitions in each tool call if your harness
+starts a fresh shell for every call. `-LiteralPath` keeps the brackets in
+the sample directory name from being interpreted as wildcards.
+
+```powershell
+$skill = 'C:/path/to/skills/proving-it-works-with-a-movie'
+$work = "$HOME/movie O'Brien λ & [take]"
+$film = "$skill/examples/film-terminal.py"
+$deadline = (Get-Date).AddSeconds(60)
+while (-not (Test-Path -LiteralPath "$work/session/ready.json")) {
+    if ((Get-Date) -gt $deadline) { throw 'Recorder not ready; inspect the serve task output.' }
+    Start-Sleep -Milliseconds 200
+}
+& uv run --script $film run "$work/session" 'echo hello' --record "$work/take-one"
+& uv run --script $film run "$work/session" 'Read-Host' --record "$work/take-two" --seconds 2
+# Exit 2 means Read-Host is still waiting. Press Enter to finish it in a new take:
+& uv run --script $film key "$work/session" Enter --record "$work/take-three"
+# A long command can continue across calls; exit 2 here is expected too:
+& uv run --script $film run "$work/session" 'Start-Sleep -Seconds 5' --record "$work/take-four" --seconds 1
+& uv run --script $film watch "$work/session" --record "$work/take-five" --seconds 10
 & uv run --script $film close "$work/session"
 ```
 
-From Git Bash the commands are the same with `skill=$(cygpath -m ...)` and
-`work=$(cygpath -m ...)`, as in assembling.md.
+Read `$LASTEXITCODE` immediately after each invocation. Stop on exit 1;
+exit 2 is expected while the interactive command is waiting for input.
+Always call `close` when finished, including after a failed command.
+
+When invoking from PowerShell 5.1, escape embedded double quotes with a
+backslash before passing a command to native uv: use the command argument
+`'python -c \"print(123)\"'`. PowerShell 7 preserves the quotes in
+`'python -c "print(123)"'` directly. This depends on the invoking shell,
+regardless of which shell you record.
+
+### Git Bash
+
+Convert paths passed to native uv/Python with `cygpath -m`. Run this block
+in a background terminal/task your harness keeps alive:
+
+```bash
+skill=$(cygpath -m /c/path/to/skills/proving-it-works-with-a-movie)
+work=$(cygpath -m "$HOME/movie O'Brien λ & [take]")
+film="$skill/examples/film-terminal.py"
+mkdir -p "$work"
+uv run --script "$film" serve "$work/session" --shell gitbash --cwd "$work"
+```
+
+In a second terminal/task, use the same paths. Repeat the three variable
+definitions in each tool call if it starts a fresh shell. Keep `set -e`
+off for these interactive calls so expected exit 2 does not end the script.
+
+```bash
+skill=$(cygpath -m /c/path/to/skills/proving-it-works-with-a-movie)
+work=$(cygpath -m "$HOME/movie O'Brien λ & [take]")
+film="$skill/examples/film-terminal.py"
+deadline=$((SECONDS + 60))
+until [[ -f "$work/session/ready.json" ]]; do
+    if (( SECONDS >= deadline )); then
+        printf '%s\n' 'Recorder not ready; inspect the serve task output.' >&2
+        exit 1
+    fi
+    sleep 0.2
+done
+uv run --script "$film" run "$work/session" 'echo hello' --record "$work/take-one"
+uv run --script "$film" run "$work/session" 'read -r answer' --record "$work/take-two" --seconds 2
+# Exit 2 means read is still waiting. Press Enter to finish it in a new take:
+uv run --script "$film" key "$work/session" Enter --record "$work/take-three"
+# A long command can continue across calls; exit 2 here is expected too:
+uv run --script "$film" run "$work/session" 'sleep 5' --record "$work/take-four" --seconds 1
+uv run --script "$film" watch "$work/session" --record "$work/take-five" --seconds 10
+uv run --script "$film" close "$work/session"
+```
+
+Read `$?` immediately after each invocation. Stop on exit 1; exit 2 means
+the command remains active. Always call `close` when finished, including
+after a failed command. The original `serve` task exits after `close`.
 
 `run` types the command and films at 5 fps into `--record` until the prompt
 comes back, holds 1.5 s so the result stays readable, and prints the status
 as JSON: `ok` is the shell's own success flag and `exit_code` the last native
 program's exit code, which PowerShell keeps from an earlier program when the
-command was a cmdlet. It exits 1 when the command failed and 2 when it is
-still running after `--seconds`. `key` presses one key and `watch` films
+command was a cmdlet. PowerShell can also leave its success flag true after
+a parse error, so check the terminal output when a command returns without
+doing the expected work. The recorder exits 1 when the shell reports failure
+and 2 when it is still running after `--seconds`. `key` presses one key and `watch` films
 without typing; both wait for the prompt the same way. Every `--record`
 directory is a `kind: frames` scene at `rate: 5`; a slow screenshot repeats
-the previous frame so the timing stays honest.
+the previous frame so the timing stays honest. Use a new or empty directory
+for every take, including retakes. A nonempty `--record` directory is refused
+before input is sent, preserving the earlier take.
 
 Long work spans takes exactly as on Unix: film the command being issued with
 a short `--seconds`, do other things, then `watch` the result as a new take.