[C1ph3r404]

Summary

• Wired MCP image/video generation tool placeholders to actual FlowWorker execution path via new _run_generation_task helper.
• Hooked up OperationRecorder and file download logic in FlowWorker.process_task to ensure generated assets are saved locally and recorded to SQLite.
• Refactored FlowWorker to reuse its active DataStore connection with OperationRecorder instead of creating new instances.
• Added support for profile pinning via --profile / GFLOW_CLI_PROFILE env var in MCP command.
• Refactored Chrome cookie snapshot decryption to avoid KeyError on Linux/macOS by only passing Local State path on Windows and handling keyring connection errors.
• Added 9 new integration tests for the wired MCP tools flow and updated worker mocks/tests to conform to the database schema.

Validation

• Focused tests added or updated for behavior changes
• Documentation updated or explicitly marked not applicable
  -uv run python scripts/ci/check_doc_links.py
• uv run ruff check src tests
• uv run pyright src
• Relevant pytest command: uv run pytest tests/worker/test_daemon.py tests/mcp/test_tools_wired.py -v (and full suite validation with uv run python -m pytest -q --cov=gflow_cli)

[github-actions]

Thanks for the contribution. This PR has been marked as an external contribution and routed for maintainer review.

Before merge, please make sure:

• The PR targets develop.
• Commits use a real Git identity or GitHub noreply email.
• External commits include a DCO sign-off: git commit -s.
• The PR does not include secrets, cookies, account tokens, signed URLs, or private captured data.
• Focused tests/docs are included for behavior changes.

GitHub Copilot code review may be requested as an advisory first pass, but maintainer approval is still required.

[ffroliva]

Council review — changes requested 🔴

Thanks for this, @C1ph3r404 — wiring the MCP tools to the real FlowWorker queue (instead of the old status:"pending" stubs) is genuinely useful work, and the core wiring is correct: task-type dispatch and payload keys line up in both directions, security is clean (no injection/traversal, output paths go through validate_job_id()), and the MCP↔CLI flag naming actually improved. The blockers below are mostly mechanical — happy to help once they're addressed.

Must-fix (blocking)

1. CI is red because the new video tests use an incomplete fake. FakeVideoStatus/_FakeVideoStatus (tests/worker/test_daemon.py:29, tests/mcp/test_tools_wired.py) only set media_id + status="completed", but production calls result.status.succeeded — a property on the real VideoStatus that returns status == "MEDIA_GENERATION_STATUS_SUCCESSFUL". The fake has no succeeded, so the worker raises, the task is marked failed, and the test asserts failed == completed. The production code is correct; the test fixtures are wrong. Please construct the real VideoStatus (or mirror succeeded/failure_reasons/error_message and use the MEDIA_GENERATION_STATUS_SUCCESSFUL string). You did this correctly for the image fakes — same treatment for video.
2. Unclosed DB connection in the new fixtures. The temp_db fixtures return store with no teardown → ResourceWarning: unclosed sqlite3.Connection in CI. Use yield store + store.close().
3. A recording error can mark a successful, credit-spent video as failed. In worker/daemon.py, record_completed_video(...) runs inside the try whose except re-raises — but it executes after the Veo credit is spent. A DB hiccup then flips a real success to failed. Recording/DB errors after a successful generation should warn, not fail the task (same principle as our exit-16 data-store handling).
4. i2v must require initial_frame. In mcp/tools.py, initial_frame is optional and only validated when present, though the docstring says it's "required for i2v". Calling i2v with no frame sends start_image=None, which routes to the text-to-video endpoint and burns a credit on the wrong call. Please enforce it for mode="i2v".
5. docs/MCP.md is now stale. Lines 48-49 still show gflow_generate_image(...) without reference_images and gflow_generate_video(prompt, initial_frame_path, ...) (old name, missing end_frame/mode). Our docs are a living spec — please update them in this PR.
6. Heads-up on auth/cookies.py. Your _get_chrome_cookies3 change touches the exact function an in-flight macOS cookie investigation (#222) depends on. No textual conflict, but it changes the macOS key_file handling and adds a KeyError→PermissionError mapping that affects which failures fall back to the headless reader. Could you split the cookie/--profile changes into a separate PR? It keeps this one scoped to "MCP wiring" and avoids entangling the #222 debugging.

Worth adding (not blocking)

• Real coverage for the headline behavior. Download and recording are fully AsyncMock'd, so "download/recording in daemon" isn't actually exercised — a test asserting a file is written and an asset/DB row recorded would lock it in.
• Extract the ~150-line process_task into _process_image_task/_process_video_task, and factor the path-validation block that's copy-pasted 4× in mcp/tools.py.
• Please rebase onto current develop — this branch was cut before some merged work, so the diff re-presents changes that are already in.

(On testing strategy: we won't ask an external PR to run live-credit e2e — but constructing the real domain dataclasses in tests, as in must-fix #1, is exactly what catches this class of bug.)

[ffroliva]

Hi @C1ph3r404 — thanks for this, the wiring is solid! To land it without a round-trip, I (maintainer) have pushed follow-up commits to your branch (maintainerCanModify) addressing the council review rather than block on back-and-forth. Your authorship on the original commits is preserved.

What changed on top of your work:

• CI-red fix: the tests hand-rolled a fake VideoStatus missing .succeeded, so completed videos flipped to failed (assert "failed" == "completed"). Now using the real VideoStatus/VideoResult.
• tool_specs wiring: the MCP tools parameter was packed into the payload but the worker never applied it, so prompt expansion (creative-director, etc.) was a silent no-op. Wired apply_tool_option into the worker so it actually mirrors the CLI --tool flag.
• daemon: post-success video recording now warns instead of failing a credit-spent task.
• mcp/tools: i2v now requires initial_frame and r2v requires reference_images (clear 400 at the boundary); any non-completed task status is treated as a failure rather than a false success.
• cleanups: closed the test DB fixtures (no ResourceWarning), deduped the image-path validation into one helper, and flattened the DataStore lifecycle in _run_generation_task.
• rebase + docs: merged develop (picks up fix(browser): diagnose + fail loud on chrome→bundled strategy downgrade (Refs #222) #223) and refreshed docs/MCP.md tool signatures.

Full suite green locally (1918 passed). Thanks again for the contribution! 🙏