From abeb9f5448d5122cb3549a48456da3cf5dee4c03 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 22:41:36 +0000 Subject: [PATCH 01/10] fix(video chain): write --out-file clips to _ (#1293) video chain accepted --out-file / -o PATH and never read it. run_chain gains an optional relocate hook that moves each downloaded clip before the chain table, the videos catalog, the seed extraction and --json see its path. The CLI numbers links over the whole manifest, so a --resume-from run writes the remaining links under the names a full run would. The stderr notice is removed. --- CHANGELOG.md | 9 ++ docs/USAGE.md | 9 +- plugins/gflow/skills/gflow-cli/SKILL.md | 2 +- skills/gflow-cli/SKILL.md | 2 +- src/gflow_cli/chain.py | 10 +- src/gflow_cli/cli_video.py | 31 ++++- tests/cli/test_cli_video_chain_out_file.py | 143 +++++++++++++++++++++ tests/cli/test_out_file_deprecation.py | 23 +--- website/docs/USAGE.md | 9 +- 9 files changed, 204 insertions(+), 34 deletions(-) create mode 100644 tests/cli/test_cli_video_chain_out_file.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2636790bf..226774436 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **`video chain --out-file PATH` (and the deprecated `-o`) now writes the clips there.** It was + accepted and ignored. Link i's clip goes to `_` next to PATH + (`out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`), numbered over the whole manifest so + `--resume-from` keeps each link's name; `--json`, the chain table and the videos catalog report + those paths, and seed frames stay under `--out-dir`. The stderr notice is gone + ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). + ## [0.99.0] — 2026-11-14 ### Added diff --git a/docs/USAGE.md b/docs/USAGE.md index 46ef7f05d..5723a9420 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -699,7 +699,7 @@ All prompts in a batch share one Flow project. The editor is opened once and sta > instead of proceeding on Flow's sticky default (typically x2) and silently > changing what the run bills. > `t2v`, `i2v`, `r2v`, `video chain`, and `video extend` additionally take `--out-file PATH` (explicit file destination). -> `video chain` does not apply it yet: it says so on stderr and the clips land under `--out-dir` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). +> `video chain` writes link i's clip to `_` next to PATH (`--out-file out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`, …), numbered over the whole manifest even for one link, so `--resume-from` writes the remaining links under the names a full run would; seed frames stay under `--out-dir`, and `--json` and the catalog report the numbered paths ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). > The mp4 lands at `` or `/.mp4`. > > **`i2v` model rules (issues #125 / #626):** every model — `omni-flash` @@ -1146,6 +1146,8 @@ Options: (fade-to-black guard). [default: 0] --aspect [9:16|16:9] Uniform aspect for every link. [default: 9:16] --profile NAME Profile name (overrides default). + --out-file FILE Write link i's clip to FILE's stem plus _i + (clip.mp4 -> clip_1.mp4, clip_2.mp4). --out-dir DIR Directory for the link mp4s + seed frames. --json Emit a machine-readable JSON result. ``` @@ -1185,7 +1187,10 @@ the manifest is currently informational). ### Output — N clips, not one file Each link is saved as its own mp4 (plus a `linkN_lastframe.jpg` seed frame -between links) under `--out-dir` (or the default output dir). **Stitching the +between links) under `--out-dir` (or the default output dir). With +`--out-file out/clip.mp4` link i's clip is written to `out/clip_.mp4` +instead (1-based over the whole manifest, so a `--resume-from` run keeps each +link's number); the seed frames stay under `--out-dir`. **Stitching the clips into a single video is a separate step — use `gflow scene` to concatenate them server-side.** Chain does not auto-concat: see the *deferred auto-concat* entry in diff --git a/plugins/gflow/skills/gflow-cli/SKILL.md b/plugins/gflow/skills/gflow-cli/SKILL.md index a502aa908..a6606854f 100644 --- a/plugins/gflow/skills/gflow-cli/SKILL.md +++ b/plugins/gflow/skills/gflow-cli/SKILL.md @@ -112,7 +112,7 @@ gflow video upscale --scale {1080p|720p|270p} [--project ID] [--out DI # removed. For multi-clip runs, loop `gflow video t2v`/`i2v` from the shell. gflow video chain [--out-dir DIR] [--dry-run] \ [--max-links N] [--resume-from N] # last-frame I2V chaining; veo models only - # --out-file is not applied by chain yet (stderr notice, #1293): clips go to --out-dir + # --out-file out/clip.mp4 writes link i to out/clip_.mp4 (#1293) # Characters (reusable, project-scoped subjects) gflow character create --project --name "" --face-prompt "" \ diff --git a/skills/gflow-cli/SKILL.md b/skills/gflow-cli/SKILL.md index a502aa908..a6606854f 100644 --- a/skills/gflow-cli/SKILL.md +++ b/skills/gflow-cli/SKILL.md @@ -112,7 +112,7 @@ gflow video upscale --scale {1080p|720p|270p} [--project ID] [--out DI # removed. For multi-clip runs, loop `gflow video t2v`/`i2v` from the shell. gflow video chain [--out-dir DIR] [--dry-run] \ [--max-links N] [--resume-from N] # last-frame I2V chaining; veo models only - # --out-file is not applied by chain yet (stderr notice, #1293): clips go to --out-dir + # --out-file out/clip.mp4 writes link i to out/clip_.mp4 (#1293) # Characters (reusable, project-scoped subjects) gflow character create --project --name "" --face-prompt "" \ diff --git a/src/gflow_cli/chain.py b/src/gflow_cli/chain.py index b136c7245..689d4cb5b 100644 --- a/src/gflow_cli/chain.py +++ b/src/gflow_cli/chain.py @@ -54,7 +54,7 @@ from gflow_cli.media import extract_last_frame if TYPE_CHECKING: - from collections.abc import Sequence + from collections.abc import Callable, Sequence from pathlib import Path from gflow_cli.api.client import FlowApiClient @@ -339,6 +339,7 @@ async def run_chain( aspect: Aspect = Aspect.PORTRAIT, seed_offset_ms: int = 0, jitter: float = 0.0, + relocate: Callable[[int, VideoResult], VideoResult] | None = None, ) -> list[ChainLinkResult]: """Render ``links`` as a sequential last-frame I2V chain. @@ -368,6 +369,11 @@ async def run_chain( before EOF (the fade-to-black mitigation). jitter: When > 0, sleep a random ``[0, jitter)`` seconds BETWEEN links (anti-bot cadence); never before link 0. + relocate: Optional ``(index, result) -> result`` that moves a downloaded + clip (``--out-file``, #1293). It runs before anything reads the + path, so the recorders, the seed extraction and the return value + all see where the clip ended up. A result with no ``local_path`` + must come back unchanged. Returns: One :class:`ChainLinkResult` per link, in order. @@ -405,6 +411,8 @@ async def run_chain( on_link_started=on_link_started, on_link_failed=on_link_failed, ) + if relocate is not None: # a missing clip is left as is and refused just below + result = relocate(index, result) if result.local_path is None: msg = f"link {index} returned no local_path (download failed)" diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index 337de962e..d9c5541d0 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -788,6 +788,26 @@ class _ChainExecConfig: jitter: float chain_id: str as_json: bool + relocate: Any = None + + +def _chain_clip_relocator(output_file: Path | None, skipped: int) -> Any: + """``--out-file`` for a chain (#1293): link i (1-based) goes to ``_``. + + ``skipped`` makes i count the whole manifest, so a resumed run names its links as a full + run would. Always numbered, even for one link, so a resume never renames a clip. + """ + if output_file is None: + return None + output_file.parent.mkdir(parents=True, exist_ok=True) + + def relocate(index: int, result: Any) -> Any: + number = skipped + index + 1 + return _relocate_single_video( + result, output_file.parent / f"{output_file.stem}_{number}{output_file.suffix}" + ) + + return relocate async def _execute_chain_links( @@ -897,6 +917,7 @@ def _on_link_completed(request: GenerateVideoRequest, result: VideoResult) -> No aspect=aspect_enum, seed_offset_ms=seed_offset, jitter=jitter, + relocate=cfg.relocate, ) return results, False, completed_paths except ChainPartialError as exc: @@ -1107,6 +1128,7 @@ async def _run_chain( jitter=jitter, chain_id=chain_id, as_json=as_json, + relocate=_chain_clip_relocator(output_file, skipped), ), ) finally: @@ -1782,7 +1804,9 @@ def r2v( ) @click.option("--profile", default=None, help="Profile name (overrides default).") @tool_option -@out_file_option(help="Explicit output file path for the generated chain clips.") +@out_file_option( + help="Write link i's clip to FILE's stem plus _i (clip.mp4 -> clip_1.mp4, clip_2.mp4)." +) @click.option( "--out-dir", "out_dir", @@ -1813,11 +1837,6 @@ def chain( as_json: bool, ) -> None: """Render the chain MANIFEST (one continuous last-frame I2V sequence).""" - if output_file is not None: # _run_chain has never written there; say so (#1293) - click.echo( - "video chain does not apply --out-file yet (#1293); clips are saved under --out-dir.", - err=True, - ) profile_name = _resolve_profile(profile) provider_dir = _make_provider_dir(profile_name) run_with_handlers( diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py new file mode 100644 index 000000000..3e0d2f1f8 --- /dev/null +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -0,0 +1,143 @@ +"""#1293: `video chain --out-file PATH` writes link i's clip to ``_``. + +The REAL ``run_chain`` runs; only the transport (``generate_video``), the frame +extractor and the two recorders are faked, so the move is seen by every consumer +of the clip path: the chain table, the videos catalog, the next link's seed +extraction and ``--json``. +""" + +from __future__ import annotations + +import inspect +import json +from pathlib import Path +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from click.testing import CliRunner as _CliRunner + +from gflow_cli._cli_helpers import _OUT_FILE_DEPRECATION_WARNING +from gflow_cli.cli import main + + +def _runner() -> _CliRunner: + """stderr captured apart on every Click version pyproject allows (>=8.1).""" + if "mix_stderr" in inspect.signature(_CliRunner.__init__).parameters: + return _CliRunner(mix_stderr=False) # type: ignore[call-arg] # Click < 8.2 + return _CliRunner() + + +def _invoke_chain( + tmp_path: Path, *, links: int, completed: int, args: list[str] +) -> tuple[Any, MagicMock, MagicMock, list[Path]]: + """Run `gflow video chain` over ``links`` links, ``completed`` already recorded.""" + from gflow_cli.api.video import GenerateVideoRequest, VideoResult, VideoStatus + from gflow_cli.chain import run_chain as real_run_chain + + manifest = tmp_path / "chain.jsonl" + manifest.write_text( + "\n".join(f'{{"prompt": "link {i}"}}' for i in range(links)), encoding="utf-8" + ) + download_dir = tmp_path / "dl" + download_dir.mkdir() + media_ids = iter(f"m{i}" for i in range(links)) + + async def _gen(*, req: GenerateVideoRequest, **_: Any) -> VideoResult: + media_id = next(media_ids) + clip = download_dir / f"{media_id}.mp4" + clip.write_bytes(b"\x00\x00\x00\x18ftypmp42") + return VideoResult( + status=VideoStatus(media_id=media_id, status="MEDIA_GENERATION_STATUS_SUCCESSFUL"), + local_path=clip, + ) + + client = MagicMock() + client.__aenter__ = AsyncMock(return_value=client) + client.__aexit__ = AsyncMock(return_value=None) + client.generate_video = AsyncMock(side_effect=_gen) + + extracted_from: list[Path] = [] + + def _extract(src: Path, dst: Path, *, offset_ms: int = 0) -> Path: + assert src.exists(), f"seed extraction read a clip that is not on disk: {src}" + extracted_from.append(src) + dst.write_bytes(b"\xff\xd8\xff\xd9") + return dst + + chain_recorder = MagicMock() + chain_recorder.completed_links.return_value = [MagicMock()] * completed + catalog = MagicMock() + assert real_run_chain.__kwdefaults__ is not None + with ( + patch("gflow_cli.cli_video._resolve_profile", return_value="default"), + patch("gflow_cli.cli_video._make_provider_dir", return_value=tmp_path / "prof"), + patch("gflow_cli.data.chain_repo.ChainLinkRecorder.open", return_value=chain_recorder), + patch("gflow_cli.cli_video.OperationRecorder.open", return_value=catalog), + patch("gflow_cli.cli_video.FlowApiClient", return_value=client), + patch.dict(real_run_chain.__kwdefaults__, {"extractor": _extract}), + ): + result = _runner().invoke( + main, + [ + "video", + "chain", + str(manifest), + "--yes", + "--json", + "--out-dir", + str(download_dir), + *args, + ], + ) + return result, chain_recorder, catalog, extracted_from + + +def _assert_written_and_reported( + result: Any, chain_recorder: MagicMock, catalog: MagicMock, expected: list[Path] +) -> None: + assert result.exit_code == 0, result.output + for path in expected: + assert path.is_file(), f"--out-file clip not written: {path}" + payload = json.loads(result.stdout) + assert [Path(link["local_path"]) for link in payload["links"]] == expected + assert [Path(p) for p in payload["completed_paths"]] == expected + assert [c.args[0].local_path for c in chain_recorder.record_chain_link.call_args_list] == ( + expected + ) + assert [ + c.kwargs["result"].local_path for c in catalog.record_completed_video.call_args_list + ] == expected + + +@pytest.mark.parametrize("flag", ["--out-file", "-o"]) +def test_chain_out_file_writes_each_link_to_a_numbered_path(tmp_path: Path, flag: str) -> None: + """Two links with ``--out-file out/clip.mp4`` land at clip_1.mp4 and clip_2.mp4.""" + target = tmp_path / "out" / "clip.mp4" + result, chain_recorder, catalog, extracted_from = _invoke_chain( + tmp_path, links=2, completed=0, args=[flag, str(target)] + ) + + expected = [target.parent / "clip_1.mp4", target.parent / "clip_2.mp4"] + _assert_written_and_reported(result, chain_recorder, catalog, expected) + assert extracted_from == [expected[0]] # link 1 was seeded from the moved clip + assert "does not apply --out-file" not in result.stderr + if flag == "-o": + assert result.stderr.strip() == _OUT_FILE_DEPRECATION_WARNING + else: + assert _OUT_FILE_DEPRECATION_WARNING not in result.stderr + + +def test_chain_out_file_resume_keeps_each_links_number(tmp_path: Path) -> None: + """A resume of a 3-link chain with link 1 done writes clip_2 and clip_3, never clip_1.""" + target = tmp_path / "out" / "clip.mp4" + result, chain_recorder, catalog, _ = _invoke_chain( + tmp_path, + links=3, + completed=1, + args=["--resume-from", "chain-abc", "--out-file", str(target)], + ) + + expected = [target.parent / "clip_2.mp4", target.parent / "clip_3.mp4"] + _assert_written_and_reported(result, chain_recorder, catalog, expected) + assert not (target.parent / "clip_1.mp4").exists() diff --git a/tests/cli/test_out_file_deprecation.py b/tests/cli/test_out_file_deprecation.py index ed46f08aa..a010ad180 100644 --- a/tests/cli/test_out_file_deprecation.py +++ b/tests/cli/test_out_file_deprecation.py @@ -355,24 +355,5 @@ def test_video_upscale_out_deprecation(tmp_path: Path) -> None: assert out_path.is_relative_to(dir2), out_path # -o DIR reached out_dir -@pytest.mark.parametrize("flag", ["--out-file", "-o"]) -def test_video_chain_says_the_file_path_is_not_applied(tmp_path: Path, flag: str) -> None: - """#1293: chain never wrote to the given path; it must say so instead of staying silent.""" - manifest = tmp_path / "chain.jsonl" - manifest.write_text('{"prompt": "a"}\n{"prompt": "b"}\n', encoding="utf-8") - - async def _ran(**_kwargs: object) -> None: - return None - - with ( - patch("gflow_cli.cli_video._resolve_profile", return_value="default"), - patch("gflow_cli.cli_video._make_provider_dir", return_value=tmp_path), - patch("gflow_cli.cli_video._run_chain", _ran), - ): - result = CliRunner().invoke( - main, ["video", "chain", str(manifest), "--yes", flag, str(tmp_path / "c.mp4")] - ) - - assert result.exit_code == 0, result.output - assert "video chain does not apply --out-file yet" in result.stderr - assert "#1293" in result.stderr +# `video chain --out-file` / `-o` (#1293) is pinned by tests/cli/test_cli_video_chain_out_file.py, +# which runs the real run_chain so the written files can be checked. diff --git a/website/docs/USAGE.md b/website/docs/USAGE.md index 4c59e8e82..7864e7f01 100644 --- a/website/docs/USAGE.md +++ b/website/docs/USAGE.md @@ -699,7 +699,7 @@ All prompts in a batch share one Flow project. The editor is opened once and sta > instead of proceeding on Flow's sticky default (typically x2) and silently > changing what the run bills. > `t2v`, `i2v`, `r2v`, `video chain`, and `video extend` additionally take `--out-file PATH` (explicit file destination). -> `video chain` does not apply it yet: it says so on stderr and the clips land under `--out-dir` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). +> `video chain` writes link i's clip to `_` next to PATH (`--out-file out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`, …), numbered over the whole manifest even for one link, so `--resume-from` writes the remaining links under the names a full run would; seed frames stay under `--out-dir`, and `--json` and the catalog report the numbered paths ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). > The mp4 lands at `` or `/.mp4`. > > **`i2v` model rules (issues #125 / #626):** every model — `omni-flash` @@ -1146,6 +1146,8 @@ Options: (fade-to-black guard). [default: 0] --aspect [9:16|16:9] Uniform aspect for every link. [default: 9:16] --profile NAME Profile name (overrides default). + --out-file FILE Write link i's clip to FILE's stem plus _i + (clip.mp4 -> clip_1.mp4, clip_2.mp4). --out-dir DIR Directory for the link mp4s + seed frames. --json Emit a machine-readable JSON result. ``` @@ -1185,7 +1187,10 @@ the manifest is currently informational). ### Output — N clips, not one file Each link is saved as its own mp4 (plus a `linkN_lastframe.jpg` seed frame -between links) under `--out-dir` (or the default output dir). **Stitching the +between links) under `--out-dir` (or the default output dir). With +`--out-file out/clip.mp4` link i's clip is written to `out/clip_.mp4` +instead (1-based over the whole manifest, so a `--resume-from` run keeps each +link's number); the seed frames stay under `--out-dir`. **Stitching the clips into a single video is a separate step — use `gflow scene` to concatenate them server-side.** Chain does not auto-concat: see the *deferred auto-concat* entry in From f96dd3bc21462b5922b0391727ac81da983e0bf9 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 22:48:15 +0000 Subject: [PATCH 02/10] fix(video chain): keep a billed clip recorded when --out-file cannot move it (#1293) A move failure (locked target, full disk) escaped run_chain as an unexpected error before the link reached the chain table, so --resume-from would bill it again. The clip now stays where it downloaded, is recorded at that path, and a warning names the target. Documents --out-file in the CONFIGURATION chain flag table. --- CHANGELOG.md | 4 +++- docs/CONFIGURATION.md | 1 + src/gflow_cli/cli_video.py | 11 +++++---- tests/cli/test_cli_video_chain_out_file.py | 27 ++++++++++++++++++++++ website/docs/CONFIGURATION.md | 1 + 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 226774436..bae1d2642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 accepted and ignored. Link i's clip goes to `_` next to PATH (`out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each link's name; `--json`, the chain table and the videos catalog report - those paths, and seed frames stay under `--out-dir`. The stderr notice is gone + those paths, and seed frames stay under `--out-dir`. A clip that cannot be moved there stays + where it downloaded and is recorded at that path, with a warning, so `--resume-from` does not + bill it again. The stderr notice is gone ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). ## [0.99.0] — 2026-11-14 diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index ef5d0c349..b55ca1955 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -643,6 +643,7 @@ waits for each link in turn, so total wallclock is the sum of all link waits. | `--jitter F` | `0.0` | Random `0..F` second pause **between** links (anti-bot cadence; never before link 0). | | `--seed-offset MS` | `0` | Extract the seed frame this many ms before EOF (fade-to-black guard). | | `--aspect` | `9:16` | `9:16` / `16:9`. Applied uniformly to every link (continuity requirement). | +| `--out-file FILE` | unset | Link i's clip goes to `_` next to FILE (`clip.mp4` gives `clip_1.mp4`, `clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each name; seed frames stay under `--out-dir`. A clip that cannot be moved stays where it downloaded, with a warning. Deprecated spelling: `-o/--output` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). | | `--out-dir DIR` | output root | Directory for the link mp4s + `linkN_lastframe.jpg` seed frames. | | `--profile NAME` | default profile | Per-subcommand profile override. | | `--json` | off | Emit a machine-readable JSON result. | diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index d9c5541d0..57647aaa6 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -802,10 +802,13 @@ def _chain_clip_relocator(output_file: Path | None, skipped: int) -> Any: output_file.parent.mkdir(parents=True, exist_ok=True) def relocate(index: int, result: Any) -> Any: - number = skipped + index + 1 - return _relocate_single_video( - result, output_file.parent / f"{output_file.stem}_{number}{output_file.suffix}" - ) + name = f"{output_file.stem}_{skipped + index + 1}{output_file.suffix}" + target = output_file.parent / name + try: + return _relocate_single_video(result, target) + except OSError as exc: # billed already: keep and record the clip where it is + logger.warning("chain_out_file_move_failed", target=str(target), error=str(exc)) + return result return relocate diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py index 3e0d2f1f8..f0049e17e 100644 --- a/tests/cli/test_cli_video_chain_out_file.py +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -128,6 +128,33 @@ def test_chain_out_file_writes_each_link_to_a_numbered_path(tmp_path: Path, flag assert _OUT_FILE_DEPRECATION_WARNING not in result.stderr +def test_chain_out_file_move_failure_keeps_the_billed_clip_recorded(tmp_path: Path) -> None: + """A clip that cannot be moved stays where it downloaded and is recorded there. + + The link is already billed: losing its chain row would make --resume-from bill it again. + """ + from gflow_cli import cli_video + + real_move = cli_video._relocate_single_video # pyright: ignore[reportPrivateUsage] + calls = 0 + + def _move(item: Any, target: Path) -> Any: + nonlocal calls + calls += 1 + if calls == 2: + raise PermissionError(13, "locked", str(target)) + return real_move(item, target) + + target = tmp_path / "out" / "clip.mp4" + with patch("gflow_cli.cli_video._relocate_single_video", _move): + result, chain_recorder, catalog, _ = _invoke_chain( + tmp_path, links=2, completed=0, args=["--out-file", str(target)] + ) + + expected = [target.parent / "clip_1.mp4", tmp_path / "dl" / "m1.mp4"] + _assert_written_and_reported(result, chain_recorder, catalog, expected) + + def test_chain_out_file_resume_keeps_each_links_number(tmp_path: Path) -> None: """A resume of a 3-link chain with link 1 done writes clip_2 and clip_3, never clip_1.""" target = tmp_path / "out" / "clip.mp4" diff --git a/website/docs/CONFIGURATION.md b/website/docs/CONFIGURATION.md index 7e1308eee..c1ca3d68c 100644 --- a/website/docs/CONFIGURATION.md +++ b/website/docs/CONFIGURATION.md @@ -643,6 +643,7 @@ waits for each link in turn, so total wallclock is the sum of all link waits. | `--jitter F` | `0.0` | Random `0..F` second pause **between** links (anti-bot cadence; never before link 0). | | `--seed-offset MS` | `0` | Extract the seed frame this many ms before EOF (fade-to-black guard). | | `--aspect` | `9:16` | `9:16` / `16:9`. Applied uniformly to every link (continuity requirement). | +| `--out-file FILE` | unset | Link i's clip goes to `_` next to FILE (`clip.mp4` gives `clip_1.mp4`, `clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each name; seed frames stay under `--out-dir`. A clip that cannot be moved stays where it downloaded, with a warning. Deprecated spelling: `-o/--output` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). | | `--out-dir DIR` | output root | Directory for the link mp4s + `linkN_lastframe.jpg` seed frames. | | `--profile NAME` | default profile | Per-subcommand profile override. | | `--json` | off | Emit a machine-readable JSON result. | From edd33b8a60f0f83cd8c2c1126576d21b7f24f131 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:08:01 +0000 Subject: [PATCH 03/10] refactor(out-file): one naming helper and typed chain relocator (#1293 review) The _ rule lived in four copies (video, image, worker, chain). paths.numbered_path now holds it, next to relocate_video and relocate_video_output, moved from cli_video. The chain relocator is typed as run_chain declares it, and the chain --out-file test reuses the shared CliRunner helper. --- src/gflow_cli/cli_image.py | 4 +- src/gflow_cli/cli_video.py | 57 +++++----------------- src/gflow_cli/paths.py | 41 ++++++++++++++++ src/gflow_cli/worker/daemon.py | 7 +-- tests/cli/test_cli_video.py | 2 +- tests/cli/test_cli_video_chain_out_file.py | 17 ++----- tests/cli/test_predictable_output.py | 8 +-- 7 files changed, 65 insertions(+), 71 deletions(-) diff --git a/src/gflow_cli/cli_image.py b/src/gflow_cli/cli_image.py index 1ff320af0..edadd6592 100644 --- a/src/gflow_cli/cli_image.py +++ b/src/gflow_cli/cli_image.py @@ -109,7 +109,7 @@ from gflow_cli.image_batch import ( MIN_COUNT as _MIN_COUNT, ) -from gflow_cli.paths import image_output_path, resolve_batch_output_dir +from gflow_cli.paths import image_output_path, numbered_path, resolve_batch_output_dir from gflow_cli.profile_store import account_locale_for from gflow_cli.refresh_resolver import ( _build_name_resolver as _build_name_resolver, # pyright: ignore[reportPrivateUsage] @@ -1123,7 +1123,7 @@ async def _download_images( if len(images) == 1: target = output_file else: - target = output_file.parent / f"{output_file.stem}_{i}{output_file.suffix}" + target = numbered_path(output_file, i) elif out is not None: target = out / f"{img.media_name}_{i}.png" else: diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index 57647aaa6..fbeb4682e 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -53,11 +53,13 @@ SceneConcatError, ) from gflow_cli.image_batch import resolve_jitter_range +from gflow_cli.paths import numbered_path, relocate_video, relocate_video_output from gflow_cli.storage import cloud_info_from_path, refuse_cloud_storage_for_local_file if TYPE_CHECKING: from collections.abc import Callable, Sequence + from gflow_cli.api.video import VideoResult from gflow_cli.api.video_extend import ExtendStarted from gflow_cli.chain import ChainLinkSpec from gflow_cli.tools.invocation import AppliedTool @@ -238,44 +240,6 @@ def _reject_duration_without_control( raise click.UsageError(f"{named}: {exc}") from exc -def _relocate_single_video(item: Any, target: Path) -> Any: - from dataclasses import replace - from typing import cast - - local_p = cast("Path | None", getattr(item, "local_path", None)) - if local_p is None or not local_p.exists(): - return item - if local_p != target: - import shutil - - # shutil.move, not Path.replace: a rename across mounts raises EXDEV (#1114). - shutil.move(local_p, target) - return replace(item, local_path=target) - return item - - -def _relocate_video_output(result: Any, output_file: Path | None) -> Any: - if output_file is None: - return result - from typing import cast - - output_file.parent.mkdir(parents=True, exist_ok=True) - if isinstance(result, (list, tuple)): - items = cast("tuple[Any, ...] | list[Any]", result) - relocated: list[Any] = [] - is_single = len(items) == 1 - for i, item in enumerate(items, start=1): - target = ( - output_file - if is_single - else output_file.parent / f"{output_file.stem}_{i}{output_file.suffix}" - ) - relocated.append(_relocate_single_video(item, target)) - return relocated - - return _relocate_single_video(result, output_file) - - async def _generate_and_report( request: Any, *, @@ -365,7 +329,7 @@ def on_started(started: VideoStarted) -> None: **resolver_kw, ) - result = _relocate_video_output(result, output_file) + result = relocate_video_output(result, output_file) try: recorder.record_completed_video( @@ -788,10 +752,12 @@ class _ChainExecConfig: jitter: float chain_id: str as_json: bool - relocate: Any = None + relocate: Callable[[int, VideoResult], VideoResult] | None = None -def _chain_clip_relocator(output_file: Path | None, skipped: int) -> Any: +def _chain_clip_relocator( + output_file: Path | None, skipped: int +) -> Callable[[int, VideoResult], VideoResult] | None: """``--out-file`` for a chain (#1293): link i (1-based) goes to ``_``. ``skipped`` makes i count the whole manifest, so a resumed run names its links as a full @@ -801,11 +767,10 @@ def _chain_clip_relocator(output_file: Path | None, skipped: int) -> Any: return None output_file.parent.mkdir(parents=True, exist_ok=True) - def relocate(index: int, result: Any) -> Any: - name = f"{output_file.stem}_{skipped + index + 1}{output_file.suffix}" - target = output_file.parent / name + def relocate(index: int, result: VideoResult) -> VideoResult: + target = numbered_path(output_file, skipped + index + 1) try: - return _relocate_single_video(result, target) + return relocate_video(result, target) except OSError as exc: # billed already: keep and record the clip where it is logger.warning("chain_out_file_move_failed", target=str(target), error=str(exc)) return result @@ -838,7 +803,7 @@ async def _execute_chain_links( chain_id = cfg.chain_id as_json = cfg.as_json - from gflow_cli.api.video import GenerateVideoRequest, VideoResult, VideoStarted + from gflow_cli.api.video import GenerateVideoRequest, VideoStarted from gflow_cli.errors import ChainPartialError # (request, media_id) pairs — matched by object IDENTITY in _on_link_failed. diff --git a/src/gflow_cli/paths.py b/src/gflow_cli/paths.py index e4e12503a..ddc3df08f 100644 --- a/src/gflow_cli/paths.py +++ b/src/gflow_cli/paths.py @@ -20,8 +20,11 @@ from __future__ import annotations import re +import shutil +from dataclasses import replace from datetime import date from pathlib import Path +from typing import Any, cast from platformdirs import user_data_dir, user_downloads_dir @@ -276,3 +279,41 @@ def correct_image_extension(path: Path) -> Path: return path path.rename(target) return target + + +def numbered_path(path: Path, number: int) -> Path: + """File ``number`` (1-based) of several written from one ``--out-file PATH``. + + The one naming rule for every command that fans one ``--out-file`` out to several + files (``hero.png`` -> ``hero_1.png``, ``hero_2.png``; #1158, #1293). + """ + return path.parent / f"{path.stem}_{number}{path.suffix}" + + +def relocate_video(item: Any, target: Path) -> Any: + """Move ``item.local_path`` to ``target``; ``item`` unchanged when there is no file.""" + local_p = cast("Path | None", getattr(item, "local_path", None)) + if local_p is None or not local_p.exists(): + return item + if local_p != target: + # shutil.move, not Path.replace: a rename across mounts raises EXDEV (#1114). + shutil.move(local_p, target) + return replace(item, local_path=target) + return item + + +def relocate_video_output(result: Any, output_file: Path | None) -> Any: + """Apply ``--out-file`` to one result or a list: one file keeps PATH, several are numbered.""" + if output_file is None: + return result + output_file.parent.mkdir(parents=True, exist_ok=True) + if isinstance(result, (list, tuple)): + items = cast("tuple[Any, ...] | list[Any]", result) + relocated: list[Any] = [] + is_single = len(items) == 1 + for i, item in enumerate(items, start=1): + target = output_file if is_single else numbered_path(output_file, i) + relocated.append(relocate_video(item, target)) + return relocated + + return relocate_video(result, output_file) diff --git a/src/gflow_cli/worker/daemon.py b/src/gflow_cli/worker/daemon.py index 79bf7afda..6dbc5234a 100644 --- a/src/gflow_cli/worker/daemon.py +++ b/src/gflow_cli/worker/daemon.py @@ -31,7 +31,7 @@ outcome_members, ) from gflow_cli.observability import exception_message_hash -from gflow_cli.paths import image_output_path +from gflow_cli.paths import image_output_path, numbered_path from gflow_cli.services.reference_uploads import generate_reusing_uploads from gflow_cli.storage import cloud_info_from_path from gflow_cli.worker import codec @@ -269,10 +269,7 @@ async def _generate(request: GenerateImageRequest) -> list[GeneratedImage]: if len(images) == 1: target = output_file else: - target = ( - output_file.parent - / f"{output_file.stem}_{i}{output_file.suffix}" - ) + target = numbered_path(output_file, i) target.parent.mkdir(parents=True, exist_ok=True) else: target = image_output_path( diff --git a/tests/cli/test_cli_video.py b/tests/cli/test_cli_video.py index 1d900943e..26cf480a9 100644 --- a/tests/cli/test_cli_video.py +++ b/tests/cli/test_cli_video.py @@ -1729,7 +1729,7 @@ def test_duration_on_omni_flash_passes_the_guard(self, tmp_path: Path) -> None: def test_output_rejects_an_existing_directory_before_generating(tmp_path: Path, sub: str) -> None: """`-o ` must fail at parse time, not after a paid generation. - `_relocate_video_output` calls `Path.replace()` onto the target, which raises + `relocate_video_output` calls `Path.replace()` onto the target, which raises `PermissionError: [WinError 5]` when the target is a directory. That happens AFTER Flow has rendered and billed the clip, so it surfaced as a bare "Unexpected error" exit 1 and left the paid mp4 orphaned under a UUID name at diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py index f0049e17e..3a481720a 100644 --- a/tests/cli/test_cli_video_chain_out_file.py +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -8,24 +8,16 @@ from __future__ import annotations -import inspect import json from pathlib import Path from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest -from click.testing import CliRunner as _CliRunner from gflow_cli._cli_helpers import _OUT_FILE_DEPRECATION_WARNING from gflow_cli.cli import main - - -def _runner() -> _CliRunner: - """stderr captured apart on every Click version pyproject allows (>=8.1).""" - if "mix_stderr" in inspect.signature(_CliRunner.__init__).parameters: - return _CliRunner(mix_stderr=False) # type: ignore[call-arg] # Click < 8.2 - return _CliRunner() +from tests.cli.test_out_file_deprecation import CliRunner def _invoke_chain( @@ -77,7 +69,7 @@ def _extract(src: Path, dst: Path, *, offset_ms: int = 0) -> Path: patch("gflow_cli.cli_video.FlowApiClient", return_value=client), patch.dict(real_run_chain.__kwdefaults__, {"extractor": _extract}), ): - result = _runner().invoke( + result = CliRunner().invoke( main, [ "video", @@ -133,9 +125,8 @@ def test_chain_out_file_move_failure_keeps_the_billed_clip_recorded(tmp_path: Pa The link is already billed: losing its chain row would make --resume-from bill it again. """ - from gflow_cli import cli_video + from gflow_cli.paths import relocate_video as real_move - real_move = cli_video._relocate_single_video # pyright: ignore[reportPrivateUsage] calls = 0 def _move(item: Any, target: Path) -> Any: @@ -146,7 +137,7 @@ def _move(item: Any, target: Path) -> Any: return real_move(item, target) target = tmp_path / "out" / "clip.mp4" - with patch("gflow_cli.cli_video._relocate_single_video", _move): + with patch("gflow_cli.cli_video.relocate_video", _move): result, chain_recorder, catalog, _ = _invoke_chain( tmp_path, links=2, completed=0, args=["--out-file", str(target)] ) diff --git a/tests/cli/test_predictable_output.py b/tests/cli/test_predictable_output.py index d0eecf57a..9462e436d 100644 --- a/tests/cli/test_predictable_output.py +++ b/tests/cli/test_predictable_output.py @@ -272,7 +272,7 @@ def _mock_generate_video(*args: object, **kwargs: object) -> VideoResult: def test_relocate_video_output_multi_count(self, tmp_path: Path) -> None: from gflow_cli.api.video import VideoResult, VideoStatus - from gflow_cli.cli_video import _relocate_video_output + from gflow_cli.paths import relocate_video_output v1_temp = tmp_path / "temp_1.mp4" v2_temp = tmp_path / "temp_2.mp4" @@ -289,7 +289,7 @@ def test_relocate_video_output_multi_count(self, tmp_path: Path) -> None: ) out_file = tmp_path / "multi_video" / "output.mp4" - relocated = _relocate_video_output([res1, res2], out_file) + relocated = relocate_video_output([res1, res2], out_file) assert len(relocated) == 2 assert relocated[0].local_path == tmp_path / "multi_video" / "output_1.mp4" @@ -305,7 +305,7 @@ def test_relocate_video_output_across_filesystems( import os from gflow_cli.api.video import VideoResult, VideoStatus - from gflow_cli.cli_video import _relocate_video_output + from gflow_cli.paths import relocate_video_output def cross_device(src: object, dst: object, *a: object, **k: object) -> None: raise OSError(errno.EXDEV, "Invalid cross-device link") @@ -322,7 +322,7 @@ def cross_device(src: object, dst: object, *a: object, **k: object) -> None: ) out_file = tmp_path / "other_mount" / "out.mp4" - relocated = _relocate_video_output(res, out_file) + relocated = relocate_video_output(res, out_file) assert relocated.local_path == out_file assert out_file.read_bytes() == b"clip" From 970fff0ff539d8bf011fb41a61aaf573126047af Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:09:09 +0000 Subject: [PATCH 04/10] fix(video chain): a resumed link keeps its manifest index and is seeded i2v (#1300) --resume-from passed links[skipped:] and run_chain numbered them from 0. The first resumed link then overwrote link 0's chain row (keyed on link_index), went out as an unseeded t2v, and left the row count short, so a later resume billed a finished link again. run_chain takes start_index and seed_clip: request mode, the result and recorded index, --json links[].index and the --out-file number count from the manifest, and the first resumed link is an i2v seeded from the last completed clip. A resume whose clip is gone is refused before the cost prompt with exit 20 naming the file, never sent as t2v. --- src/gflow_cli/chain.py | 42 +++- src/gflow_cli/cli_video.py | 42 ++-- tests/cli/test_cli_video_chain.py | 16 +- tests/cli/test_cli_video_chain_out_file.py | 243 ++++++++++++--------- tests/features/test_video_chain_steps.py | 20 +- tests/test_chain.py | 49 +++++ 6 files changed, 279 insertions(+), 133 deletions(-) diff --git a/src/gflow_cli/chain.py b/src/gflow_cli/chain.py index 689d4cb5b..d7f0509cd 100644 --- a/src/gflow_cli/chain.py +++ b/src/gflow_cli/chain.py @@ -325,6 +325,24 @@ def _build_link_result( ) +async def _resume_seed( + extractor: FrameExtractor, + *, + start_index: int, + seed_clip: Path | None, + out_dir: Path, + offset_ms: int, +) -> Path | None: + """The seed frame for ``links[0]``: none at link 0, else re-extracted from ``seed_clip``.""" + if start_index == 0: + return None + if seed_clip is None: + msg = f"link {start_index} continues a chain: pass seed_clip, the clip of the link before" + raise ValueError(msg) + dst = out_dir / f"link{start_index - 1}_lastframe.jpg" + return await asyncio.to_thread(extractor, src=seed_clip, dst=dst, offset_ms=offset_ms) + + async def run_chain( *, client: FlowApiClient, @@ -340,6 +358,8 @@ async def run_chain( seed_offset_ms: int = 0, jitter: float = 0.0, relocate: Callable[[int, VideoResult], VideoResult] | None = None, + start_index: int = 0, + seed_clip: Path | None = None, ) -> list[ChainLinkResult]: """Render ``links`` as a sequential last-frame I2V chain. @@ -374,6 +394,10 @@ async def run_chain( path, so the recorders, the seed extraction and the return value all see where the clip ended up. A result with no ``local_path`` must come back unchanged. + start_index: The manifest index of ``links[0]`` (``--resume-from``, #1300). Request + mode, ``ChainLinkResult.index`` and the recorded ``link_index`` all count from it. + seed_clip: The clip of link ``start_index - 1``; its last frame seeds ``links[0]``. + Required when ``start_index`` is past 0, so a resumed link is never sent as t2v. Returns: One :class:`ChainLinkResult` per link, in order. @@ -386,18 +410,26 @@ async def run_chain( ChainPartialError: A link failed with a ``WireFormatError`` (i2v routed to the t2v backstop) or ``WafRejectionError`` (403). Carries the ``Path`` of every link completed before the failure. + ValueError: ``start_index`` is past 0 but no ``seed_clip`` was given. """ reject_unusable_links(model=model, links=links) results: list[ChainLinkResult] = [] completed_paths: list[Path] = [] - prev_frame: Path | None = None + prev_frame = await _resume_seed( + extractor, + start_index=start_index, + seed_clip=seed_clip, + out_dir=out_dir, + offset_ms=seed_offset_ms, + ) + last_index = start_index + len(links) - 1 - for index, spec in enumerate(links): - if index > 0 and jitter > 0: + for index, spec in enumerate(links, start=start_index): + if index > start_index and jitter > 0: await asyncio.sleep(random.uniform(0.0, jitter)) # noqa: S311 # cadence, not crypto - _log.info("chain_link_started", index=index, total_links=len(links)) + _log.info("chain_link_started", index=index, total_links=last_index + 1) req = _build_link_request( spec=spec, index=index, model=model, aspect=aspect, prev_frame=prev_frame ) @@ -427,7 +459,7 @@ async def run_chain( _notify_link_failed(on_link_failed, req, exc) raise exc - is_last = index == len(links) - 1 + is_last = index == last_index # RECORD-BEFORE-EXTRACT: persist the downloaded clip before decoding it. # The frame_path is the planned seed-frame destination; it is filled in diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index fbeb4682e..903fad447 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -707,21 +707,23 @@ def _resolve_chain_resume( settings: Any, profile_name: str, profile_dir: Path, -) -> tuple[str, int]: - """Resolve the chain_id and number of already-completed links. +) -> tuple[str, int, Path | None]: + """Resolve the chain_id, the number of already-completed links and the last one's clip. For a fresh run mints a new UUID. For a resume, opens the chain recorder to count completed links and returns early (raises SystemExit via console.print + return sentinel) when the chain is already done. - Returns ``(chain_id, skipped)`` — callers check ``skipped >= len(links)`` - themselves via the returned value; this helper raises nothing. + Returns ``(chain_id, skipped, seed_clip)`` — callers check ``skipped >= len(links)`` + themselves. ``seed_clip`` seeds the first remaining link (#1300); a resume whose clip + is gone is refused here, before the cost prompt, rather than sent as an unseeded t2v. """ import uuid from gflow_cli.data.chain_repo import ChainLinkRecorder + from gflow_cli.errors import FrameExtractionError if resume_from is None: - return str(uuid.uuid4()), 0 + return str(uuid.uuid4()), 0, None chain_id = resume_from probe = ChainLinkRecorder.open( @@ -731,10 +733,18 @@ def _resolve_chain_resume( chain_id=chain_id, ) try: - skipped = len(probe.completed_links()) + completed = probe.completed_links() finally: probe.close() - return chain_id, skipped + seed_clip = Path(completed[-1].local_path) if completed else None + if seed_clip is not None and len(completed) < len(links) and not seed_clip.is_file(): + raise FrameExtractionError( + detail=f"cannot resume chain {chain_id}: the clip of its last completed link, " + f"{seed_clip}, is missing, so the next link has nothing to continue from", + remediation_hint="Put the clip back at that path, or start a new chain.", + charged="no", + ) + return chain_id, len(completed), seed_clip @dataclass(frozen=True) @@ -753,14 +763,16 @@ class _ChainExecConfig: chain_id: str as_json: bool relocate: Callable[[int, VideoResult], VideoResult] | None = None + start_index: int = 0 + seed_clip: Path | None = None def _chain_clip_relocator( - output_file: Path | None, skipped: int + output_file: Path | None, ) -> Callable[[int, VideoResult], VideoResult] | None: - """``--out-file`` for a chain (#1293): link i (1-based) goes to ``_``. + """``--out-file`` for a chain (#1293): link ``index`` goes to ``_``. - ``skipped`` makes i count the whole manifest, so a resumed run names its links as a full + ``index`` counts the whole manifest (#1300), so a resumed run names its links as a full run would. Always numbered, even for one link, so a resume never renames a clip. """ if output_file is None: @@ -768,7 +780,7 @@ def _chain_clip_relocator( output_file.parent.mkdir(parents=True, exist_ok=True) def relocate(index: int, result: VideoResult) -> VideoResult: - target = numbered_path(output_file, skipped + index + 1) + target = numbered_path(output_file, index + 1) try: return relocate_video(result, target) except OSError as exc: # billed already: keep and record the clip where it is @@ -886,6 +898,8 @@ def _on_link_completed(request: GenerateVideoRequest, result: VideoResult) -> No seed_offset_ms=seed_offset, jitter=jitter, relocate=cfg.relocate, + start_index=cfg.start_index, + seed_clip=cfg.seed_clip, ) return results, False, completed_paths except ChainPartialError as exc: @@ -1011,7 +1025,7 @@ async def _run_chain( # Resume: bind the prior chain_id and skip completed links so they are not # regenerated. A fresh run mints a new id. - chain_id, skipped = _resolve_chain_resume( + chain_id, skipped, seed_clip = _resolve_chain_resume( resume_from, links, settings=settings, @@ -1096,7 +1110,9 @@ async def _run_chain( jitter=jitter, chain_id=chain_id, as_json=as_json, - relocate=_chain_clip_relocator(output_file, skipped), + relocate=_chain_clip_relocator(output_file), + start_index=skipped, + seed_clip=seed_clip, ), ) finally: diff --git a/tests/cli/test_cli_video_chain.py b/tests/cli/test_cli_video_chain.py index e548a67a0..a8738cb1a 100644 --- a/tests/cli/test_cli_video_chain.py +++ b/tests/cli/test_cli_video_chain.py @@ -61,6 +61,16 @@ def _fake_link_result(index: int, tmp_path: Path) -> Any: ) +def _done_links(tmp_path: Path, n: int) -> list[Any]: + """Recorded links whose clips are on disk, so a resume can seed from the last (#1300).""" + links = [] + for i in range(n): + clip = tmp_path / f"done{i}.mp4" + clip.touch() + links.append(MagicMock(link_index=i, local_path=str(clip))) + return links + + def _assert_no_numeric_credit_claim(output: str) -> None: numeric_credit_lines = [ line @@ -138,7 +148,7 @@ def test_chain_resumed_dry_run_counts_only_remaining_operations(tmp_path: Path) runner = CliRunner() manifest = _manifest(tmp_path, 3) fake_recorder = MagicMock() - fake_recorder.completed_links.return_value = [MagicMock(), MagicMock()] + fake_recorder.completed_links.return_value = _done_links(tmp_path, 2) with ( patch("gflow_cli.cli_video._resolve_profile", return_value="default"), @@ -170,7 +180,7 @@ def test_chain_resume_with_all_links_completed_returns_before_external_work( runner = CliRunner() manifest = _manifest(tmp_path, 3) fake_recorder = MagicMock() - fake_recorder.completed_links.return_value = [MagicMock(), MagicMock(), MagicMock()] + fake_recorder.completed_links.return_value = _done_links(tmp_path, 3) with ( patch("gflow_cli.cli_video._resolve_profile", return_value="default"), @@ -462,7 +472,7 @@ def test_chain_resume_skips_completed_links(tmp_path: Path) -> None: # Recorder reports the first link already paid for. fake_recorder = MagicMock() - fake_recorder.completed_links.return_value = [MagicMock()] # 1 completed + fake_recorder.completed_links.return_value = _done_links(tmp_path, 1) fake_client = MagicMock() fake_client.__aenter__ = AsyncMock(return_value=fake_client) diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py index 3a481720a..2be5f4467 100644 --- a/tests/cli/test_cli_video_chain_out_file.py +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -1,9 +1,10 @@ -"""#1293: `video chain --out-file PATH` writes link i's clip to ``_``. +"""`video chain --out-file` (#1293) and `--resume-from` link numbering (#1300). -The REAL ``run_chain`` runs; only the transport (``generate_video``), the frame -extractor and the two recorders are faked, so the move is seen by every consumer -of the clip path: the chain table, the videos catalog, the next link's seed -extraction and ``--json``. +The REAL ``run_chain`` and the REAL ``ChainLinkRecorder`` run, on the per-test database +the autouse ``_isolate_settings`` fixture provides. Only the transport +(``generate_video``), the frame extractor and the videos catalog are faked, so every +consumer of a clip path is the real one: the chain table, the next link's seed and +``--json``. """ from __future__ import annotations @@ -14,105 +15,115 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +import structlog from gflow_cli._cli_helpers import _OUT_FILE_DEPRECATION_WARNING -from gflow_cli.cli import main +from gflow_cli.api.video import GenerateVideoRequest, Mode, VideoResult, VideoStatus +from gflow_cli.cli_video import video from tests.cli.test_out_file_deprecation import CliRunner +_KEPT_EVENT = "chain_clip_kept_at_download_path" -def _invoke_chain( - tmp_path: Path, *, links: int, completed: int, args: list[str] -) -> tuple[Any, MagicMock, MagicMock, list[Path]]: - """Run `gflow video chain` over ``links`` links, ``completed`` already recorded.""" - from gflow_cli.api.video import GenerateVideoRequest, VideoResult, VideoStatus - from gflow_cli.chain import run_chain as real_run_chain - manifest = tmp_path / "chain.jsonl" - manifest.write_text( - "\n".join(f'{{"prompt": "link {i}"}}' for i in range(links)), encoding="utf-8" - ) - download_dir = tmp_path / "dl" - download_dir.mkdir() - media_ids = iter(f"m{i}" for i in range(links)) +@pytest.fixture(autouse=True) +def _capture(install_log_capture: structlog.testing.LogCapture) -> structlog.testing.LogCapture: + """Keep structlog off stdout so ``--json`` stays one parseable document.""" + return install_log_capture + - async def _gen(*, req: GenerateVideoRequest, **_: Any) -> VideoResult: - media_id = next(media_ids) - clip = download_dir / f"{media_id}.mp4" +class _Flow: + """A fake Flow: one clip per successful link, a 403 for the prompts in ``fail``.""" + + def __init__(self, tmp_path: Path) -> None: + self.download_dir = tmp_path / "dl" + self.download_dir.mkdir(exist_ok=True) + self.fail: set[str] = set() + self.calls: list[tuple[str, Mode, str | None, bool]] = [] # prompt, mode, seed, ok + + async def generate_video(self, *, req: GenerateVideoRequest, **_: Any) -> VideoResult: + from gflow_cli.errors import WafRejectionError + + seed = req.start_image.read_text() if req.start_image is not None else None + ok = req.prompt not in self.fail + self.calls.append((req.prompt, req.mode, seed, ok)) + if not ok: + raise WafRejectionError("403") + media_id = "m" + req.prompt.removeprefix("link ") + clip = self.download_dir / f"{media_id}.mp4" clip.write_bytes(b"\x00\x00\x00\x18ftypmp42") return VideoResult( status=VideoStatus(media_id=media_id, status="MEDIA_GENERATION_STATUS_SUCCESSFUL"), local_path=clip, ) - client = MagicMock() - client.__aenter__ = AsyncMock(return_value=client) - client.__aexit__ = AsyncMock(return_value=None) - client.generate_video = AsyncMock(side_effect=_gen) - extracted_from: list[Path] = [] +def _extract(src: Path, dst: Path, *, offset_ms: int = 0) -> Path: + assert src.exists(), f"seed extraction read a clip that is not on disk: {src}" + dst.write_text(src.name) # the frame names the clip it came from + return dst + - def _extract(src: Path, dst: Path, *, offset_ms: int = 0) -> Path: - assert src.exists(), f"seed extraction read a clip that is not on disk: {src}" - extracted_from.append(src) - dst.write_bytes(b"\xff\xd8\xff\xd9") - return dst +def _invoke(tmp_path: Path, flow: _Flow, *, links: int, args: list[str]) -> tuple[Any, MagicMock]: + """Run `gflow video chain` once over a ``links``-link manifest.""" + from gflow_cli.chain import run_chain as real_run_chain - chain_recorder = MagicMock() - chain_recorder.completed_links.return_value = [MagicMock()] * completed + manifest = tmp_path / "chain.jsonl" + manifest.write_text( + "\n".join(f'{{"prompt": "link {i}"}}' for i in range(links)), encoding="utf-8" + ) + client = MagicMock() + client.__aenter__ = AsyncMock(return_value=client) + client.__aexit__ = AsyncMock(return_value=None) + client.generate_video = AsyncMock(side_effect=flow.generate_video) catalog = MagicMock() assert real_run_chain.__kwdefaults__ is not None with ( patch("gflow_cli.cli_video._resolve_profile", return_value="default"), patch("gflow_cli.cli_video._make_provider_dir", return_value=tmp_path / "prof"), - patch("gflow_cli.data.chain_repo.ChainLinkRecorder.open", return_value=chain_recorder), patch("gflow_cli.cli_video.OperationRecorder.open", return_value=catalog), - patch("gflow_cli.cli_video.FlowApiClient", return_value=client), + patch("gflow_cli.cli_video.FlowApiClient", return_value=client) as client_cls, patch.dict(real_run_chain.__kwdefaults__, {"extractor": _extract}), ): result = CliRunner().invoke( - main, - [ - "video", - "chain", - str(manifest), - "--yes", - "--json", - "--out-dir", - str(download_dir), - *args, - ], + video, + ["chain", str(manifest), "--yes", "--json", "--out-dir", str(flow.download_dir)] + args, ) - return result, chain_recorder, catalog, extracted_from + catalog.flow_client_cls = client_cls # lets a test see whether a browser was opened + return result, catalog -def _assert_written_and_reported( - result: Any, chain_recorder: MagicMock, catalog: MagicMock, expected: list[Path] -) -> None: - assert result.exit_code == 0, result.output - for path in expected: - assert path.is_file(), f"--out-file clip not written: {path}" - payload = json.loads(result.stdout) - assert [Path(link["local_path"]) for link in payload["links"]] == expected - assert [Path(p) for p in payload["completed_paths"]] == expected - assert [c.args[0].local_path for c in chain_recorder.record_chain_link.call_args_list] == ( - expected +def _rows(tmp_path: Path, chain_id: str) -> list[tuple[int, Path]]: + from gflow_cli.config import get_settings + from gflow_cli.data.chain_repo import ChainLinkRecorder + + recorder = ChainLinkRecorder.open( + get_settings(), profile_name="default", profile_dir=tmp_path / "prof", chain_id=chain_id ) - assert [ - c.kwargs["result"].local_path for c in catalog.record_completed_video.call_args_list - ] == expected + try: + return [(r.link_index, Path(r.local_path)) for r in recorder.completed_links()] + finally: + recorder.close() + + +def _catalog_paths(catalog: MagicMock) -> list[Path]: + return [c.kwargs["result"].local_path for c in catalog.record_completed_video.call_args_list] @pytest.mark.parametrize("flag", ["--out-file", "-o"]) def test_chain_out_file_writes_each_link_to_a_numbered_path(tmp_path: Path, flag: str) -> None: """Two links with ``--out-file out/clip.mp4`` land at clip_1.mp4 and clip_2.mp4.""" - target = tmp_path / "out" / "clip.mp4" - result, chain_recorder, catalog, extracted_from = _invoke_chain( - tmp_path, links=2, completed=0, args=[flag, str(target)] - ) + flow = _Flow(tmp_path) + out = tmp_path / "out" + result, catalog = _invoke(tmp_path, flow, links=2, args=[flag, str(out / "clip.mp4")]) - expected = [target.parent / "clip_1.mp4", target.parent / "clip_2.mp4"] - _assert_written_and_reported(result, chain_recorder, catalog, expected) - assert extracted_from == [expected[0]] # link 1 was seeded from the moved clip + assert result.exit_code == 0, result.output + expected = [out / "clip_1.mp4", out / "clip_2.mp4"] + assert all(p.is_file() for p in expected), list(out.iterdir()) + payload = json.loads(result.stdout) + assert [Path(link["local_path"]) for link in payload["links"]] == expected + assert _rows(tmp_path, payload["chain_id"]) == list(enumerate(expected)) + assert _catalog_paths(catalog) == expected + assert flow.calls[1][2] == "clip_1.mp4" # link 2 was seeded from the moved clip assert "does not apply --out-file" not in result.stderr if flag == "-o": assert result.stderr.strip() == _OUT_FILE_DEPRECATION_WARNING @@ -120,42 +131,62 @@ def test_chain_out_file_writes_each_link_to_a_numbered_path(tmp_path: Path, flag assert _OUT_FILE_DEPRECATION_WARNING not in result.stderr -def test_chain_out_file_move_failure_keeps_the_billed_clip_recorded(tmp_path: Path) -> None: - """A clip that cannot be moved stays where it downloaded and is recorded there. - - The link is already billed: losing its chain row would make --resume-from bill it again. - """ - from gflow_cli.paths import relocate_video as real_move - - calls = 0 - - def _move(item: Any, target: Path) -> Any: - nonlocal calls - calls += 1 - if calls == 2: - raise PermissionError(13, "locked", str(target)) - return real_move(item, target) - - target = tmp_path / "out" / "clip.mp4" - with patch("gflow_cli.cli_video.relocate_video", _move): - result, chain_recorder, catalog, _ = _invoke_chain( - tmp_path, links=2, completed=0, args=["--out-file", str(target)] - ) - - expected = [target.parent / "clip_1.mp4", tmp_path / "dl" / "m1.mp4"] - _assert_written_and_reported(result, chain_recorder, catalog, expected) - - -def test_chain_out_file_resume_keeps_each_links_number(tmp_path: Path) -> None: - """A resume of a 3-link chain with link 1 done writes clip_2 and clip_3, never clip_1.""" - target = tmp_path / "out" / "clip.mp4" - result, chain_recorder, catalog, _ = _invoke_chain( - tmp_path, - links=3, - completed=1, - args=["--resume-from", "chain-abc", "--out-file", str(target)], +@pytest.mark.sharpit(allow_n_plus_one=True) # three runs of one chain, on purpose (#1300) +def test_resumes_keep_manifest_indexes_seed_i2v_and_bill_each_link_once(tmp_path: Path) -> None: + """#1300: a 3-link chain stops at link 1, then at link 2, then completes.""" + flow = _Flow(tmp_path) + out = tmp_path / "out" + clips = [out / f"clip_{n}.mp4" for n in (1, 2, 3)] + + flow.fail = {"link 1"} + first, _ = _invoke(tmp_path, flow, links=3, args=["--out-file", str(out / "clip.mp4")]) + assert first.exit_code == 21, first.output + chain_id = json.loads(first.stdout)["chain_id"] + assert _rows(tmp_path, chain_id) == [(0, clips[0])] + + flow.fail = {"link 2"} + resume = ["--resume-from", chain_id, "--out-file", str(out / "clip.mp4")] + second, _ = _invoke(tmp_path, flow, links=3, args=resume) + assert second.exit_code == 21, second.output + assert _rows(tmp_path, chain_id) == [(0, clips[0]), (1, clips[1])] + + flow.fail = set() + third, _ = _invoke(tmp_path, flow, links=3, args=resume) + assert third.exit_code == 0, third.output + assert [link["index"] for link in json.loads(third.stdout)["links"]] == [2] + assert _rows(tmp_path, chain_id) == list(enumerate(clips)) + assert all(p.is_file() for p in clips) + + assert flow.calls == [ + ("link 0", Mode.T2V, None, True), + ("link 1", Mode.I2V, "clip_1.mp4", False), + ("link 1", Mode.I2V, "clip_1.mp4", True), # resumed: i2v, seeded from link 1's clip + ("link 2", Mode.I2V, "clip_2.mp4", False), + ("link 2", Mode.I2V, "clip_2.mp4", True), + ] + billed = [prompt for prompt, _, _, ok in flow.calls if ok] + assert billed == ["link 0", "link 1", "link 2"] # no link generated twice + + +def test_a_resume_whose_seed_clip_is_gone_is_refused_before_submit(tmp_path: Path) -> None: + """#1300: with the last paid clip missing, the next link cannot be seeded; never t2v.""" + from gflow_cli.chain import ChainLinkResult + from gflow_cli.config import get_settings + from gflow_cli.data.chain_repo import ChainLinkRecorder + + missing = tmp_path / "dl" / "m0.mp4" + recorder = ChainLinkRecorder.open( + get_settings(), profile_name="default", profile_dir=tmp_path / "prof", chain_id="c1" ) + recorder.record_chain_link( + ChainLinkResult(index=0, prompt="link 0", local_path=missing, media_id="m0") + ) + recorder.close() + flow = _Flow(tmp_path) + + result, catalog = _invoke(tmp_path, flow, links=2, args=["--resume-from", "c1"]) - expected = [target.parent / "clip_2.mp4", target.parent / "clip_3.mp4"] - _assert_written_and_reported(result, chain_recorder, catalog, expected) - assert not (target.parent / "clip_1.mp4").exists() + assert result.exit_code == 20, result.output + assert str(missing) in json.loads(result.stdout)["error"]["detail"] + assert flow.calls == [] + catalog.flow_client_cls.assert_not_called() diff --git a/tests/features/test_video_chain_steps.py b/tests/features/test_video_chain_steps.py index 501777620..da077ff43 100644 --- a/tests/features/test_video_chain_steps.py +++ b/tests/features/test_video_chain_steps.py @@ -151,13 +151,20 @@ async def _boom(**_kwargs: Any) -> Any: chain_state["run_chain"] = mock_run +def _done_link(tmp_path: Path, index: int) -> MagicMock: + """A recorded link whose clip is on disk, so a resume can seed from it (#1300).""" + clip = tmp_path / f"done{index}.mp4" + clip.touch() + return MagicMock(link_index=index, local_path=str(clip)) + + @given(parsers.parse("the recorder reports {n:d} completed link")) @given(parsers.parse("the recorder reports {n:d} completed links")) def _recorder_completed( - chain_state: dict[str, Any], monkeypatch: pytest.MonkeyPatch, n: int + chain_state: dict[str, Any], monkeypatch: pytest.MonkeyPatch, tmp_path: Path, n: int ) -> None: recorder = MagicMock(name="ChainLinkRecorder") - recorder.completed_links.return_value = [MagicMock() for _ in range(n)] + recorder.completed_links.return_value = [_done_link(tmp_path, i) for i in range(n)] monkeypatch.setattr( "gflow_cli.data.chain_repo.ChainLinkRecorder.open", lambda *_a, **_kw: recorder ) @@ -167,16 +174,17 @@ def _recorder_completed( @given(parsers.parse("the recorder reports {n:d} completed link whose seed frame is absent")) def _recorder_completed_no_seed( - chain_state: dict[str, Any], monkeypatch: pytest.MonkeyPatch, n: int + chain_state: dict[str, Any], monkeypatch: pytest.MonkeyPatch, tmp_path: Path, n: int ) -> None: """Models the record-before-extract guarantee: the link's clip is recorded (so ``completed_links`` counts it and resume skips regeneration) even though its seed frame file is absent. Resume must therefore re-submit only the remaining links — the recorded link is NOT regenerated.""" recorder = MagicMock(name="ChainLinkRecorder") - completed_link = MagicMock() - completed_link.seed_frame_path = None # extraction never completed - recorder.completed_links.return_value = [completed_link for _ in range(n)] + completed = [_done_link(tmp_path, i) for i in range(n)] + for link in completed: + link.seed_frame_path = None # extraction never completed + recorder.completed_links.return_value = completed monkeypatch.setattr( "gflow_cli.data.chain_repo.ChainLinkRecorder.open", lambda *_a, **_kw: recorder ) diff --git a/tests/test_chain.py b/tests/test_chain.py index cedd324e2..94587e4e3 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -511,3 +511,52 @@ async def _gen(*, req: GenerateVideoRequest, **_: Any) -> VideoResult: failed_req, failed_exc = failed[0] assert failed_req.prompt == "the cat stretches and walks off" assert isinstance(failed_exc, WireFormatError) + + +async def test_a_resumed_link_keeps_its_manifest_index_and_is_seeded(tmp_path: Path) -> None: + """#1300: link 2 of a resumed chain is link 2, an i2v seeded from link 1's clip.""" + prev_clip = tmp_path / "link1.mp4" + prev_clip.write_bytes(b"\x00\x00\x00\x18ftypmp42") + client = _make_client([_ok_result("m2", tmp_path / "link2.mp4")]) + recorder = MagicMock() + extracted: list[tuple[Path, Path]] = [] + + def _extract(src: Path, dst: Path, *, offset_ms: int = 0) -> Path: + dst.write_bytes(b"\xff\xd8\xff\xd9") + extracted.append((src, dst)) + return dst + + out = await run_chain( + client=client, + links=[ChainLinkSpec(prompt="p2")], + out_dir=tmp_path, + model=VideoModel.VEO_3_1_LITE, + extractor=_extract, + recorder=recorder, + start_index=2, + seed_clip=prev_clip, + ) + + req = client.generate_video.await_args.kwargs["req"] + assert req.mode == Mode.I2V + assert extracted == [(prev_clip, tmp_path / "link1_lastframe.jpg")] + assert req.start_image == tmp_path / "link1_lastframe.jpg" + assert [r.index for r in out] == [2] + assert recorder.record_chain_link.call_args.args[0].index == 2 + assert out[0].frame_path is None # still the final link + + +async def test_a_resumed_link_without_a_seed_is_refused_not_sent_as_t2v(tmp_path: Path) -> None: + """#1300: an index past 0 with nothing to seed it must not fall back to t2v.""" + client = _make_client([_ok_result("m1", tmp_path / "link1.mp4")]) + + with pytest.raises(ValueError, match="seed_clip"): + await run_chain( + client=client, + links=[ChainLinkSpec(prompt="p1")], + out_dir=tmp_path, + model=VideoModel.VEO_3_1_LITE, + extractor=_fake_extractor([]), + start_index=1, + ) + client.generate_video.assert_not_awaited() From b8aa57c32a0295627d160ab489a591b5670917af Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:09:52 +0000 Subject: [PATCH 05/10] fix(video chain): --out-file never replaces an existing file or folder (#1293 review) A second run with the same --out-file moved its clips over the first run's. An existing _ now keeps the new clip at its download path, recorded there. --- src/gflow_cli/cli_video.py | 2 ++ tests/cli/test_cli_video_chain_out_file.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index 903fad447..9aa26ee85 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -782,6 +782,8 @@ def _chain_clip_relocator( def relocate(index: int, result: VideoResult) -> VideoResult: target = numbered_path(output_file, index + 1) try: + if target.exists(): # an earlier run's clip, or a folder: never replace it + raise FileExistsError(f"{target} already exists") return relocate_video(result, target) except OSError as exc: # billed already: keep and record the clip where it is logger.warning("chain_out_file_move_failed", target=str(target), error=str(exc)) diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py index 2be5f4467..36ea11dd2 100644 --- a/tests/cli/test_cli_video_chain_out_file.py +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -190,3 +190,23 @@ def test_a_resume_whose_seed_clip_is_gone_is_refused_before_submit(tmp_path: Pat assert str(missing) in json.loads(result.stdout)["error"]["detail"] assert flow.calls == [] catalog.flow_client_cls.assert_not_called() + + +def test_an_existing_target_is_never_overwritten(tmp_path: Path) -> None: + """A second run with the same --out-file keeps the old files and its own clips.""" + flow = _Flow(tmp_path) + out = tmp_path / "out" + out.mkdir() + (out / "clip_1.mp4").write_bytes(b"earlier run") + (out / "clip_2.mp4").mkdir() + + result, catalog = _invoke(tmp_path, flow, links=2, args=["--out-file", str(out / "clip.mp4")]) + + assert result.exit_code == 0, result.output + assert (out / "clip_1.mp4").read_bytes() == b"earlier run" + kept = [flow.download_dir / "m0.mp4", flow.download_dir / "m1.mp4"] + assert all(p.is_file() for p in kept) + payload = json.loads(result.stdout) + assert [Path(link["local_path"]) for link in payload["links"]] == kept + assert _rows(tmp_path, payload["chain_id"]) == list(enumerate(kept)) + assert _catalog_paths(catalog) == kept From 3543d4073e7a4d737a8d8a7edd170483a4eeadf5 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:10:58 +0000 Subject: [PATCH 06/10] fix(video chain): guard the --out-file move like the failure hook (#1293 review) Any exception from the relocate hook escaped run_chain before the link was recorded, leaving a paid clip without a chain row. The move now runs off the event loop, a failure keeps the clip at its download path, recorded there, with a warning naming the link number, media id and where the clip stayed, and a partial copy the move left is removed. --- src/gflow_cli/chain.py | 33 +++++++++++++--- src/gflow_cli/cli_video.py | 13 ++++--- tests/cli/test_cli_video_chain_out_file.py | 45 +++++++++++++++++++++- tests/test_chain.py | 29 ++++++++++++++ 4 files changed, 108 insertions(+), 12 deletions(-) diff --git a/src/gflow_cli/chain.py b/src/gflow_cli/chain.py index d7f0509cd..324f55b8d 100644 --- a/src/gflow_cli/chain.py +++ b/src/gflow_cli/chain.py @@ -325,6 +325,28 @@ def _build_link_result( ) +async def _relocate_guarded( + relocate: Callable[[int, VideoResult], VideoResult], index: int, result: VideoResult +) -> VideoResult: + """Run the ``--out-file`` move off the loop; on any failure keep the clip where it is. + + The clip is already paid for, so it must reach the recorder whatever the hook does: a + link without a chain row is billed again on ``--resume-from``. Same rule as + :func:`_notify_link_failed`. + """ + try: + return await asyncio.to_thread(relocate, index, result) + except Exception as exc: # noqa: BLE001 — see docstring + _log.warning( + "chain_clip_kept_at_download_path", + link=index + 1, + media_id=result.status.media_id, + kept_at=str(result.local_path), + reason=str(exc), + ) + return result + + async def _resume_seed( extractor: FrameExtractor, *, @@ -390,10 +412,11 @@ async def run_chain( jitter: When > 0, sleep a random ``[0, jitter)`` seconds BETWEEN links (anti-bot cadence); never before link 0. relocate: Optional ``(index, result) -> result`` that moves a downloaded - clip (``--out-file``, #1293). It runs before anything reads the - path, so the recorders, the seed extraction and the return value - all see where the clip ended up. A result with no ``local_path`` - must come back unchanged. + clip (``--out-file``, #1293). It runs off the event loop before + anything reads the path, so the recorders, the seed extraction and + the return value all see where the clip ended up. If it raises, the + clip stays at its download path and a warning says so. A result with + no ``local_path`` must come back unchanged. start_index: The manifest index of ``links[0]`` (``--resume-from``, #1300). Request mode, ``ChainLinkResult.index`` and the recorded ``link_index`` all count from it. seed_clip: The clip of link ``start_index - 1``; its last frame seeds ``links[0]``. @@ -444,7 +467,7 @@ async def run_chain( on_link_failed=on_link_failed, ) if relocate is not None: # a missing clip is left as is and refused just below - result = relocate(index, result) + result = await _relocate_guarded(relocate, index, result) if result.local_path is None: msg = f"link {index} returned no local_path (download failed)" diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index 9aa26ee85..de7e44ee9 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -773,7 +773,8 @@ def _chain_clip_relocator( """``--out-file`` for a chain (#1293): link ``index`` goes to ``_``. ``index`` counts the whole manifest (#1300), so a resumed run names its links as a full - run would. Always numbered, even for one link, so a resume never renames a clip. + run would. Always numbered, even for one link, so a resume never renames a clip. Raises + instead of replacing an existing target; ``run_chain`` then keeps the clip where it is. """ if output_file is None: return None @@ -781,13 +782,13 @@ def _chain_clip_relocator( def relocate(index: int, result: VideoResult) -> VideoResult: target = numbered_path(output_file, index + 1) + if target.exists(): # an earlier run's clip, or a folder: never replace it + raise FileExistsError(f"{target} already exists") try: - if target.exists(): # an earlier run's clip, or a folder: never replace it - raise FileExistsError(f"{target} already exists") return relocate_video(result, target) - except OSError as exc: # billed already: keep and record the clip where it is - logger.warning("chain_out_file_move_failed", target=str(target), error=str(exc)) - return result + except OSError: + target.unlink(missing_ok=True) # a partial copy: the check above saw no file here + raise return relocate diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py index 36ea11dd2..68115ee8a 100644 --- a/tests/cli/test_cli_video_chain_out_file.py +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -192,7 +192,9 @@ def test_a_resume_whose_seed_clip_is_gone_is_refused_before_submit(tmp_path: Pat catalog.flow_client_cls.assert_not_called() -def test_an_existing_target_is_never_overwritten(tmp_path: Path) -> None: +def test_an_existing_target_is_never_overwritten( + tmp_path: Path, _capture: structlog.testing.LogCapture +) -> None: """A second run with the same --out-file keeps the old files and its own clips.""" flow = _Flow(tmp_path) out = tmp_path / "out" @@ -210,3 +212,44 @@ def test_an_existing_target_is_never_overwritten(tmp_path: Path) -> None: assert [Path(link["local_path"]) for link in payload["links"]] == kept assert _rows(tmp_path, payload["chain_id"]) == list(enumerate(kept)) assert _catalog_paths(catalog) == kept + warnings = [e for e in _capture.entries if e["event"] == _KEPT_EVENT] + assert [(w["link"], w["media_id"], w["kept_at"]) for w in warnings] == [ + (1, "m0", str(kept[0])), + (2, "m1", str(kept[1])), + ] + + +def test_a_failed_move_keeps_the_clip_and_removes_its_partial_copy( + tmp_path: Path, _capture: structlog.testing.LogCapture +) -> None: + """A move that dies mid-copy leaves no half file, and the paid clip stays recorded.""" + import shutil + + real_move = shutil.move + calls = 0 + + def _move(src: Any, dst: Any) -> Any: + nonlocal calls + calls += 1 + if calls == 2: + Path(dst).write_bytes(b"half") + raise OSError(28, "No space left on device") + return real_move(src, dst) + + flow = _Flow(tmp_path) + out = tmp_path / "out" + with patch("shutil.move", _move): + result, catalog = _invoke( + tmp_path, flow, links=2, args=["--out-file", str(out / "clip.mp4")] + ) + + assert result.exit_code == 0, result.output + assert not (out / "clip_2.mp4").exists() + expected = [out / "clip_1.mp4", flow.download_dir / "m1.mp4"] + payload = json.loads(result.stdout) + assert _rows(tmp_path, payload["chain_id"]) == list(enumerate(expected)) + assert _catalog_paths(catalog) == expected + warnings = [e for e in _capture.entries if e["event"] == _KEPT_EVENT] + assert [(w["link"], w["media_id"], w["kept_at"]) for w in warnings] == [ + (2, "m1", str(expected[1])) + ] diff --git a/tests/test_chain.py b/tests/test_chain.py index 94587e4e3..a54ad8bfc 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -560,3 +560,32 @@ async def test_a_resumed_link_without_a_seed_is_refused_not_sent_as_t2v(tmp_path start_index=1, ) client.generate_video.assert_not_awaited() + + +async def test_a_raising_relocate_leaves_the_downloaded_clip_recorded(tmp_path: Path) -> None: + """#1293 review: the relocate hook runs off the loop, and any error it raises is + absorbed so the paid clip is still recorded and returned at its download path.""" + import threading + + clip = tmp_path / "link0.mp4" + client = _make_client([_ok_result("m0", clip)]) + recorder = MagicMock() + threads: list[int] = [] + + def _relocate(index: int, result: VideoResult) -> VideoResult: + threads.append(threading.get_ident()) + raise RuntimeError("relocate bug") + + out = await run_chain( + client=client, + links=[ChainLinkSpec(prompt="p0")], + out_dir=tmp_path, + model=VideoModel.VEO_3_1_LITE, + extractor=_fake_extractor([]), + recorder=recorder, + relocate=_relocate, + ) + + assert threads and threads[0] != threading.get_ident(), "relocate ran on the event loop" + assert [r.local_path for r in out] == [clip] + assert recorder.record_chain_link.call_args.args[0].local_path == clip From 8e539a74eb2db0d208a5071125e95ec03f6e0197 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:11:55 +0000 Subject: [PATCH 07/10] fix(video chain): refuse an --out-file under an existing file before anything runs (#1293 review) The clips' folder was first created after the cost prompt and the browser boot, so an --out-file whose parent is a file died as exit 1 with Chrome already up. It is now a usage error (exit 2) up front, in --dry-run too. --- src/gflow_cli/cli_video.py | 5 +++++ tests/cli/test_cli_video_chain_out_file.py | 24 ++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index de7e44ee9..e38a6d48e 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -1824,6 +1824,11 @@ def chain( as_json: bool, ) -> None: """Render the chain MANIFEST (one continuous last-frame I2V sequence).""" + # Before the cost prompt and the browser: the clips' folder must be creatable. + blocker = next((p for p in output_file.parents if p.exists()), None) if output_file else None + if blocker is not None and not blocker.is_dir(): + msg = f"{blocker} is a file, so {output_file} cannot be written under it." + raise click.BadParameter(msg, param_hint="'--out-file'") profile_name = _resolve_profile(profile) provider_dir = _make_provider_dir(profile_name) run_with_handlers( diff --git a/tests/cli/test_cli_video_chain_out_file.py b/tests/cli/test_cli_video_chain_out_file.py index 68115ee8a..e27966266 100644 --- a/tests/cli/test_cli_video_chain_out_file.py +++ b/tests/cli/test_cli_video_chain_out_file.py @@ -253,3 +253,27 @@ def _move(src: Any, dst: Any) -> Any: assert [(w["link"], w["media_id"], w["kept_at"]) for w in warnings] == [ (2, "m1", str(expected[1])) ] + + +@pytest.mark.parametrize("gate", ["--yes", "--dry-run"]) +def test_an_out_file_under_a_file_is_a_usage_error_before_anything_runs( + tmp_path: Path, gate: str +) -> None: + blocker = tmp_path / "not_a_dir" + blocker.write_text("x") + manifest = tmp_path / "chain.jsonl" + manifest.write_text('{"prompt": "a"}\n{"prompt": "b"}\n', encoding="utf-8") + + with ( + patch("gflow_cli.cli_video._resolve_profile", return_value="default"), + patch("gflow_cli.cli_video._make_provider_dir", return_value=tmp_path / "prof"), + patch("gflow_cli.cli_video.FlowApiClient") as client_cls, + ): + result = CliRunner().invoke( + video, ["chain", str(manifest), gate, "--out-file", str(blocker / "clip.mp4")] + ) + + assert result.exit_code == 2, result.output + assert "--out-file" in result.stderr + assert str(blocker) in result.stderr + client_cls.assert_not_called() From bdca1b4b1b15a44f08de0217e71b37e3e3b7e0ad Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:12:26 +0000 Subject: [PATCH 08/10] docs(video chain): --out-dir/--out-file roles, kept clips and #1300 resume (#1293 review) The --out-dir help and CONFIGURATION row still said the link mp4s land there, USAGE's single-file note read as covering chain, and PROJECT_STATUS still said the file was not written. Adds the #1300 fix to the CHANGELOG entry. --- CHANGELOG.md | 14 +++++++++++--- docs/CONFIGURATION.md | 6 +++--- docs/PROJECT_STATUS.md | 4 ++-- docs/USAGE.md | 7 ++++--- src/gflow_cli/cli_video.py | 2 +- website/docs/CONFIGURATION.md | 6 +++--- website/docs/USAGE.md | 7 ++++--- 7 files changed, 28 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bae1d2642..298199564 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 accepted and ignored. Link i's clip goes to `_` next to PATH (`out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each link's name; `--json`, the chain table and the videos catalog report - those paths, and seed frames stay under `--out-dir`. A clip that cannot be moved there stays - where it downloaded and is recorded at that path, with a warning, so `--resume-from` does not - bill it again. The stderr notice is gone + those paths, and seed frames stay under `--out-dir`. An existing file or folder at a numbered + path is never replaced, and a clip whose move fails is never lost: either way it stays where it + downloaded, is recorded there, and a warning names the link, its media id and the path. A + FILE under an existing file is a usage error before the cost prompt. The stderr notice is gone ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). +- **`video chain --resume-from` continues the chain instead of restarting it.** The first resumed + link was numbered 0: it overwrote link 0's chain row, went out as an unseeded text-to-video, + and left the chain one row short, so a later resume billed a finished link again. Resumed links + now keep their manifest index (chain table, `--json` `links[].index`, `--out-file` number), the + first one is an image-to-video seeded from the last completed clip, and a resume whose clip is + gone is refused with exit 20 naming the file, before anything is billed + ([#1300](https://github.com/ffroliva/gflow-cli/issues/1300)). ## [0.99.0] — 2026-11-14 diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index b55ca1955..393aefff9 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -639,12 +639,12 @@ waits for each link in turn, so total wallclock is the sum of all link waits. | `--max-links N` | unset | Cap link count; exit 11 (`ConfigurationError`) if the manifest has more. A spend guardrail. | | `-y` / `--yes` | off | Skip the pending video operation confirmation prompt. | | `--dry-run` | off | Print the pending video operation plan and submit nothing. | -| `--resume-from CHAIN_ID` | unset | Resume a prior chain by its id; already-completed links are skipped (not regenerated). | +| `--resume-from CHAIN_ID` | unset | Resume a prior chain by its id; already-completed links are skipped (not regenerated), and the next link is an i2v seeded from the last completed clip, which must still be at its recorded path (exit 20 otherwise, before anything is billed; [#1300](https://github.com/ffroliva/gflow-cli/issues/1300)). | | `--jitter F` | `0.0` | Random `0..F` second pause **between** links (anti-bot cadence; never before link 0). | | `--seed-offset MS` | `0` | Extract the seed frame this many ms before EOF (fade-to-black guard). | | `--aspect` | `9:16` | `9:16` / `16:9`. Applied uniformly to every link (continuity requirement). | -| `--out-file FILE` | unset | Link i's clip goes to `_` next to FILE (`clip.mp4` gives `clip_1.mp4`, `clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each name; seed frames stay under `--out-dir`. A clip that cannot be moved stays where it downloaded, with a warning. Deprecated spelling: `-o/--output` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). | -| `--out-dir DIR` | output root | Directory for the link mp4s + `linkN_lastframe.jpg` seed frames. | +| `--out-file FILE` | unset | Link i's clip goes to `_` next to FILE (`clip.mp4` gives `clip_1.mp4`, `clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each name; seed frames stay under `--out-dir`. An existing file or folder there is never replaced: that clip, like one whose move fails, stays where it downloaded, recorded there, with a warning. A FILE under an existing file is a usage error (exit 2) before anything runs. Deprecated spelling: `-o/--output` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). | +| `--out-dir DIR` | output root | Directory for the `linkN_lastframe.jpg` seed frames, and for the link mp4s unless `--out-file` is given. | | `--profile NAME` | default profile | Per-subcommand profile override. | | `--json` | off | Emit a machine-readable JSON result. | diff --git a/docs/PROJECT_STATUS.md b/docs/PROJECT_STATUS.md index 3720eadb7..fad82f5e2 100644 --- a/docs/PROJECT_STATUS.md +++ b/docs/PROJECT_STATUS.md @@ -20,8 +20,8 @@ rate limit, content policy, network, wire format and timeout to `40`–`44` and into `3`. The default stays the legacy codes; `--json` errors carry both `exit_code` and `exit_code_legacy`. The table is in [ERRORS.md](ERRORS.md). -**Changed.** `video chain` has never written to `-o PATH`; it now says so on stderr instead of staying -silent (#1293 tracks writing the file). +**Changed.** In this release `video chain` does not write to `-o PATH` and says so on stderr instead of +staying silent; writing the file is fixed on `develop` for the next release (#1293). Internally, `UiAutomationTransport` and `MigratedComposer` are split into single-purpose mixins (#1249 wave 2). diff --git a/docs/USAGE.md b/docs/USAGE.md index 5723a9420..887d220bd 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -699,8 +699,8 @@ All prompts in a batch share one Flow project. The editor is opened once and sta > instead of proceeding on Flow's sticky default (typically x2) and silently > changing what the run bills. > `t2v`, `i2v`, `r2v`, `video chain`, and `video extend` additionally take `--out-file PATH` (explicit file destination). -> `video chain` writes link i's clip to `_` next to PATH (`--out-file out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`, …), numbered over the whole manifest even for one link, so `--resume-from` writes the remaining links under the names a full run would; seed frames stay under `--out-dir`, and `--json` and the catalog report the numbered paths ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). -> The mp4 lands at `` or `/.mp4`. +> `video chain` writes link i's clip to `_` next to PATH (`--out-file out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`, …), numbered over the whole manifest even for one link, so `--resume-from` writes the remaining links under the names a full run would; seed frames stay under `--out-dir`, and `--json` and the catalog report the numbered paths. An existing file or folder at a numbered path is never replaced: that clip, like one whose move fails, stays at its `--out-dir` path, is recorded there, and a warning names it ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). +> For `t2v`, `i2v` and `r2v` the mp4 lands at `` or `/.mp4`. > > **`i2v` model rules (issues #125 / #626):** every model — `omni-flash` > included — supports i2v with a **start frame** and with an **end frame** @@ -1148,7 +1148,8 @@ Options: --profile NAME Profile name (overrides default). --out-file FILE Write link i's clip to FILE's stem plus _i (clip.mp4 -> clip_1.mp4, clip_2.mp4). - --out-dir DIR Directory for the link mp4s + seed frames. + --out-dir DIR Directory for the seed frames, and for the link + mp4s unless --out-file is given. --json Emit a machine-readable JSON result. ``` diff --git a/src/gflow_cli/cli_video.py b/src/gflow_cli/cli_video.py index e38a6d48e..e61e6c187 100644 --- a/src/gflow_cli/cli_video.py +++ b/src/gflow_cli/cli_video.py @@ -1799,7 +1799,7 @@ def r2v( "out_dir", default=None, type=click.Path(file_okay=False, path_type=Path), - help="Directory to save the link mp4s + seed frames. Defaults to the output dir.", + help="Directory for the seed frames, and the link mp4s unless --out-file is given.", ) @click.option( "--json", diff --git a/website/docs/CONFIGURATION.md b/website/docs/CONFIGURATION.md index c1ca3d68c..c96b8d920 100644 --- a/website/docs/CONFIGURATION.md +++ b/website/docs/CONFIGURATION.md @@ -639,12 +639,12 @@ waits for each link in turn, so total wallclock is the sum of all link waits. | `--max-links N` | unset | Cap link count; exit 11 (`ConfigurationError`) if the manifest has more. A spend guardrail. | | `-y` / `--yes` | off | Skip the pending video operation confirmation prompt. | | `--dry-run` | off | Print the pending video operation plan and submit nothing. | -| `--resume-from CHAIN_ID` | unset | Resume a prior chain by its id; already-completed links are skipped (not regenerated). | +| `--resume-from CHAIN_ID` | unset | Resume a prior chain by its id; already-completed links are skipped (not regenerated), and the next link is an i2v seeded from the last completed clip, which must still be at its recorded path (exit 20 otherwise, before anything is billed; [#1300](https://github.com/ffroliva/gflow-cli/issues/1300)). | | `--jitter F` | `0.0` | Random `0..F` second pause **between** links (anti-bot cadence; never before link 0). | | `--seed-offset MS` | `0` | Extract the seed frame this many ms before EOF (fade-to-black guard). | | `--aspect` | `9:16` | `9:16` / `16:9`. Applied uniformly to every link (continuity requirement). | -| `--out-file FILE` | unset | Link i's clip goes to `_` next to FILE (`clip.mp4` gives `clip_1.mp4`, `clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each name; seed frames stay under `--out-dir`. A clip that cannot be moved stays where it downloaded, with a warning. Deprecated spelling: `-o/--output` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). | -| `--out-dir DIR` | output root | Directory for the link mp4s + `linkN_lastframe.jpg` seed frames. | +| `--out-file FILE` | unset | Link i's clip goes to `_` next to FILE (`clip.mp4` gives `clip_1.mp4`, `clip_2.mp4`), numbered over the whole manifest so `--resume-from` keeps each name; seed frames stay under `--out-dir`. An existing file or folder there is never replaced: that clip, like one whose move fails, stays where it downloaded, recorded there, with a warning. A FILE under an existing file is a usage error (exit 2) before anything runs. Deprecated spelling: `-o/--output` ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). | +| `--out-dir DIR` | output root | Directory for the `linkN_lastframe.jpg` seed frames, and for the link mp4s unless `--out-file` is given. | | `--profile NAME` | default profile | Per-subcommand profile override. | | `--json` | off | Emit a machine-readable JSON result. | diff --git a/website/docs/USAGE.md b/website/docs/USAGE.md index 7864e7f01..4ebddb08a 100644 --- a/website/docs/USAGE.md +++ b/website/docs/USAGE.md @@ -699,8 +699,8 @@ All prompts in a batch share one Flow project. The editor is opened once and sta > instead of proceeding on Flow's sticky default (typically x2) and silently > changing what the run bills. > `t2v`, `i2v`, `r2v`, `video chain`, and `video extend` additionally take `--out-file PATH` (explicit file destination). -> `video chain` writes link i's clip to `_` next to PATH (`--out-file out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`, …), numbered over the whole manifest even for one link, so `--resume-from` writes the remaining links under the names a full run would; seed frames stay under `--out-dir`, and `--json` and the catalog report the numbered paths ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). -> The mp4 lands at `` or `/.mp4`. +> `video chain` writes link i's clip to `_` next to PATH (`--out-file out/clip.mp4` gives `out/clip_1.mp4`, `out/clip_2.mp4`, …), numbered over the whole manifest even for one link, so `--resume-from` writes the remaining links under the names a full run would; seed frames stay under `--out-dir`, and `--json` and the catalog report the numbered paths. An existing file or folder at a numbered path is never replaced: that clip, like one whose move fails, stays at its `--out-dir` path, is recorded there, and a warning names it ([#1293](https://github.com/ffroliva/gflow-cli/issues/1293)). +> For `t2v`, `i2v` and `r2v` the mp4 lands at `` or `/.mp4`. > > **`i2v` model rules (issues #125 / #626):** every model — `omni-flash` > included — supports i2v with a **start frame** and with an **end frame** @@ -1148,7 +1148,8 @@ Options: --profile NAME Profile name (overrides default). --out-file FILE Write link i's clip to FILE's stem plus _i (clip.mp4 -> clip_1.mp4, clip_2.mp4). - --out-dir DIR Directory for the link mp4s + seed frames. + --out-dir DIR Directory for the seed frames, and for the link + mp4s unless --out-file is given. --json Emit a machine-readable JSON result. ``` From fbbbbf7c053c97b22cefc250a00474f9171f680b Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:12:53 +0000 Subject: [PATCH 09/10] test(e2e): video chain --out-file through the CLI (#1293, #1300) The chain e2e calls run_chain directly, so it cannot see --out-file. The new e2e_video test drives video chain over a 2-link manifest and checks clip_1.mp4 and clip_2.mp4 are mp4s, --json reports exactly those paths, and link 2 was recorded as i2v. Collected, not run here: it spends Veo credits. --- tests/e2e/test_chain_e2e.py | 73 +++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/e2e/test_chain_e2e.py b/tests/e2e/test_chain_e2e.py index 3ad18a4ca..946952021 100644 --- a/tests/e2e/test_chain_e2e.py +++ b/tests/e2e/test_chain_e2e.py @@ -195,3 +195,76 @@ async def test_chain_two_link_seeds_i2v_from_last_frame( f"its start frame through the media dialog (i2v routed to ...StartImage, " f"not ...Text — issue #125); captured events: {events}" ) + + +def test_chain_cli_out_file_writes_numbered_mp4s_and_seeds_link_2( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """CHAIN-E2E-2 (#1293, #1300): the CLI, not ``run_chain``, honours ``--out-file``. + + ``gflow video chain <2-link manifest> --out-file /out/clip.mp4 --yes --json`` + writes ``clip_1.mp4`` and ``clip_2.mp4`` as mp4 files, ``--json`` reports exactly + those paths, and the catalog recorded link 2 as an i2v (a misrouted seeded link + aborts the chain with ``WireFormatError``, so exit 0 is itself route evidence). + + Spends ~2 Veo credits per run. + """ + import json + import sqlite3 + + from click.testing import CliRunner + + from gflow_cli.cli import main + from gflow_cli.config import reset_settings + from gflow_cli.paths import looks_like_video + + db = tmp_path / "e2e_chain_cli.db" + monkeypatch.delenv("GFLOW_CLI_HOME", raising=False) + monkeypatch.setenv("GFLOW_CLI_DB_PATH", str(db)) + reset_settings() + + name = os.environ.get("GFLOW_CLI_E2E_PROFILE", "").strip() + if not name: + pytest.skip("set GFLOW_CLI_E2E_PROFILE to a logged-in profile, then run with -m e2e_video") + from gflow_cli.auth import profile_dir as _resolve_profile_dir + + if not _resolve_profile_dir(name).exists(): + pytest.skip(f"profile not found for {name!r} — run `gflow auth login --profile {name}`") + + _model() # skips on an alias chains refuse + model_alias = os.environ.get(_E2E_MODEL_ENV, "veo-lite").strip().lower() + ratio = "9:16" if _aspect() is Aspect.PORTRAIT else "16:9" + manifest = tmp_path / "chain.jsonl" + manifest.write_text( + "\n".join(json.dumps({"prompt": p}) for p in (_LINK0_PROMPT, _LINK1_PROMPT)), + encoding="utf-8", + ) + out = tmp_path / "out" / "clip.mp4" + + result = CliRunner().invoke( + main, + [ + "video", "chain", str(manifest), + "--profile", name, "--model", model_alias, "--aspect", ratio, + "--out-dir", str(tmp_path / "dl"), "--out-file", str(out), + "--yes", "--json", + ], + ) # fmt: skip + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout[result.stdout.index("{\n") :]) + expected = [out.parent / "clip_1.mp4", out.parent / "clip_2.mp4"] + assert [Path(link["local_path"]) for link in payload["links"]] == expected + assert [Path(p) for p in payload["completed_paths"]] == expected + for clip in expected: + assert clip.is_file(), f"--out-file clip missing: {clip}" + assert looks_like_video(clip.read_bytes()[:16]), f"not an mp4: {clip}" + + with sqlite3.connect(db) as conn: + modes = [ + row[0] + for row in conn.execute( + "SELECT mode FROM operations WHERE command = 'video chain' ORDER BY started_at" + ) + ] + assert modes == ["t2v", "i2v"], f"link 2 must go out as i2v; catalog modes: {modes}" From 68c47c5744aaffe64cf2ce68f3b33ca943e02a68 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 14 Nov 2026 23:17:22 +0000 Subject: [PATCH 10/10] fix(e2e, docs): chain e2e reads i2v from the real catalog; document exit 20 on resume (#1300) Chain links record command 'video t2v'/'video i2v', so the e2e's filter on 'video chain' matched nothing and would have failed after billing (checked at $0 against a real catalog). The database holds that run only, so the filter is dropped. USAGE now names the refused resume under exit 20 and on --resume-from. --- docs/USAGE.md | 6 ++++-- tests/e2e/test_chain_e2e.py | 6 ++---- website/docs/USAGE.md | 6 ++++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 887d220bd..9237957b0 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1139,7 +1139,9 @@ Options: --dry-run Resolve the manifest, print the plan + credit cost, and spend nothing. --resume-from CHAIN_ID Resume a prior chain by its id; already-paid links - are skipped (not re-billed). + are skipped (not re-billed), and the next link is + seeded from the last paid clip, which must still be + at its recorded path (exit 20 otherwise). --jitter FLOAT Random 0..JITTER second pause between links (anti-bot cadence). [default: 0.0] --seed-offset INTEGER Extract the seed frame this many ms before EOF @@ -2312,7 +2314,7 @@ The table below is the default (legacy) set. `GFLOW_CLI_EXIT_CODES=2` opts in to | `17` | `ModelModeIncompatibilityError` | The chosen video model can't do the requested mode — today that is `omni-flash` or `omni-1.2-flash` for `chain` (issues #125, #626) | Use a Veo 3.1 model (`veo-lite` / `veo-fast` / `veo-quality` / `veo-lite-lp`) for `chain`. Single-clip `i2v` with omni-flash, `--end-frame` included, is accepted | | `18` | `VideoModelSelectionError` | gflow could not select the requested video model in Flow's editor for an `i2v` run (model-picker option not found) | Usually transient — retry; if it persists, Flow's model-picker UI changed (report referencing #125) | | `19` | `SceneConcatError` | Server-side scene render/concat failed (`gflow scene --out-file`) | Retry; the recorded compose survives, so re-render is safe | -| `20` | `FrameExtractionError` | Could not extract the last frame for a video chain link | Check the source video downloaded intact; retry the link | +| `20` | `FrameExtractionError` | Could not extract the last frame for a video chain link, or a `video chain --resume-from` whose last completed clip is no longer at its recorded path (refused before anything is billed, [#1300](https://github.com/ffroliva/gflow-cli/issues/1300)) | Check the source video downloaded intact; retry the link. For a resume, put the clip back at the path named, or start a new chain | | `21` | `ChainPartialError` | A video chain stopped mid-way; earlier links completed | Resume from the last completed link shown in the error | | `22` | `UpscaleUnavailableError` | 4K image upscale is gated to Flow **Ultra** accounts; or requested video export quality is disabled | For images, use `--scale 2k` or upgrade the Flow plan; for videos, check available options in Flow | | `23` | `UiSelectorDriftError` | A Flow editor control could not be located — Google changed the frontend (issues #183, #493) — **or** a blocking announcement overlay survived dismissal, so no control below it can be clicked (probe `overlay_close_button`, #593). On the migrated host a missing submit control is checked against the wallet first and reported as **37** when Flow swapped in its insufficient-credits warning, so a short balance no longer arrives here | If the detail says the model picker is **hidden**, nothing was submitted: turn the **Agent** chip off in a browser and re-run ([#788](https://github.com/ffroliva/gflow-cli/issues/788)). Otherwise update gflow-cli; file a bug with the probe name + the diagnostics JSON / debug screenshot referenced in the error message, plus the incident bundle's `report.md` when one was written. For `overlay_close_button`: open the project once in Chrome and dismiss the announcement — the dismissal persists on your account | diff --git a/tests/e2e/test_chain_e2e.py b/tests/e2e/test_chain_e2e.py index 946952021..d66f609b5 100644 --- a/tests/e2e/test_chain_e2e.py +++ b/tests/e2e/test_chain_e2e.py @@ -260,11 +260,9 @@ def test_chain_cli_out_file_writes_numbered_mp4s_and_seeds_link_2( assert clip.is_file(), f"--out-file clip missing: {clip}" assert looks_like_video(clip.read_bytes()[:16]), f"not an mp4: {clip}" + # The database holds this run only; chain links record command "video t2v"/"video i2v". with sqlite3.connect(db) as conn: modes = [ - row[0] - for row in conn.execute( - "SELECT mode FROM operations WHERE command = 'video chain' ORDER BY started_at" - ) + row[0] for row in conn.execute("SELECT mode FROM operations ORDER BY started_at, rowid") ] assert modes == ["t2v", "i2v"], f"link 2 must go out as i2v; catalog modes: {modes}" diff --git a/website/docs/USAGE.md b/website/docs/USAGE.md index 4ebddb08a..be5194460 100644 --- a/website/docs/USAGE.md +++ b/website/docs/USAGE.md @@ -1139,7 +1139,9 @@ Options: --dry-run Resolve the manifest, print the plan + credit cost, and spend nothing. --resume-from CHAIN_ID Resume a prior chain by its id; already-paid links - are skipped (not re-billed). + are skipped (not re-billed), and the next link is + seeded from the last paid clip, which must still be + at its recorded path (exit 20 otherwise). --jitter FLOAT Random 0..JITTER second pause between links (anti-bot cadence). [default: 0.0] --seed-offset INTEGER Extract the seed frame this many ms before EOF @@ -2312,7 +2314,7 @@ The table below is the default (legacy) set. `GFLOW_CLI_EXIT_CODES=2` opts in to | `17` | `ModelModeIncompatibilityError` | The chosen video model can't do the requested mode — today that is `omni-flash` or `omni-1.2-flash` for `chain` (issues #125, #626) | Use a Veo 3.1 model (`veo-lite` / `veo-fast` / `veo-quality` / `veo-lite-lp`) for `chain`. Single-clip `i2v` with omni-flash, `--end-frame` included, is accepted | | `18` | `VideoModelSelectionError` | gflow could not select the requested video model in Flow's editor for an `i2v` run (model-picker option not found) | Usually transient — retry; if it persists, Flow's model-picker UI changed (report referencing #125) | | `19` | `SceneConcatError` | Server-side scene render/concat failed (`gflow scene --out-file`) | Retry; the recorded compose survives, so re-render is safe | -| `20` | `FrameExtractionError` | Could not extract the last frame for a video chain link | Check the source video downloaded intact; retry the link | +| `20` | `FrameExtractionError` | Could not extract the last frame for a video chain link, or a `video chain --resume-from` whose last completed clip is no longer at its recorded path (refused before anything is billed, [#1300](https://github.com/ffroliva/gflow-cli/issues/1300)) | Check the source video downloaded intact; retry the link. For a resume, put the clip back at the path named, or start a new chain | | `21` | `ChainPartialError` | A video chain stopped mid-way; earlier links completed | Resume from the last completed link shown in the error | | `22` | `UpscaleUnavailableError` | 4K image upscale is gated to Flow **Ultra** accounts; or requested video export quality is disabled | For images, use `--scale 2k` or upgrade the Flow plan; for videos, check available options in Flow | | `23` | `UiSelectorDriftError` | A Flow editor control could not be located — Google changed the frontend (issues #183, #493) — **or** a blocking announcement overlay survived dismissal, so no control below it can be clicked (probe `overlay_close_button`, #593). On the migrated host a missing submit control is checked against the wallet first and reported as **37** when Flow swapped in its insufficient-credits warning, so a short balance no longer arrives here | If the detail says the model picker is **hidden**, nothing was submitted: turn the **Agent** chip off in a browser and re-run ([#788](https://github.com/ffroliva/gflow-cli/issues/788)). Otherwise update gflow-cli; file a bug with the probe name + the diagnostics JSON / debug screenshot referenced in the error message, plus the incident bundle's `report.md` when one was written. For `overlay_close_button`: open the project once in Chrome and dismiss the announcement — the dismissal persists on your account |