diff --git a/CLAUDE.md b/CLAUDE.md index 1509d3af..e12537db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -400,7 +400,7 @@ Agent backends are pluggable. `roboco/llm/providers/` defines an `AgentProvider` **RoboCo X account (default-off).** The Head-of-Marketing voice on X (Twitter): drafts a post when a release publishes, drafts replies to meaningful mentions, and — a third, independent capability — periodically investigates RoboCo's own shipped features and drafts a spotlight for an under-publicized one. NOTHING auto-posts across any of the three; every tweet is held in a panel queue for the CEO to edit/approve. Gated by `ROBOCO_X_ENGINE_ENABLED` (+ `_MENTIONS_INTERVAL_SECONDS` / `_MENTIONS_MAX_PER_CYCLE` / `_MENTIONS_MIN_ENGAGEMENT` / `_MAX_OPEN_POSTS` / `X_ACCOUNT_USER_ID`); inert without credentials regardless. Mirrors the `ReleaseManagerEngine` held-artifact shape: `XEngine` (`roboco/services/x_engine.py`) originates a held task (`source` `x_post` / `x_reply` / `x_feature`, `confirmed_by_human=False`, Secretary-owned, skipped by every dispatcher) whose marker payload carries a body clamped to 280 chars. Release posts hook `ReleaseProposalService.approve`'s publish-success branch via a small `draft_release_post` seam; mentions ride a dedicated `_x_mentions_poll_loop` (no webhook infra exists) deduped by a `x_seen_mentions` ledger + per-cycle/open caps — both are **local-model-drafted** (never a cloud LLM in the hot path). The spotlight half is the one exception to "no agent spawn": gated by its own sub-switch `ROBOCO_X_FEATURE_SPOTLIGHT_ENABLED` (+ `_INTERVAL_SECONDS`, default 3 days) on top of `x_engine_enabled`, `_x_feature_spotlight_loop` opens a held PENDING exploration task (`source=x_feature_exploration`, team=Board, assigned to Head of Marketing, carrying a `x_seen_features` dedup-ledger snapshot marker) that `_dispatch_pm_work` routes (mirroring `ROADMAP_SOURCE`) to a one-shot real cloud-LLM spawn of the Head of Marketing — full read tools, investigates CHANGELOG.md/feature-flags/docs/map/charter/KB, calls the Head-of-Marketing-only `propose_feature_spotlight` do-tool exactly once, which marks the feature slug seen (`x_seen_features` table, migration 061) and materializes a brand-new `source=x_feature` held draft (completing the exploration task as a side effect — a deliberate asymmetry from `propose_roadmap`, which instead leaves its own task open). The four OAuth 1.0a secrets live Fernet-encrypted in a singleton `x_credentials` row (migration 059, all-or-nothing set/clear, mirroring the git-token pattern; the API only ever returns `has_credentials`) — decryption is server-side only, agents never hold creds or egress. `XPostService.approve` (CEO-only route) is the ONLY caller of `x_client.post_tweet`: it posts under a Redis single-flight lock, **re-reads the committed task state inside the lock and commits COMPLETED before releasing** so a concurrent approve can't double-post, and is idempotent (an already-posted draft is a no-op). The hand-rolled OAuth 1.0a HMAC-SHA1 signer (`roboco/services/x_client.py`) adds no dependency; a `NullXClient` makes the unconfigured path a graceful no-op (research-engine posture). All three draft kinds share one voice: `XEngine._voice_guide` reads the CEO-editable `company_goals.brand_voice` charter field (migration 061, panel-editable in Business → Goals) and appends it to a generic baseline (`_HOM_VOICE`) — the baseline alone until the CEO supplies a real sample. Panel: `x-post-queue.tsx` (editable draft + 280 counter, approve/reject, a `sourceMeta`-driven label/icon per source including "Feature spotlight") + `x-credentials-card.tsx` (4 write-only secret inputs). -**RoboCo video engine (default-off).** Bespoke motion-graphics videos (release announcements, feature spotlights, on-demand CEO briefs) authored by a UX/UI dev and distributed to X/TikTok — nothing renders or posts without the flags on, and nothing posts without an explicit CEO approval. Gated by `ROBOCO_VIDEO_ENGINE_ENABLED` (+ sub-switches `ROBOCO_VIDEO_ON_RELEASE` / `ROBOCO_VIDEO_ON_SPOTLIGHT`, and `_MAX_OPEN_POSTS` / `_RENDER_INTERVAL_SECONDS` / `_RENDER_TIMEOUT_SECONDS` / `_REQUEST_TIMEOUT_SECONDS` / `_OUTPUT_DIR`); a CEO on-demand brief rides `POST /video/request` regardless of the release/spotlight sub-switches. A project opts in via `projects.video_engine_enabled` (migration 063, mirroring `ci_watch_enabled`): the global flag arms the subsystem, the per-project flag opts a repo into authoring against its `motion/` — `VideoEngine._opted_in_project` no-ops `open_video_task` until the operator flips it in the panel's edit-project dialog. Two task kinds mirror the XEngine/ReleaseManagerEngine "originate a CEO-scoped artifact" shape but split across the real delivery lifecycle: `VideoEngine.open_video_task` (`roboco/services/video_engine.py`) opens a normal, ASSIGNED **authoring task** (`source=video`, `confirmed_by_human=True`, team=UX/UI, balanced across `ux-dev-1`/`ux-dev-2` by open-task count) that dispatches like any other pre-assigned code task — NOT held, NOT in any dispatcher's skip bucket. The assigned dev builds a HyperFrames HTML composition under `motion/compositions//` and calls the UX/UI-team-gated `propose_video` do-tool (metadata-only: composition id, input props, per-platform captions — every developer role carries the tool on their manifest, but the runtime `_caller_team` check rejects a be-dev/fe-dev) to stamp the task's `video_draft` marker, then commits + `open_pr` through the normal PR-review gate. Once that authoring task reaches `completed`, the orchestrator's `_video_render_loop` (bounded retry, `_MAX_VIDEO_RENDER_ATTEMPTS`) tars the merged `motion/` dir from the project's read-clone and POSTs it to the credential-free **video-renderer sidecar** (`VideoRenderer` in `roboco/services/video_renderer_client.py`, `ROBOCO_VIDEO_RENDERER_BASE_URL`) to render both the 9:16 and 1:1 cuts to MP4 (`video_output_dir`); on success `VideoEngine._originate_video_post` materializes a held **video-post draft** (`source=video_post`, `confirmed_by_human=False`, Secretary-owned, skipped by every dispatcher) carrying `mp4_paths` (`{vertical, square}` absolute paths) + the per-platform captions. The CEO reviews it in the panel's video queue (`video-post-queue.tsx`; `GET /video/posts` lists drafts including `mp4_paths` so the panel knows which cuts exist, `GET /video/posts/{id}/media?cut=vertical|square` streams the MP4 bytes for the preview player, CEO-gated throughout) and edits captions / approves / rejects. `VideoPostService.approve` (`roboco/services/video_post_service.py`) is the ONLY caller of the X-v2 (`XVideoPoster` in `x_video_client.py`) and TikTok inbox-upload (`TikTokPoster` in `tiktok_client.py`) posters; because a video upload + transcode/poll can run well past a minute, the critical section runs under a heartbeat-renewed Redis mutex (`heartbeat_mutex.py`, mirroring `ReleaseProposalService`'s release-execute lock shape) rather than a flat lock, commits each platform's posted-id durably before attempting the next (a partial failure never re-posts an already-succeeded platform on retry), and is idempotent (an already-`COMPLETED` draft returns the stored ids without calling a poster again). TikTok's four OAuth2 secrets live Fernet-encrypted in a singleton `tiktok_credentials` row (mirroring the git-token / `x_credentials` pattern; the API only ever returns `has_credentials`) — set via the panel's TikTok credentials card. `NullVideoRenderer` / `NullXVideoPoster` / `NullTikTokPoster` make every unconfigured leg a graceful no-op rather than a crash. +**RoboCo video engine (default-off).** Bespoke motion-graphics videos (release announcements, feature spotlights, on-demand CEO briefs) authored by a UX/UI dev and distributed to X/TikTok — nothing renders or posts without the flags on, and nothing posts without an explicit CEO approval. Gated by `ROBOCO_VIDEO_ENGINE_ENABLED` (+ sub-switches `ROBOCO_VIDEO_ON_RELEASE` / `ROBOCO_VIDEO_ON_SPOTLIGHT`, and `_MAX_OPEN_POSTS` / `_RENDER_INTERVAL_SECONDS` / `_RENDER_TIMEOUT_SECONDS` / `_REQUEST_TIMEOUT_SECONDS` / `_OUTPUT_DIR`); a CEO on-demand brief rides `POST /video/request` regardless of the release/spotlight sub-switches. A project opts in via `projects.video_engine_enabled` (migration 063, mirroring `ci_watch_enabled`): the global flag arms the subsystem, the per-project flag opts a repo into authoring against its `motion/` — `VideoEngine._opted_in_project` no-ops `open_video_task` until the operator flips it in the panel's edit-project dialog. Two task kinds mirror the XEngine/ReleaseManagerEngine "originate a CEO-scoped artifact" shape but split across the real delivery lifecycle: `VideoEngine.open_video_task` (`roboco/services/video_engine.py`) opens a normal, ASSIGNED **authoring task** (`source=video`, `confirmed_by_human=True`, team=UX/UI, balanced across `ux-dev-1`/`ux-dev-2` by open-task count) that dispatches like any other pre-assigned code task — NOT held, NOT in any dispatcher's skip bucket. The assigned dev builds a HyperFrames HTML composition under `motion/compositions//` and calls the UX/UI-team-gated `propose_video` do-tool (metadata-only: composition id, input props, per-platform captions — every developer role carries the tool on their manifest, but the runtime `_caller_team` check rejects a be-dev/fe-dev) to stamp the task's `video_draft` marker, then commits + `open_pr` through the normal PR-review gate. Once that authoring task reaches `completed`, the orchestrator's `_video_render_loop` (bounded retry, `_MAX_VIDEO_RENDER_ATTEMPTS`) tars the merged `motion/` dir from the project's read-clone and POSTs it to the credential-free **video-renderer sidecar** (`VideoRenderer` in `roboco/services/video_renderer_client.py`, `ROBOCO_VIDEO_RENDERER_BASE_URL`) to render both the 9:16 and 1:1 cuts to MP4 (`video_output_dir`); on success `VideoEngine._originate_video_post` materializes a held **video-post draft** (`source=video_post`, `confirmed_by_human=False`, Secretary-owned, skipped by every dispatcher) carrying `mp4_paths` (`{vertical, square}` absolute paths) + the per-platform captions. The CEO reviews it in the panel's video queue (`video-post-queue.tsx`; `GET /video/posts` lists drafts including `mp4_paths` so the panel knows which cuts exist, `GET /video/posts/{id}/media?cut=vertical|square` streams the MP4 bytes for the preview player, CEO-gated throughout) and edits captions / approves / rejects. `VideoPostService.approve` (`roboco/services/video_post_service.py`) is the ONLY caller of the X-v2 (`XVideoPoster` in `x_video_client.py`) and TikTok inbox-upload (`TikTokPoster` in `tiktok_client.py`) posters; because a video upload + transcode/poll can run well past a minute, the critical section runs under a heartbeat-renewed Redis mutex (`heartbeat_mutex.py`, mirroring `ReleaseProposalService`'s release-execute lock shape) rather than a flat lock, commits each platform's posted-id durably before attempting the next (a partial failure never re-posts an already-succeeded platform on retry), and is idempotent (an already-`COMPLETED` draft returns the stored ids without calling a poster again). TikTok's four OAuth2 secrets live Fernet-encrypted in a singleton `tiktok_credentials` row (mirroring the git-token / `x_credentials` pattern; the API only ever returns `has_credentials`) — set via the panel's TikTok credentials card. `NullVideoRenderer` / `NullXVideoPoster` / `NullTikTokPoster` make every unconfigured leg a graceful no-op rather than a crash. **Artifact verification (2026-07-16):** authoring is gated on the RENDERED clip, not its source — the `request_render` do-verb (developer/QA, mirroring `request_sandbox`'s shape) renders the caller's actual composition via the sidecar's new frames mode (`POST /render` with `frames=N` → ffprobe'd duration + N midpoint-sampled PNGs, tar.gz back) and extracts them to the container-shared `{workspaces_root}/{project}/.previews/{task8}/{orientation}/`; the dev renders their own working tree (worktree-aware, `head_sha`/`dirty` provenance), QA a read-only `git archive` export of the assembled branch (`WorkspaceService.export_branch_motion` — the read clone's pinned HEAD undisturbed). Success stamps the `render_preview` marker; `i_am_done` on a `source=video` task refuses without it (`Requirement.RENDER_VERIFIED` in the tracing gate, mirrored in the possibilities-matrix fast path — the canonical source string lives in foundation as `markers.VIDEO_TASK_SOURCE`), the dev spawn prompt orders Read-every-frame verification, and QA's `claim_review` evidence carries a `video_context` block so the reviewer checks output, not source. A CEO reject of a held video-post draft with a non-empty reason now also re-enters the flow: `VideoPostService.reject` → `VideoEngine.reauthor_from_rejection` opens a fresh authoring task carrying the verbatim feedback + a revise-in-place pointer at the existing composition (best-effort, never fails the reject). **Board roadmap engine (default-off).** The Board originating strategic work: on a weekly interval (`ROBOCO_ROADMAP_ENGINE_ENABLED` + `_INTERVAL_SECONDS` / `_MIN_ITEMS_PER_CYCLE` / `_MAX_ITEMS_PER_CYCLE`) `RoadmapEngine` (`roboco/services/roadmap_engine.py`) opens ONE held **exploration** task (`source="board_roadmap"`, `confirmed_by_human=False`, PENDING, Product-Owner-assigned, `Team.BOARD`), deduped to one open cycle at a time. A dedicated one-shot `_dispatch_roadmap_exploration` spawns the Product Owner **solo** — deliberately NOT `_handle_board_assigned_task` (which would also spawn Head of Marketing and fire the Approve-&-Start handoff, both wrong for a PO-authored cycle) — reusing the `_board_dispatched` one-shot tracker + respawn breaker, and short-circuiting once the cycle is authored. The PO explores (read-only git, KB/RAG, metrics, releases, charter, optional web research) and makes ONE `propose_roadmap` call (a content verb gated to `product_owner` only, `_ROADMAP_ROLES`; wired through the do_server/Choreographer like `pitch`) authoring a **themed cycle** — a one-line goal + 3-7 item drafts — persisted as a `roadmap_cycle` marker on the exploration task (no table/migration). The CEO acts per-item in the panel roadmap queue (`roadmap-review-queue.tsx`; `/api/roadmap/cycles{,/items/{id}/approve,/items/{id}/reject}`, CEO-only): approve materializes that item as a BACKLOG task (`source="roadmap"`, no assignee — never auto-starts; normal PM activation picks it up) via `PrompterService.create_task_from_draft`, reject records a reason; when every item is terminal the exploration task completes (`RoadmapService`, idempotent per item). Dispatchers skip `board_roadmap` (never delivery work). `create_task_from_draft` honors a draft-declared `source` only from a `{prompter, roadmap}` whitelist — an LLM-authored draft can't impersonate a privileged origin. diff --git a/agents/prompts/_generated/developer.md b/agents/prompts/_generated/developer.md index 27babf63..9b53c3fa 100644 --- a/agents/prompts/_generated/developer.md +++ b/agents/prompts/_generated/developer.md @@ -30,6 +30,7 @@ | `draft_playbook` | `draft_playbook(title: str, problem: str, procedure: str, tags: list[str] = PydanticUndefined, source_task_id: UUID | None = None)` | | `propose_video` | `propose_video(composition_id: str, x_caption: str, tiktok_caption: str, platforms: list[str], input_props: str | Any | None = None)` | | `request_sandbox` | `request_sandbox(services: list[str] | None = None, extensions: str | list[str] | None = None)` | +| `request_render` | `request_render(composition_id: str | None = None, orientation: str = 'vertical', frame_count: int = 8, input_props: str | Any | None = None)` | | `notify_list` | `notify_list(unread_only: bool = True, pending_ack_only: bool = False, limit: int = 20)` | | `notify_get` | `notify_get(notification_id: UUID)` | | `notify_ack` | `notify_ack(notification_id: UUID)` | diff --git a/agents/prompts/_generated/qa.md b/agents/prompts/_generated/qa.md index 3848df15..25abfa95 100644 --- a/agents/prompts/_generated/qa.md +++ b/agents/prompts/_generated/qa.md @@ -25,6 +25,7 @@ | `evidence` | `evidence(task_id: UUID)` | | `draft_playbook` | `draft_playbook(title: str, problem: str, procedure: str, tags: list[str] = PydanticUndefined, source_task_id: UUID | None = None)` | | `request_sandbox` | `request_sandbox(services: list[str] | None = None, extensions: str | list[str] | None = None)` | +| `request_render` | `request_render(composition_id: str | None = None, orientation: str = 'vertical', frame_count: int = 8, input_props: str | Any | None = None)` | | `notify_list` | `notify_list(unread_only: bool = True, pending_ack_only: bool = False, limit: int = 20)` | | `notify_get` | `notify_get(notification_id: UUID)` | | `notify_ack` | `notify_ack(notification_id: UUID)` | diff --git a/agents/prompts/_generated/verbs.md b/agents/prompts/_generated/verbs.md index 5dcb2a23..26e541a9 100644 --- a/agents/prompts/_generated/verbs.md +++ b/agents/prompts/_generated/verbs.md @@ -39,6 +39,7 @@ real tools live in their agent_sdk drivers, not role_config. | `draft_playbook` | `draft_playbook(title: str, problem: str, procedure: str, tags: list[str] = PydanticUndefined, source_task_id: UUID | None = None)` | | `propose_video` | `propose_video(composition_id: str, x_caption: str, tiktok_caption: str, platforms: list[str], input_props: str | Any | None = None)` | | `request_sandbox` | `request_sandbox(services: list[str] | None = None, extensions: str | list[str] | None = None)` | +| `request_render` | `request_render(composition_id: str | None = None, orientation: str = 'vertical', frame_count: int = 8, input_props: str | Any | None = None)` | | `notify_list` | `notify_list(unread_only: bool = True, pending_ack_only: bool = False, limit: int = 20)` | | `notify_get` | `notify_get(notification_id: UUID)` | | `notify_ack` | `notify_ack(notification_id: UUID)` | @@ -69,6 +70,7 @@ real tools live in their agent_sdk drivers, not role_config. | `evidence` | `evidence(task_id: UUID)` | | `draft_playbook` | `draft_playbook(title: str, problem: str, procedure: str, tags: list[str] = PydanticUndefined, source_task_id: UUID | None = None)` | | `request_sandbox` | `request_sandbox(services: list[str] | None = None, extensions: str | list[str] | None = None)` | +| `request_render` | `request_render(composition_id: str | None = None, orientation: str = 'vertical', frame_count: int = 8, input_props: str | Any | None = None)` | | `notify_list` | `notify_list(unread_only: bool = True, pending_ack_only: bool = False, limit: int = 20)` | | `notify_get` | `notify_get(notification_id: UUID)` | | `notify_ack` | `notify_ack(notification_id: UUID)` | diff --git a/docs/map/video-engine.md b/docs/map/video-engine.md index f063eff3..97f04e55 100644 --- a/docs/map/video-engine.md +++ b/docs/map/video-engine.md @@ -15,17 +15,19 @@ The RoboCo video engine: a default-off subsystem that authors bespoke short mark | `roboco/runtime/heartbeat_mutex.py` | `HeartbeatMutex` — Redis mutex with heartbeat-renewed TTL, shared with `ReleaseProposalService`'s release-execute lock shape; backs `VideoPostService.approve`'s long video-upload critical section. | — | | `roboco/mcp/do_server.py` `propose_video` | Do-tool the UX/UI dev calls exactly once per authoring task to stamp the `video_draft` marker (composition id + per-platform captions + input props); metadata-only, does not render. | — | | `roboco/services/gateway/content_actions.py` `propose_video` | Server-side action: team-gated (`_caller_team` rejects be-dev/fe-dev), resolves the caller's open video task, `markers.set_video_draft` with the metadata. | — | +| `roboco/services/gateway/content_actions.py` `request_render` | Do-verb (developer/QA): renders the caller's ACTUAL composition to keyframe PNGs via the sidecar's frames mode and stamps the `render_preview` marker — dev renders their own tree (worktree-aware, `head_sha`/`dirty` stamped), QA a read-only branch export (`WorkspaceService.export_branch_motion`). Frames land at the container-shared `{workspaces_root}/{project}/.previews/{task8}/{orientation}/`. | — | +| `roboco/foundation/policy/tracing.py` `RENDER_VERIFIED` | `i_am_done` requirement on `source=video` tasks: no stamped `render_preview` → tracing gap naming `render_preview` (hint: call `request_render`, Read every frame). Mirrored in the possibilities-matrix fast path. | — | | `alembic/versions/062_tiktok_credentials.py` | Migration 062 — the `tiktok_credentials` singleton row (Fernet-encrypted OAuth2 secrets, all-or-nothing set/clear, mirroring the git-token / `x_credentials` pattern). | 44 | | `video-renderer/` | The sidecar: `server.js` (HTTP+tarball boundary), `render.js` (`@hyperframes/producer` `createRenderJob` + `executeRenderJob`, system `ffmpeg`, headless Chromium). Credential-free and git-free — reads only what's POSTed. | — | | `docker/video-renderer.Dockerfile` | Sidecar image (`roboco-video-renderer`): Node + Chromium + system `ffmpeg`; installs `@hyperframes/producer`. No RoboCo source, no creds. | — | ## Data Flow -DETECT → AUTHOR: a release publish (`ROBOCO_VIDEO_ON_RELEASE`), a CEO-approved feature-spotlight draft that requests one (`ROBOCO_VIDEO_ON_SPOTLIGHT`), or a CEO on-demand `POST /api/video/request` calls `VideoEngine.open_video_task`, which creates a normal ASSIGNED UX/UI authoring task (`source=video`, `confirmed_by_human=True`, balanced across the two ux-devs) — NOT held, NOT in any dispatcher's skip bucket. The assigned dev authors `motion/compositions//{vertical,square}.html` (HyperFrames render params on ``), calls the `propose_video` do-tool exactly once (server-side `content_actions.propose_video` is team-gated and stamps `video_draft`), then `commit` + `open_pr` through the normal PR-review gate. The authoring task rides the standard QA/doc/review lifecycle to `completed`. +DETECT → AUTHOR: a release publish (`ROBOCO_VIDEO_ON_RELEASE`), a CEO-approved feature-spotlight draft that requests one (`ROBOCO_VIDEO_ON_SPOTLIGHT`), or a CEO on-demand `POST /api/video/request` calls `VideoEngine.open_video_task`, which creates a normal ASSIGNED UX/UI authoring task (`source=video`, `confirmed_by_human=True`, balanced across the two ux-devs) — NOT held, NOT in any dispatcher's skip bucket. The assigned dev authors `motion/compositions//{vertical,square}.html` (HyperFrames render params on ``), calls the `propose_video` do-tool exactly once (server-side `content_actions.propose_video` is team-gated and stamps `video_draft`), then verifies the ARTIFACT: `request_render` renders the dev's actual working tree to keyframe PNGs the dev must Read (every scene fully visible and legible — the gate that catches an authored duration shorter than its scene list), iterating fix → re-render until the frames prove the brief; `i_am_done` refuses without the stamped `render_preview` marker (`Requirement.RENDER_VERIFIED`). Then `commit` + `open_pr` through the normal PR-review gate. The authoring task rides the standard QA/doc/review lifecycle to `completed`, with QA's `claim_review` evidence carrying a `video_context` block (the dev's preview + an instruction to `request_render` the branch state fresh). RENDER: once the authoring task is `completed`, the orchestrator's `_video_render_loop` (bounded retry, `_MAX_VIDEO_RENDER_ATTEMPTS`) resolves the project's read-clone at the merged HEAD, tars the `motion/` dir, and POSTs it to the credential-free `video-renderer` sidecar (`ROBOCO_VIDEO_RENDERER_BASE_URL`). The sidecar untars, runs `@hyperframes/producer`'s `createRenderJob` + `executeRenderJob` per orientation (headless Chrome + system `ffmpeg`, `ROBOCO_VIDEO_RENDER_TIMEOUT_SECONDS` per render), and streams both 9:16 and 1:1 MP4s back. `VideoRenderer` saves them to `ROBOCO_VIDEO_OUTPUT_DIR` (`_save` also PUTs each to MinIO when `minio_endpoint` is set, non-fatal). On success `VideoEngine._originate_video_post` materializes a held `video_post` draft (`source=video_post`, `confirmed_by_human=False`, Secretary-owned, skipped by every dispatcher) carrying `mp4_paths` (`{vertical, square}` absolute paths) + the per-platform captions. -CEO ACT: `GET /api/video/posts` lists held drafts (including `mp4_paths`); `GET /api/video/posts/{id}/media?cut=vertical|square` streams the MP4 bytes for the preview player (CEO-gated, falls back to `FileResponse` on `S3Error`/unconfigured MinIO). The CEO edits captions and approves/rejects in the panel's `video-post-queue.tsx`. `POST /api/video/posts/{id}/approve` is the ONLY caller of `XVideoPoster` / `TikTokPoster`: it acquires `HeartbeatMutex`, re-reads the committed task state inside the lock, commits `COMPLETED` before releasing (so a concurrent approve can't double-post), commits each platform's posted-id durably before attempting the next (a partial failure never re-posts an already-succeeded platform on retry), and is idempotent (an already-`COMPLETED` draft returns the stored ids without calling a poster). `POST /api/video/posts/{id}/reject` cancels the draft with a reason. +CEO ACT: `GET /api/video/posts` lists held drafts (including `mp4_paths`); `GET /api/video/posts/{id}/media?cut=vertical|square` streams the MP4 bytes for the preview player (CEO-gated, falls back to `FileResponse` on `S3Error`/unconfigured MinIO). The CEO edits captions and approves/rejects in the panel's `video-post-queue.tsx`. `POST /api/video/posts/{id}/approve` is the ONLY caller of `XVideoPoster` / `TikTokPoster`: it acquires `HeartbeatMutex`, re-reads the committed task state inside the lock, commits `COMPLETED` before releasing (so a concurrent approve can't double-post), commits each platform's posted-id durably before attempting the next (a partial failure never re-posts an already-succeeded platform on retry), and is idempotent (an already-`COMPLETED` draft returns the stored ids without calling a poster). `POST /api/video/posts/{id}/reject` cancels the draft with a reason — and, for a non-empty reason, `VideoEngine.reauthor_from_rejection` opens a fresh authoring task (same occasion, brief = the CEO's verbatim feedback + revise-in-place pointer at the existing composition) so the rejection feedback re-enters the delivery flow instead of dying on the cancelled draft; best-effort, never fails the reject. ## Config Flags diff --git a/docs/rag/architecture/video-engine.md b/docs/rag/architecture/video-engine.md index e2cabeba..e61fd60a 100644 --- a/docs/rag/architecture/video-engine.md +++ b/docs/rag/architecture/video-engine.md @@ -24,6 +24,10 @@ Even when enabled, distribution requires an explicit per-clip CEO approval — a All three open a normal, **assigned** UX/UI authoring task (balanced across the two ux-devs) rather than a held draft — the dev builds a HyperFrames HTML composition under `motion/compositions//` (one `/vertical.html` + `/square.html` carrying the HyperFrames render params on ``) and proposes its composition id + per-platform captions via the team-gated `propose_video` do-tool, then ships it through the standard commit/PR/QA/doc/review lifecycle. +## Artifact verification (request_render) + +Authoring is gated on the RENDERED artifact, not just its source: the `request_render` do-tool (developer/QA) renders the caller's actual composition through the sidecar and extracts evenly spaced keyframe PNGs to a container-shared `.previews/` path, returning their absolute paths in the envelope's `evidence.frames`. The agent must Read every frame and verify each scene/feature from the brief appears fully and legibly — a 14-second cut that only ever shows its first scene is exactly what this catches. A developer renders their own working tree (worktree-aware, `head_sha`/`dirty` provenance stamped); QA renders a read-only `git archive` export of the assembled branch — never a working tree. A successful render stamps the task's `render_preview` marker, and `i_am_done` on a video-authoring task refuses without it (`Requirement.RENDER_VERIFIED`, mirrored in the possibilities-matrix fast path), so no video task can complete on a source-only self-review. QA's `claim_review` evidence carries a `video_context` block (composition id + the dev's stamped preview + an instruction to re-render the branch state) so the reviewer checks output, not source. + ## Render loop and the sidecar Once the authoring task completes, an orchestrator render loop (`_video_render_loop`, interval `ROBOCO_VIDEO_RENDER_INTERVAL_SECONDS`) tars the merged `motion/` source and POSTs it to the `video-renderer` sidecar (`ROBOCO_VIDEO_RENDERER_BASE_URL`). The sidecar is credential-free and git-free — it reads only what it's POSTed, bundles the composition, renders both the 9:16 and 1:1 MP4 cuts (`ROBOCO_VIDEO_RENDER_TIMEOUT_SECONDS` per render), and streams the bytes back. The orchestrator writes the MP4s to `ROBOCO_VIDEO_OUTPUT_DIR` (bind-mounted in all three compose files so renders survive container recreation) — the sidecar never writes to disk directly. The sidecar renders via `@hyperframes/producer` — agent-authored HTML rendered in headless Chrome plus system `ffmpeg` for the seek-driven, frame-deterministic MP4 cut. HyperFrames was chosen for its Apache-2.0 license (no React/TSX toolchain lock-in for the authoring agents), agent-authored HTML as the composition substrate, and seek-driven deterministic renders that match the post-pipeline shape without a node-bundled renderer. @@ -36,7 +40,7 @@ The render pass materializes a held `video_post` draft — mirroring the X-post/ |----------|--------| | `GET /api/video/posts` | List every held video draft awaiting decision. | | `POST /api/video/posts/{task_id}/approve` | Post the rendered clip to X (native video, v2 media upload) and/or TikTok (inbox upload). Idempotent — approving an already-posted draft is a no-op. | -| `POST /api/video/posts/{task_id}/reject` | Cancel the draft with a reason. Terminal. | +| `POST /api/video/posts/{task_id}/reject` | Cancel the draft with a reason — and route the feedback back into the flow: a non-empty reason opens a fresh authoring task (same occasion, brief = the CEO's verbatim feedback + a revise-in-place pointer at the existing composition), so a rejection is a rework loop, not a dead end. | | `POST /api/video/request` | Open an on-demand authoring task (CEO-only). | Approval runs under a Redis heartbeat-renewed lock so a double-click can't double-post; the task is marked `COMPLETED` under the same lock before it releases. diff --git a/docs/rag/tools/task-tools.md b/docs/rag/tools/task-tools.md index e8d06772..cb492b28 100644 --- a/docs/rag/tools/task-tools.md +++ b/docs/rag/tools/task-tools.md @@ -166,3 +166,7 @@ Your plan's steps are the progress checklist; the percentage is derived from com ## Sandbox DB/Redis/Mongo (Developer + QA) `request_sandbox(services=None, extensions=None)` — a **content tool** on `roboco-do`, not a flow verb — provisions a throwaway sandbox Postgres/Redis/Mongo on demand, for a project that opted in (`projects.sandbox_services`). Only `developer` and `qa` carry it. Omit `services` for the project's whole opted-in set; requesting one outside it is rejected naming the allowed set. `extensions` is an optional per-service map of extensions/modules to activate (e.g. `{"postgres": ["vector"]}`), unioned with the project's standing `sandbox_extensions` set and bounded by a fixed allowlist (pg: vector/postgis/pg_trgm/citext/uuid-ossp; redis: search/json/bloom — no `plpython3u`); an unallowed feature or a feature for a non-opted service is rejected naming the allowed set. Creds come back in the envelope's `evidence`, one entry per service, including ready-to-`export` `ROBOCO_TEST_*` values for gate tooling and an `available_extensions` list of what was activated. Calling it again is a cheap no-op (same creds) as long as the requested features are a subset of the cached set. See `docs/rag/architecture/sandbox-db.md`. + +## Video render preview (Developer + QA, video-authoring tasks) + +`request_render(composition_id=None, orientation="vertical", frame_count=8, input_props=None)` — a **content tool** on `roboco-do` — renders your ACTUAL HyperFrames composition through the video-renderer sidecar and returns evenly spaced keyframe PNGs. Only valid on a `source=video` authoring task. Omit `composition_id` to use the one you already proposed via `propose_video`. The envelope's `evidence.frames` lists absolute paths (readable from your container): **Read every frame** and verify each scene/feature from the brief appears fully and legibly — the composition source looking right is NOT evidence the rendered clip is right. A developer renders their own working tree; QA renders a read-only export of the assembled branch. A successful render stamps the task's `render_preview` marker — `i_am_done` on a video task refuses without it. If a scene is missing, clipped, or rushed, fix the composition and call it again. See `docs/rag/architecture/video-engine.md`. diff --git a/roboco/api/routes/v1/do.py b/roboco/api/routes/v1/do.py index 6953f4b9..f83dd798 100644 --- a/roboco/api/routes/v1/do.py +++ b/roboco/api/routes/v1/do.py @@ -32,6 +32,7 @@ from roboco.api.schemas.v1.do import ( PRUpdateRequest, ReadMessagesRequest, RejectPlaybookRequest, + RequestRenderRequest, RequestSandboxRequest, ) from roboco.security import ( @@ -274,6 +275,23 @@ async def do_request_sandbox( return envelope_to_response(env, request) +@router.post("/request_render") +async def do_request_render( + request: Request, + body: RequestRenderRequest, + x_agent_id: _AgentIdHeader, + actions: _ContentActionsDep, +) -> dict: + env = await actions.request_render( + agent_id=x_agent_id, + composition_id=body.composition_id, + orientation=body.orientation, + frame_count=body.frame_count, + input_props=body.input_props, + ) + return envelope_to_response(env, request) + + # --------------------------------------------------------------------------- # Wave 1 — pre-gateway parity # --------------------------------------------------------------------------- diff --git a/roboco/api/schemas/v1/do.py b/roboco/api/schemas/v1/do.py index 81585cda..c9f8d46e 100644 --- a/roboco/api/schemas/v1/do.py +++ b/roboco/api/schemas/v1/do.py @@ -183,6 +183,17 @@ class RequestSandboxRequest(BaseModel): extensions: dict[str, list[str]] | None = None +class RequestRenderRequest(BaseModel): + """Render a video composition to preview frames for artifact + verification. Omitted `composition_id` falls back to the task's + `video_draft` marker.""" + + composition_id: str | None = None + orientation: str = "vertical" + frame_count: int = 8 + input_props: dict[str, Any] | None = None + + class ProgressRequest(BaseModel): """Progress update; % is DERIVED from the plan checklist. diff --git a/roboco/foundation/policy/content/markers.py b/roboco/foundation/policy/content/markers.py index 8ca0cf8e..7b2d95dd 100644 --- a/roboco/foundation/policy/content/markers.py +++ b/roboco/foundation/policy/content/markers.py @@ -48,6 +48,7 @@ X_SPOTLIGHT_SKIP_REASON = "x_spotlight_skip_reason" ROADMAP_CYCLE = "roadmap_cycle" VIDEO_DRAFT = "video_draft" VIDEO_REJECT_REASON = "video_reject_reason" +RENDER_PREVIEW = "render_preview" VAULT_CURATION_DISPATCHED = "vault_curation_dispatched" VAULT_NOTE_REF = "vault_note_ref" DOCS_SYNC_RELEASE_VERSION = "docs_sync_release_version" @@ -302,6 +303,29 @@ def set_video_reject_reason(task: HasMarkers, reason: str) -> None: set_marker(task, VIDEO_REJECT_REASON, reason) +# Task-source tag for a video-authoring task. Canonical here so foundation- +# layer gates can key on it without importing the services layer; +# services/task.py's VIDEO_SOURCE aliases this. +VIDEO_TASK_SOURCE = "video" + + +def get_render_preview(task: HasMarkers) -> dict[str, Any] | None: + """The last ``request_render`` preview stamped on a video-authoring task. + + Payload: {at, composition_id, orientation, frame_count, duration_seconds, + frames (absolute container paths, readable from every agent container), + head_sha, dirty, rendered_by, source ("workspace"|"branch")}. Its + presence is what the i_am_done RENDER_VERIFIED gate checks — proof the + author looked at the actual rendered artifact, not just the source. + """ + val = get_marker(task, RENDER_PREVIEW) + return val if isinstance(val, dict) else None + + +def set_render_preview(task: HasMarkers, payload: dict[str, Any]) -> None: + set_marker(task, RENDER_PREVIEW, payload) + + # --- vault curation --------------------------------------------------------- # One-shot guard for the root-completion Auditor spawn (orchestrator): set the # moment the spawn fires so a restart can't re-spawn a root another process diff --git a/roboco/foundation/policy/tracing.py b/roboco/foundation/policy/tracing.py index 686c133e..a4ff778c 100644 --- a/roboco/foundation/policy/tracing.py +++ b/roboco/foundation/policy/tracing.py @@ -19,6 +19,8 @@ from dataclasses import dataclass, field from enum import StrEnum from typing import Any +from roboco.foundation.policy.content import markers + class Requirement(StrEnum): PLAN = "plan" @@ -55,6 +57,10 @@ class Requirement(StrEnum): # before the developer can resubmit. Trivially satisfied by an empty # ledger (no findings ever filed). FINDINGS_ADDRESSED = "findings_addressed" + # A video-authoring task must carry a request_render preview before + # i_am_done — proof the author looked at the rendered artifact, not just + # the source. No-op for a non-video task. + RENDER_VERIFIED = "render_verified" @dataclass(frozen=True) @@ -218,6 +224,13 @@ def _check_findings_addressed(_task: Any, ctx: GateContext) -> list[str]: return [f"finding:{fid}" for fid in ctx.open_finding_ids] +def _check_render_verified(task: Any, _ctx: GateContext) -> list[str]: + """The gate that makes a video dev look at the rendered artifact.""" + if getattr(task, "source", None) != markers.VIDEO_TASK_SOURCE: + return [] + return [] if markers.get_render_preview(task) else ["render_preview"] + + def _check_subtasks_terminal(task: Any, _ctx: GateContext) -> list[str]: """Caller passes a task whose `_subtasks_all_terminal` boolean is set by the choreographer based on a DB query. Validator just reads it.""" @@ -250,6 +263,7 @@ _CHECKERS: dict[Requirement, Checker] = { Requirement.PR_REVIEWER_NOTES_MIN_CHARS: _check_pr_reviewer_notes_min_chars, Requirement.QUICK_CONTEXT_MIN_CHARS: _check_quick_context_min_chars, Requirement.FINDINGS_ADDRESSED: _check_findings_addressed, + Requirement.RENDER_VERIFIED: _check_render_verified, } @@ -314,6 +328,9 @@ VERB_REQUIREMENTS: dict[str, frozenset[Requirement]] = { # `resolved_findings`) before resubmitting. Trivially satisfied # when the ledger has no rows for this task. Requirement.FINDINGS_ADDRESSED, + # A video-authoring task must have a request_render preview on + # file. No-op for every other task source. + Requirement.RENDER_VERIFIED, } ), # QA pass/fail. diff --git a/roboco/mcp/do_server.py b/roboco/mcp/do_server.py index 200caedd..e839234c 100644 --- a/roboco/mcp/do_server.py +++ b/roboco/mcp/do_server.py @@ -50,6 +50,10 @@ _COMMIT_TIMEOUT = 190 # 1035s — images are pre-pulled at startup in practice, so cold pulls here # are the exception, but the timeout must cover the worst case anyway. _SANDBOX_TIMEOUT = 1080 +# request_render runs an inline sidecar render call; the sidecar's own render +# ceiling is video_render_timeout_seconds (default 600s) plus its request +# timeout for uploading the tarball — 660s gives that headroom. +_RENDER_TIMEOUT = 660 # Tight timeout for SDK loopback — local sidecar; gateway path must not stall. _SDK_TIMEOUT = 2.0 # FastAPI's default missing-route status. Every /api/v1/do/* route returns @@ -658,6 +662,10 @@ def propose_video( composition in motion/compositions//. Then commit + open_pr to send it through the normal PR-review gate. + After proposing, call request_render and verify the frames it returns + before i_am_done — the composition's SOURCE looking right is not the + same as the RENDERED artifact looking right. + Args: composition_id: The HyperFrames composition id (the directory name under motion/compositions/, e.g. 'release-announcement'). @@ -754,6 +762,44 @@ def request_sandbox( ) +def request_render( + composition_id: str | None = None, + orientation: str = "vertical", + frame_count: int = 8, + input_props: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Render your video composition to a strip of preview frames — call + this after building or altering the HyperFrames composition, on a + video-authoring task, to verify the RENDERED artifact rather than just + trusting the source looks right. + + Omit ``composition_id`` to use the one already proposed via + propose_video. Returns ``evidence.frames`` — absolute paths to the + rendered frame images. Read EVERY frame with your file tools before + calling i_am_done; if a scene is missing, clipped, or wrong, fix the + composition and call this again. A developer renders from their own + working tree; QA renders from a read-only export of the assembled + branch — either way this never mutates your working tree. + + Args: + composition_id: The composition id under motion/compositions/. Omit + to use the task's already-proposed composition_id. + orientation: 'vertical' or 'square'. + frame_count: How many evenly-spaced preview frames to extract (1-32). + input_props: Optional props passed into the composition at render time. + """ + return _post( + "/api/v1/do/request_render", + { + "composition_id": composition_id, + "orientation": orientation, + "frame_count": frame_count, + "input_props": input_props, + }, + timeout=_RENDER_TIMEOUT, + ) + + def draft_playbook( title: str, problem: str, @@ -962,6 +1008,7 @@ _TOOLS: dict[str, Any] = { "notify": notify, "evidence": evidence, "request_sandbox": request_sandbox, + "request_render": request_render, "progress": progress, "notify_list": notify_list, "notify_get": notify_get, diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 53e1c3f0..362075e5 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -77,6 +77,7 @@ from roboco.services.task import ( ROADMAP_SOURCE, SELF_HEAL_SOURCE, VIDEO_HELD_SOURCES, + VIDEO_SOURCE, X_FEATURE_EXPLORATION_SOURCE, X_SOURCES, ) @@ -14268,6 +14269,28 @@ If the fast path refuses (a gate it checks is not actually met), the state, f'Call evidence(task_id="{task_id}") to check status.' ) + def _video_prompt_block(self) -> str: + """Video-authoring instructions appended for a ``source=VIDEO_SOURCE`` + task: build + propose the composition, then render it and eyeball + every keyframe — a clean source review is not evidence the RENDERED + clip is right; the frames are.""" + return """ +## VIDEO TASK — verify the RENDERED clip, not just the source + +1. Build/extend the composition under motion/compositions// per the brief. +2. propose_video(composition_id="", x_caption="...", tiktok_caption="...", + platforms=[...], input_props={}) — once. +3. request_render() renders your actual composition and + returns keyframe PNG paths — Read every one. Confirm each scene/feature + named in the brief appears fully and legibly, and that the authored + data-duration actually covers all scenes (nothing cut off or rushed). +4. A frame doesn't prove it: fix the composition, then request_render() + again. Repeat until every frame checks out. + +i_am_done() refuses without a stamped render preview — a clean self-review of +the source is not enough; the rendered frames are the evidence. +""" + async def _build_dev_prompt(self, task: dict[str, Any]) -> str: """Build state-aware initial prompt for a developer.""" task_id = task.get("id", "unknown") @@ -14298,6 +14321,9 @@ If the fast path refuses (a gate it checks is not actually met), the instructions = self._get_workflow_instructions( workflow_state, task_id, open_findings_block ) + video_block = ( + self._video_prompt_block() if task.get("source") == VIDEO_SOURCE else "" + ) # The task spec travels with the prompt so the dev starts with the # actual ask (file:line targets, constraints, the intake's rationale) @@ -14320,7 +14346,7 @@ and constraints come from the intake analysis and PM decomposition. Re-articulat only the HOW (the solution); the WHAT is already decided upstream. {instructions} - +{video_block} Start by calling evidence(task_id="{task_id}") for full details, acceptance criteria, and the upstream parent/ancestor context (the original intake analysis). diff --git a/roboco/services/gateway/choreographer/_impl.py b/roboco/services/gateway/choreographer/_impl.py index 9ef394ce..b1ee2dbc 100644 --- a/roboco/services/gateway/choreographer/_impl.py +++ b/roboco/services/gateway/choreographer/_impl.py @@ -56,6 +56,7 @@ from roboco.services.gateway.remediation import ( hint_for_missing_qa_notes, hint_for_missing_reflect, hint_for_open_findings, + hint_for_render_preview, hint_for_short_dev_notes, hint_for_short_doc_notes, hint_for_short_pr_reviewer_notes, @@ -2856,12 +2857,20 @@ class Choreographer: async def _fast_path_rejection(self, ctx: _IAmDoneContext) -> Any: """The fast path's ordered rejection cascade; None means proceed. - Order matters: notes first (cheap, no writes), then finding + Order matters: notes first (cheap, no writes), then the video + render-preview check (cheap, no writes — a video task cannot fast- + path around looking at the rendered artifact), then finding resolution + guards, then the open-findings re-check, then the quality verdict with its toolchain backstop. """ if soup := self._soup_reason(ctx.notes, "notes", 4): return soup + if getattr( + ctx.task, "source", None + ) == markers.VIDEO_TASK_SOURCE and not markers.get_render_preview(ctx.task): + return await self._build_tracing_gap( + ctx.agent_id, ctx.task_id, ["render_preview"], task=ctx.task + ) await self._apply_resolved_findings(ctx) guards = ( lambda: self._check_submit_qa_field_gates( @@ -3477,6 +3486,7 @@ class Choreographer: "submit_verification automatically; if you see this gap, " "retry i_am_done after the previous call returned." ), + "render_preview": hint_for_render_preview(), } return simple_hints.get(missing_key) diff --git a/roboco/services/gateway/choreographer/qa.py b/roboco/services/gateway/choreographer/qa.py index 9d37fca2..83352853 100644 --- a/roboco/services/gateway/choreographer/qa.py +++ b/roboco/services/gateway/choreographer/qa.py @@ -202,6 +202,28 @@ class QAMixin(_Base): return [{"could_not_run": True, "reason": reason}] return list(result.get("findings", [])) + @staticmethod + def _qa_video_context(t: Any) -> dict[str, Any] | None: + """QA-facing artifact context for a video-authoring task. + + None for every non-video task. Carries the composition id (from the + ``video_draft`` marker) + the latest ``request_render`` preview so QA + is pointed at the rendered artifact instead of the source alone. + """ + if getattr(t, "source", None) != markers.VIDEO_TASK_SOURCE: + return None + draft = markers.get_video_draft(t) or {} + return { + "composition_id": draft.get("composition_id"), + "render_preview": markers.get_render_preview(t), + "note": ( + "This task ships a rendered video. Call request_render to " + "render the PR branch state, then Read every returned frame " + "image — verify each acceptance criterion's scene appears " + "fully and legibly. Do not pass on source reading alone." + ), + } + async def _build_qa_claim_evidence( self, qa_agent_id: UUID, t: Any, task_id: UUID ) -> Any: @@ -264,6 +286,7 @@ class QAMixin(_Base): prior_findings=prior_findings, parent_context=parent_context, collision_context=collision_context, + video_context=self._qa_video_context(t), ) async def _verify_qa_owner( diff --git a/roboco/services/gateway/content_actions.py b/roboco/services/gateway/content_actions.py index 913cc2ea..f4cbe629 100644 --- a/roboco/services/gateway/content_actions.py +++ b/roboco/services/gateway/content_actions.py @@ -11,9 +11,15 @@ Pure orchestration; no DB writes outside what the underlying services do. from __future__ import annotations +import asyncio import contextlib +import io import re +import shutil +import tarfile from dataclasses import dataclass +from datetime import UTC, datetime +from pathlib import Path from typing import TYPE_CHECKING, Any, ClassVar import structlog @@ -32,6 +38,7 @@ from roboco.services.gateway.evidence_builder import build_evidence_for_task from roboco.services.x_client import MAX_TWEET_CHARS if TYPE_CHECKING: + from collections.abc import Callable from uuid import UUID from roboco.foundation.identity import Team @@ -306,6 +313,19 @@ class ContentActionsDeps: orchestrator: Any = None +@dataclass(frozen=True) +class _RenderSource: + """A request_render source: a directory containing ``motion/``, plus its + provenance. ``cleanup`` (set only for a QA branch-export scratch dir) is + always called by request_render's ``finally``, dev or QA alike.""" + + root: Path + head_sha: str | None + dirty: bool + kind: str # "workspace" (dev's own tree) | "branch" (QA's read-only export) + cleanup: Callable[[], None] | None = None + + _VALID_NOTIFY_PRIORITIES: frozenset[str] = frozenset(p.value for p in _comms.Priority) # The Board roles that may author a pitch (a product proposal for CEO approval). _PITCH_ROLES: frozenset[str] = frozenset({"product_owner", "head_marketing"}) @@ -2195,6 +2215,452 @@ class ContentActions: context_briefing={}, ) + async def _render_active_video_task( + self, agent_id: UUID + ) -> tuple[Any, Envelope | None]: + """request_render's task guard: an active, project-bound video- + authoring task, or a clean invalid_state rejection.""" + from roboco.services.task import VIDEO_SOURCE + + t = await self.task.get_active_task_for_agent(agent_id) + if t is None or t.project_id is None or t.source != VIDEO_SOURCE: + return None, Envelope.invalid_state( + message="no active video-authoring task assigned to you", + remediate=( + "request_render is only available on a video-authoring " + "task — claim your assigned authoring task first" + ), + context_briefing={}, + ) + return t, None + + @staticmethod + def _render_resolve_composition_id( + task: Any, composition_id: str | None + ) -> tuple[str, Envelope | None]: + """Explicit ``composition_id``, else the task's ``video_draft`` + marker's, validated with the SAME charset regex ``propose_video`` + enforces so an unrenderable id is refused here, not deep inside the + sidecar call.""" + resolved = composition_id or (markers.get_video_draft(task) or {}).get( + "composition_id" + ) + if not resolved or not str(resolved).strip(): + return "", Envelope.incomplete_input( + missing=["composition_id"], + field_hints={"composition_id": "the HyperFrames composition id"}, + remediate=( + "pass composition_id explicitly, or call propose_video " + "first so it's on the task's video_draft marker" + ), + context_briefing={}, + ) + resolved = str(resolved).strip() + if not _COMPOSITION_ID_RE.fullmatch(resolved): + return "", Envelope.invalid_state( + message=f"composition_id {resolved!r} is not renderable", + remediate=( + "letters, digits, '_' or '-' with optional interior dots — " + "match the directory name under motion/compositions/" + ), + context_briefing={}, + ) + return resolved, None + + _RENDER_ORIENTATIONS: ClassVar[frozenset[str]] = frozenset({"vertical", "square"}) + _RENDER_MAX_FRAMES: ClassVar[int] = 32 + + @classmethod + def _render_validate_params( + cls, orientation: str, frame_count: int + ) -> Envelope | None: + """``orientation``/``frame_count`` bounds guard, folded into one + rejection point so ``request_render`` keeps one return per guard.""" + if orientation not in cls._RENDER_ORIENTATIONS: + return Envelope.invalid_state( + message=f"orientation {orientation!r} must be 'vertical' or 'square'", + remediate="pass orientation='vertical' or orientation='square'", + context_briefing={}, + ) + if not isinstance(frame_count, int) or not ( + 1 <= frame_count <= cls._RENDER_MAX_FRAMES + ): + bound = cls._RENDER_MAX_FRAMES + return Envelope.invalid_state( + message=f"frame_count {frame_count!r} must be an integer 1-{bound}", + remediate=f"pass frame_count between 1 and {bound}", + context_briefing={}, + ) + return None + + @staticmethod + def _render_resolve_input_props( + task: Any, input_props: dict[str, Any] | None + ) -> dict[str, Any]: + if input_props is not None: + return input_props + draft = markers.get_video_draft(task) or {} + return draft.get("input_props") or draft.get("suggested_input_props") or {} + + @staticmethod + async def _render_git_rev(workspace: Path, ref: str) -> str | None: + """Best-effort ``git rev-parse `` in ``workspace``; ``None`` on + any failure (missing ref, missing dir, no git binary). The render + preview's provenance stamp is best-effort — a git hiccup must never + block the render response itself.""" + try: + proc = await asyncio.create_subprocess_exec( + "git", + "-C", + str(workspace), + "rev-parse", + ref, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + out, _ = await proc.communicate() + except OSError: + return None + if proc.returncode != 0: + return None + sha = out.decode().strip() + return sha or None + + @classmethod + async def _render_git_head_and_dirty( + cls, workspace: Path + ) -> tuple[str | None, bool]: + """Best-effort ``(HEAD sha, has-uncommitted-changes)`` for a dev's own + working tree. ``(None, False)`` on any failure.""" + head = await cls._render_git_rev(workspace, "HEAD") + if head is None: + return None, False + try: + proc = await asyncio.create_subprocess_exec( + "git", + "-C", + str(workspace), + "status", + "--porcelain", + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + out, _ = await proc.communicate() + except OSError: + return head, False + dirty = bool(out.decode().strip()) if proc.returncode == 0 else False + return head, dirty + + async def _render_dev_source( + self, agent_id: UUID, agent_slug: str, task: Any, project: Any + ) -> tuple[Any, Envelope | None]: + """A developer's own working tree — the per-task worktree when one + exists on disk, else the clone root (F123).""" + from roboco.services.workspace import WorkspaceError + + agent = await self.task.agent_for(agent_id) + if agent is None or not agent.team: + return None, Envelope.invalid_state( + message="your team could not be resolved", + remediate="ensure your agent record has a team, then retry", + context_briefing={}, + ) + try: + clone_root = self.workspace.get_clone_root_path( + project.slug, agent.team, agent_slug + ) + worktree = self.workspace.get_worktree_path( + project.slug, agent.team, agent_slug, task.id.hex[:8] + ) + except WorkspaceError as exc: + return None, Envelope.invalid_state( + message=f"could not resolve your workspace path: {exc}", + remediate="retry request_render; escalate to your PM if it persists", + context_briefing={}, + ) + root = worktree if worktree.exists() else clone_root + head_sha, dirty = await self._render_git_head_and_dirty(root) + return ( + _RenderSource(root=root, head_sha=head_sha, dirty=dirty, kind="workspace"), + None, + ) + + async def _render_qa_source( + self, task: Any, project: Any + ) -> tuple[Any, Envelope | None]: + """QA never renders from a working tree: a read-only export of the + assembled branch's ``motion/`` subtree via + ``WorkspaceService.export_branch_motion``.""" + from roboco.services.workspace import WorkspaceError + + branch = getattr(task, "branch_name", None) + if not branch: + return None, Envelope.invalid_state( + message="task has no recorded branch to export for a QA render", + remediate=( + "the assembled PR's branch must exist before requesting a render" + ), + context_briefing={}, + ) + try: + scratch = await self.workspace.export_branch_motion(project, branch) + except WorkspaceError as exc: + return None, Envelope.invalid_state( + message=f"could not export branch {branch!r} for render: {exc}", + remediate=( + "ensure the branch is pushed to origin, then retry request_render" + ), + context_briefing={}, + ) + read_clone = await self.workspace.ensure_read_clone(project.slug) + head_sha = await self._render_git_rev( + read_clone, f"refs/remotes/origin/{branch}" + ) + + def _cleanup() -> None: + shutil.rmtree(scratch, ignore_errors=True) + + return ( + _RenderSource( + root=scratch, + head_sha=head_sha, + dirty=False, + kind="branch", + cleanup=_cleanup, + ), + None, + ) + + async def _render_resolve_source( + self, agent_id: UUID, agent_slug: str, task: Any, project: Any + ) -> tuple[Any, Envelope | None]: + """Dispatch the render SOURCE by the caller's role: developer → their + own tree; QA → a read-only branch export; anyone else → refused.""" + role = await self._caller_role(agent_id) + if role == "developer": + return await self._render_dev_source(agent_id, agent_slug, task, project) + if role == "qa": + return await self._render_qa_source(task, project) + return None, Envelope.not_authorized( + message=f"role {role!r} may not request a render", + remediate="request_render is developer/QA only", + context_briefing={}, + ) + + async def _render_resolve_project_and_source( + self, agent_id: UUID, agent_slug: str, task: Any + ) -> tuple[Any, Envelope | None]: + """Resolve the task's project, then the caller's render source — + bundled into one combined-rejection helper so ``request_render``'s + own return count stays under the xenon/PLR0911 budget. Success value + is a ``(project, source)`` pair.""" + from roboco.services.project import get_project_service + + project = await get_project_service(self.task.session).get(task.project_id) + if project is None: + return None, Envelope.invalid_state( + message="task's project could not be resolved", + remediate="retry shortly; the project record may be mid-update", + context_briefing={}, + ) + source, rejection = await self._render_resolve_source( + agent_id, agent_slug, task, project + ) + if rejection is not None: + return None, rejection + return (project, source), None + + def _render_extract_frames( + self, project_slug: str, task_id: Any, orientation: str, frames_tar_gz: bytes + ) -> list[str]: + """Extract the sidecar's frames tar.gz to the container-shared + preview dir, wiping any stale render first; returns sorted absolute + frame paths. Every agent container mounts the same /data/workspaces + volume, so this path is identical inside every container regardless + of who rendered — that's what lets a dev render and a QA (or PM) + read the same frames from their own container.""" + out_dir = ( + Path(settings.workspaces_root) + / project_slug + / ".previews" + / task_id.hex[:8] + / orientation + ) + if out_dir.exists(): + shutil.rmtree(out_dir) + out_dir.mkdir(parents=True, exist_ok=True) + with tarfile.open(fileobj=io.BytesIO(frames_tar_gz)) as tar: + tar.extractall(out_dir, filter="data") + return sorted(str(p) for p in out_dir.rglob("*") if p.is_file()) + + async def _render_execute( + self, + *, + task: Any, + project: Any, + agent_slug: str, + resolved_id: str, + resolved_props: dict[str, Any], + orientation: str, + frame_count: int, + source: Any, + ) -> Envelope: + """The render call + frame extraction + marker stamp, once every + guard in ``request_render`` has passed. Always cleans up a QA + scratch-dir source (dev sources have no cleanup callback).""" + try: + comp_dir = source.root / "motion" / "compositions" / resolved_id + if not comp_dir.is_dir(): + expected = f"motion/compositions/{resolved_id}/" + return Envelope.invalid_state( + message=f"no composition found at {expected}", + remediate=( + f"build the composition under {expected} first " + "(propose_video / commit it), then retry request_render" + ), + context_briefing={}, + ) + from roboco.services.video_renderer_client import ( + VideoRendererError, + get_video_renderer, + ) + + try: + frames_tar_gz, duration = await get_video_renderer().render_frames( + str(source.root / "motion"), + composition_id=resolved_id, + input_props=resolved_props, + orientation=orientation, + frame_count=frame_count, + ) + except VideoRendererError as exc: + return Envelope.invalid_state( + message=f"render failed: {exc}", + remediate=( + "retry request_render shortly; escalate to your PM if " + "it keeps failing" + ), + context_briefing={}, + ) + frames = self._render_extract_frames( + project.slug, task.id, orientation, frames_tar_gz + ) + payload = { + "at": datetime.now(UTC).isoformat(), + "composition_id": resolved_id, + "orientation": orientation, + "frame_count": frame_count, + "duration_seconds": duration, + "frames": frames, + "head_sha": source.head_sha, + "dirty": source.dirty, + "rendered_by": agent_slug, + "source": source.kind, + } + markers.set_render_preview(task, payload) + # The post-completion render loop keys on video_draft.composition_id + # — a dev who only ever passed composition_id explicitly (never + # propose_video) must still leave it stamped, or the loop skips the + # completed task silently (proven live on task 1dae04a7). + draft = markers.get_video_draft(task) or {} + if not draft.get("composition_id"): + markers.set_video_draft(task, {**draft, "composition_id": resolved_id}) + await self.task.session.flush() + await self._touch_heartbeat(task.id) + return Envelope.ok( + status=str(task.status), + task_id=str(task.id), + next=( + "Read every frames[] path with your file tools; if any " + "scene is missing or clipped, fix the composition and " + "call request_render again." + ), + evidence={ + **payload, + "note": ( + "Read each frame image and verify every scene/feature " + "from the brief appears fully and legibly before " + "i_am_done." + ), + }, + context_briefing={}, + ) + finally: + if source.cleanup is not None: + source.cleanup() + + async def request_render( + self, + *, + agent_id: UUID, + composition_id: str | None = None, + orientation: str = "vertical", + frame_count: int = 8, + input_props: dict[str, Any] | None = None, + ) -> Envelope: + """Render a video composition to a strip of preview frames the + caller reads with file tools — verifying the RENDERED artifact, not + just the HyperFrames source, which can look plausible and still + render wrong (missing scene, clipped layout, wrong text). + + Guards, in order: video engine flag off; caller has no active, + project-bound video-authoring task (`_render_active_video_task`); + the renderer sidecar unconfigured (`video_renderer_base_url`); + `composition_id` unresolvable or failing `propose_video`'s own + charset regex, or `orientation`/`frame_count` out of range; the + caller's role-based SOURCE (`_render_resolve_source` — a developer's + own tree, or QA's read-only branch export, never a QA working tree); + the resolved source missing `motion/compositions//`; the sidecar + call itself (`VideoRendererError` -> a retryable rejection). On + success, extracts the returned frames to a container-shared + `.previews///` path and stamps `render_preview` — + the marker `i_am_done`'s RENDER_VERIFIED gate checks. + """ + if not settings.video_engine_enabled: + return Envelope.invalid_state( + message="the video engine is disabled", + remediate="ROBOCO_VIDEO_ENGINE_ENABLED is off — nothing to render", + context_briefing={}, + ) + t, rejection = await self._render_active_video_task(agent_id) + if rejection is not None: + return rejection + renderer_rej = ( + None + if settings.video_renderer_base_url.strip() + else Envelope.invalid_state( + message="the video-renderer sidecar is not configured", + remediate="ask the CEO to set ROBOCO_VIDEO_RENDERER_BASE_URL", + context_briefing={}, + ) + ) + resolved_id, rej_id = self._render_resolve_composition_id(t, composition_id) + rej_params = self._render_validate_params(orientation, frame_count) + rejection = renderer_rej or rej_id or rej_params + if rejection is not None: + return rejection + resolved_props = self._render_resolve_input_props(t, input_props) + + from roboco.agents_config import _resolve_to_slug + + agent_slug = _resolve_to_slug(str(agent_id)) + resolved, rejection = await self._render_resolve_project_and_source( + agent_id, agent_slug, t + ) + if rejection is not None: + return rejection + project, source = resolved + return await self._render_execute( + task=t, + project=project, + agent_slug=agent_slug, + resolved_id=resolved_id, + resolved_props=resolved_props, + orientation=orientation, + frame_count=frame_count, + source=source, + ) + # ========================================================================= # Wave 1 — pre-gateway parity restoration # ========================================================================= diff --git a/roboco/services/gateway/evidence_builder.py b/roboco/services/gateway/evidence_builder.py index ee46335a..6fa9458a 100644 --- a/roboco/services/gateway/evidence_builder.py +++ b/roboco/services/gateway/evidence_builder.py @@ -26,6 +26,7 @@ _EVIDENCE_OMIT_WHEN_EMPTY = ( "parent_context", "description", "collision_context", + "video_context", ) @@ -64,6 +65,10 @@ class EvidencePayload: # declared-vs-actual drift. Empty for a root or a task with no colliding # siblings; the block is omitted when empty (zero token cost). collision_context: list[dict[str, Any]] = field(default_factory=list) + # QA-facing artifact context for a video-authoring task (composition_id + + # the latest request_render preview + a verification instruction). None + # for every non-video task; omitted from the dict when empty. + video_context: dict[str, Any] | None = None def as_dict(self) -> dict[str, Any]: data = asdict(self) @@ -167,6 +172,7 @@ def build_evidence_for_task( prior_findings: list[Any] | None = None, parent_context: list[dict[str, Any]] | None = None, collision_context: list[dict[str, Any]] | None = None, + video_context: dict[str, Any] | None = None, ) -> EvidencePayload: """Compose an EvidencePayload from a Task model + supplemental data. @@ -177,7 +183,9 @@ def build_evidence_for_task( reads the intake's original analysis verbatim. ``collision_context`` is the prebuilt collision-map block (the caller fetches siblings + actual files and runs the pure ``build_collision_context``); passed through - verbatim so this module stays DB-free. + verbatim so this module stays DB-free. ``video_context`` is the + prebuilt video-artifact block (caller-assembled from the task's + ``video_draft`` marker + render preview); passed through verbatim. """ return EvidencePayload( pr_number=task.pr_number, @@ -194,6 +202,7 @@ def build_evidence_for_task( revision_findings=render_findings(revision_findings), prior_findings=render_findings(prior_findings), collision_context=list(collision_context or []), + video_context=video_context, ) diff --git a/roboco/services/gateway/remediation.py b/roboco/services/gateway/remediation.py index bebd5118..d1932c91 100644 --- a/roboco/services/gateway/remediation.py +++ b/roboco/services/gateway/remediation.py @@ -101,6 +101,14 @@ def hint_for_short_pr_reviewer_notes(*, min_chars: int) -> str: ) +def hint_for_render_preview() -> str: + return ( + "call request_render() to render this task's PR branch, Read every " + "returned frame image, and verify each scene/feature from the brief " + "appears fully and legibly — then retry i_am_done" + ) + + def hint_for_short_quick_context(*, min_chars: int, task_id: str) -> str: return ( f"your quick_context section is empty or under {min_chars} chars. Before " diff --git a/roboco/services/gateway/role_config.py b/roboco/services/gateway/role_config.py index c7d60577..a40ca4ec 100644 --- a/roboco/services/gateway/role_config.py +++ b/roboco/services/gateway/role_config.py @@ -59,6 +59,10 @@ _DEV_DO = ( # On-demand sandbox DB/Redis/Mongo — carried unconditionally (declarative # manifest), gated for real by request_sandbox's project opt-in check. "request_sandbox", + # Render the video composition to preview frames (dev's own working + # tree); manifest carries it unconditionally, gated for real by + # request_render's active-video-task + flag checks. + "request_render", *_NOTIFY_RECEIVER, ) @@ -69,6 +73,9 @@ _QA_DO = ( "evidence", "draft_playbook", "request_sandbox", + # QA's render source is a read-only branch export, never a working tree + # (see request_render/_render_qa_source); gated the same way as above. + "request_render", *_NOTIFY_RECEIVER, ) diff --git a/roboco/services/task.py b/roboco/services/task.py index 2c00bf9c..af9aa777 100644 --- a/roboco/services/task.py +++ b/roboco/services/task.py @@ -642,8 +642,10 @@ X_SOURCES = (X_POST_SOURCE, X_REPLY_SOURCE, X_FEATURE_SOURCE) # Source tag for a video-authoring task: the VideoEngine assigns this to a # UX/UI dev to build a HyperFrames composition. Unlike X_SOURCES above it IS # dispatched — a normal, pre-assigned delivery task like any other cell code -# task — so it stays out of every held-source skip bucket. -VIDEO_SOURCE = "video" +# task — so it stays out of every held-source skip bucket. Canonical string +# lives in foundation (markers.VIDEO_TASK_SOURCE) so the tracing gate can +# key on it without importing this layer. +VIDEO_SOURCE = markers.VIDEO_TASK_SOURCE # Source tag for a held video-post draft: mp4s + captions ready for CEO # approval. Like release_manager/X_SOURCES this is NEVER dispatched — held diff --git a/roboco/services/video_engine.py b/roboco/services/video_engine.py index 0e17c8ca..192891eb 100644 --- a/roboco/services/video_engine.py +++ b/roboco/services/video_engine.py @@ -49,6 +49,8 @@ _AUTHORING_ACCEPTANCE_CRITERIA = [ "Captions within platform limits", "Composition follows motion/README.md's design bar and uses the " "panel-demo kit register where the occasion shows the product", + "request_render preview frames verified — every scene in the brief " + "appears fully and legibly in the rendered cut", ] _POST_ACCEPTANCE_CRITERIA = ["CEO approves or rejects the draft"] @@ -64,7 +66,8 @@ _MOTION_DESIGN_POINTER = ( "Before authoring: read motion/README.md's design bar and motion/kit/" "README.md. Build in the panel-demo register on motion/kit/ — extend " "compositions/panel-demo/ rather than starting from scratch or shipping " - "a text card." + "a text card. Before submitting: call request_render and read every " + "returned frame to verify the RENDERED cut, not just the source." ) @@ -143,6 +146,26 @@ def _release_video_brief(version: str, changelog: str, highlights: list[str]) -> return "\n\n".join(parts) +def _reauthor_brief(reason: str, draft: dict[str, Any]) -> str: + """The revision brief for a CEO-rejected cut: the verbatim rejection + feedback, a revise-in-place pointer at the existing composition (when + known), then the original brief/script for context.""" + parts = [ + "REVISION of a CEO-rejected cut. CEO rejection feedback " + f"(address every point): {reason}" + ] + composition_id = draft.get("composition_id") + if composition_id: + parts.append( + f"Revise the EXISTING composition motion/compositions/{composition_id}/ " + "in place — do not start a new composition." + ) + original = draft.get("brief") or draft.get("script") or "" + if original: + parts.append(original) + return "\n\n".join(parts) + + class VideoEngine(BaseService): """Open video-authoring tasks (event hooks + on-demand), both gated.""" @@ -448,6 +471,77 @@ class VideoEngine(BaseService): ) return task + # ---- reject -> re-author (CEO feedback loop) --------------------------- + + async def _resolve_reauthor_project( + self, post_task: TaskTable, draft: dict[str, Any] + ) -> UUID | None: + """The project to re-author against: the rejected post's own + ``project_id`` (the normal case), else its source authoring task's + (via the draft's ``source_task_id``) — a defensive fallback for a + draft that somehow landed without one.""" + if post_task.project_id is not None: + return cast("UUID", post_task.project_id) + source_task_id = draft.get("source_task_id") + if not source_task_id: + return None + source_task = await get_task_service(self.session).get( + cast("UUID", source_task_id) + ) + return cast("UUID", source_task.project_id) if source_task else None + + async def reauthor_from_rejection( + self, post_task: TaskTable, reason: str + ) -> TaskTable | None: + """Route a CEO's rejection reason into a fresh authoring task that + revises the SAME composition in place, instead of the feedback going + nowhere. + + Reads the rejected ``video_post`` draft's carried-forward + ``video_draft`` marker (occasion, brief/script, composition_id, + platforms, input_props, source_task_id) and re-opens via + ``open_video_task`` under the SAME occasion — that call's own dedup + only scans OPEN drafts, so the just-cancelled post never blocks it (a + second reject while a revision is already open correctly dedups + against it instead of stacking a third). + + Best-effort: never raises. A missing draft marker, an unresolvable + project, or any other failure just logs a warning and returns None — + the caller's reject must succeed regardless of this seam. + """ + try: + draft = markers.get_video_draft(post_task) + if draft is None: + self.log.warning( + "video-engine: reauthor skipped, no video_draft marker", + task_id=str(post_task.id), + ) + return None + project_id = await self._resolve_reauthor_project(post_task, draft) + if project_id is None: + self.log.warning( + "video-engine: reauthor skipped, no project resolvable", + task_id=str(post_task.id), + ) + return None + return await self.open_video_task( + occasion=str(draft.get("occasion") or post_task.title), + script=str(draft.get("script") or ""), + platforms=list(draft.get("platforms") or []), + brief=_reauthor_brief(reason, draft), + suggested_input_props=( + draft.get("input_props") or draft.get("suggested_input_props") + ), + project_id=project_id, + ) + except Exception as exc: + self.log.warning( + "video-engine: reauthor from rejection failed", + task_id=str(post_task.id), + error=str(exc), + ) + return None + # ---- re-render (CEO-triggered retry) ----------------------------------- async def rerender(self, task_id: UUID) -> TaskTable | None: diff --git a/roboco/services/video_post_service.py b/roboco/services/video_post_service.py index c0ebd876..82534a11 100644 --- a/roboco/services/video_post_service.py +++ b/roboco/services/video_post_service.py @@ -488,7 +488,9 @@ class VideoPostService(BaseService): ) async def reject(self, task_id: UUID, reason: str) -> TaskTable | None: - """Record the CEO's reason and cancel the draft (never posted). + """Record the CEO's reason, cancel the draft (never posted), and + route the feedback into a fresh authoring task that revises the same + composition. Acquires the same post-mutex ``approve()`` holds (same key, same non-blocking acquire style) so a reject can't interleave with a @@ -534,9 +536,31 @@ class VideoPostService(BaseService): markers.set_video_reject_reason(locked, reason) locked.status = TaskStatus.CANCELLED await self.session.flush() - return locked + cancelled = locked finally: await mutex.release(token) + # Outside the lock/try-finally, after the cancel is committed/flushed: + # a reauthor failure must never fail or roll back the reject above. + if reason.strip(): + await self._reauthor_after_reject(cancelled, reason) + return cancelled + + async def _reauthor_after_reject(self, task: TaskTable, reason: str) -> None: + """Best-effort: hand the CEO's reject reason to VideoEngine so it + opens a revision authoring task. Never raises.""" + # Local import: no cycle (video_engine doesn't import this module), + # but mirrors the lazy get_video_engine import every other caller + # (release_proposal.py, x_post_service.py) uses. + from roboco.services.video_engine import get_video_engine + + try: + await get_video_engine(self.session).reauthor_from_rejection(task, reason) + except Exception as exc: + logger.warning( + "video-post reauthor-from-rejection failed for task %s: %s", + task.id, + exc, + ) def get_video_post_service( diff --git a/roboco/services/video_renderer_client.py b/roboco/services/video_renderer_client.py index d976aef1..49f85f97 100644 --- a/roboco/services/video_renderer_client.py +++ b/roboco/services/video_renderer_client.py @@ -76,7 +76,7 @@ class VideoRenderer: "video-renderer sidecar not configured (video_renderer_base_url unset)" ) tar_bytes = await asyncio.to_thread(self._tar_source, source_dir) - mp4_bytes = await self._post( + response = await self._post( tar_bytes, composition_id=composition_id, input_props=input_props, @@ -84,11 +84,45 @@ class VideoRenderer: ) return await asyncio.to_thread( self._save, - mp4_bytes, + response.content, render_key=render_key, orientation=orientation, ) + async def render_frames( + self, + source_dir: str, + *, + composition_id: str, + input_props: dict[str, Any], + orientation: str, + frame_count: int, + ) -> tuple[bytes, float]: + """Render one cut and extract ``frame_count`` preview keyframes. + + Returns the frames tar.gz bytes plus the sidecar-probed real render + duration (parsed from the ``X-Video-Duration`` response header, or + ``0.0`` when absent/unparsable — never a save, never MinIO: this is + the artifact-verification surface, not the publish path). + """ + if not self._base_url: + raise VideoRendererError( + "video-renderer sidecar not configured (video_renderer_base_url unset)" + ) + tar_bytes = await asyncio.to_thread(self._tar_source, source_dir) + response = await self._post( + tar_bytes, + composition_id=composition_id, + input_props=input_props, + orientation=orientation, + frames=frame_count, + ) + try: + duration = float(response.headers.get("X-Video-Duration", "")) + except ValueError: + duration = 0.0 + return response.content, duration + @staticmethod def _tar_source(source_dir: str) -> bytes: """Tar ``source_dir`` (the motion/ package) into an in-memory gzip archive.""" @@ -109,26 +143,31 @@ class VideoRenderer: composition_id: str, input_props: dict[str, Any], orientation: str, - ) -> bytes: - """POST the tarball + render params; return the MP4 response body. + frames: int | None = None, + ) -> httpx.Response: + """POST the tarball + render params; return the raw response. Timeout is split: connect/write/pool use the short request timeout (sending the tar), while `read` gets the long render timeout (the - sidecar renders before it writes the response body). + sidecar renders before it writes the response body). ``frames`` set + switches the sidecar to the frames-tar branch instead of the MP4. """ timeout = httpx.Timeout( settings.video_request_timeout_seconds, read=settings.video_render_timeout_seconds, ) + data = { + "composition_id": composition_id, + "orientation": orientation, + "input_props": json.dumps(input_props), + } + if frames is not None: + data["frames"] = str(frames) client = await self._http(timeout) try: response = await client.post( f"{self._base_url}/render", - data={ - "composition_id": composition_id, - "orientation": orientation, - "input_props": json.dumps(input_props), - }, + data=data, files={"source": ("motion.tar.gz", tar_bytes, "application/gzip")}, timeout=timeout, ) @@ -138,7 +177,7 @@ class VideoRenderer: raise VideoRendererError( f"render failed: HTTP {response.status_code}: {response.text[:200]}" ) - return response.content + return response @staticmethod def _save(mp4_bytes: bytes, *, render_key: str, orientation: str) -> str: diff --git a/roboco/services/workspace.py b/roboco/services/workspace.py index 1697fe2d..c4054e01 100644 --- a/roboco/services/workspace.py +++ b/roboco/services/workspace.py @@ -20,6 +20,7 @@ Example: import asyncio import contextlib +import io import json import math import os @@ -27,6 +28,7 @@ import re import shlex import shutil import subprocess +import tarfile import tempfile import time from collections.abc import Iterator @@ -1966,6 +1968,78 @@ class WorkspaceService: await asyncio.to_thread(_ensure_agent_owned, workspace) return workspace + async def export_branch_motion(self, project: Any, branch: str) -> Path: + """Export ``branch``'s ``motion/`` subtree into a fresh scratch dir — + request_render's QA source. Read-only by construction: fetches + ``origin/`` into the project's shared READ CLONE (never a + per-agent working tree) and ``git archive``s the commit directly — + no checkout, so the read clone's own pinned HEAD is undisturbed for + every other conventions-standard reader of that same clone. Caller + owns cleanup of the returned scratch dir. + + Raises WorkspaceError only when the branch itself can't be fetched + (bad ref, auth failure, network). A branch that fetches fine but has + no ``motion/`` at that commit yields an EMPTY scratch dir — the + caller's ``motion/compositions//`` existence check handles that + uniformly alongside the ordinary "missing composition" case, rather + than a second distinct error path here. + """ + from roboco.services.project import get_project_service + + project_service = get_project_service(self.session) + workspace = await self.ensure_read_clone(project.slug) + git_token = await self._read_clone_token(project_service, project.slug) + + prefix: list[str] = [] + if git_token: + import base64 + + basic = base64.b64encode(f"x-access-token:{git_token}".encode()).decode() + prefix = ["-c", f"http.extraheader=Authorization: Basic {basic}"] + + def _git(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", *prefix, *args], + cwd=str(workspace), + capture_output=True, + text=True, + check=False, + ) + + def _export() -> Path: + fetched = _git("fetch", "--no-tags", "origin", branch) + if fetched.returncode != 0: + raise WorkspaceError( + f"export_branch_motion: fetch of {branch!r} failed: " + f"{fetched.stderr.strip()}" + ) + sha = _git("rev-parse", "FETCH_HEAD").stdout.strip() + scratch = Path(tempfile.mkdtemp(prefix="render-src-")) + if not sha: + return scratch + archive = subprocess.run( + ["git", "archive", sha, "--", "motion"], + cwd=str(workspace), + capture_output=True, + check=False, + ) + if archive.returncode != 0: + # Most commonly: no motion/ tree at this commit. Leave the + # scratch dir empty so the caller's composition-path check + # reports the clean "missing composition" rejection instead + # of a second distinct error here. + logger.warning( + "export_branch_motion: git archive found no motion/ tree", + branch=branch, + stderr=archive.stderr.decode(errors="replace").strip()[:200], + ) + return scratch + with tarfile.open(fileobj=io.BytesIO(archive.stdout)) as tar: + tar.extractall(scratch, filter="data") + return scratch + + return await asyncio.to_thread(_export) + async def delete_workspace( self, project_slug: str, diff --git a/tests/foundation/test_tracing.py b/tests/foundation/test_tracing.py index af368553..b541196b 100644 --- a/tests/foundation/test_tracing.py +++ b/tests/foundation/test_tracing.py @@ -35,6 +35,7 @@ def test_requirement_enum_has_canonical_values() -> None: "pr_reviewer_notes>=min", "quick_context>=min", "findings_addressed", + "render_verified", } actual = {r.value for r in tracing.Requirement} assert actual == expected, f"Requirement drift: {actual ^ expected}" diff --git a/tests/unit/foundation/policy/test_tracing_render_verified_gate.py b/tests/unit/foundation/policy/test_tracing_render_verified_gate.py new file mode 100644 index 00000000..5ddf4101 --- /dev/null +++ b/tests/unit/foundation/policy/test_tracing_render_verified_gate.py @@ -0,0 +1,58 @@ +"""The RENDER_VERIFIED tracing requirement — i_am_done's video-artifact gate. + +Pure unit tests against ``foundation.policy.tracing`` (no DB, no +choreographer): the checker itself, its registration in +``VERB_REQUIREMENTS["i_am_done"]``, and the "non-video task is untouched" +contract. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from roboco.foundation.policy import tracing as tr +from roboco.foundation.policy.content import markers + + +def test_render_verified_is_required_by_i_am_done() -> None: + assert tr.Requirement.RENDER_VERIFIED in tr.VERB_REQUIREMENTS["i_am_done"] + + +def test_render_verified_not_required_by_submit_up_or_submit_root() -> None: + assert tr.Requirement.RENDER_VERIFIED not in tr.VERB_REQUIREMENTS["submit_up"] + assert tr.Requirement.RENDER_VERIFIED not in tr.VERB_REQUIREMENTS["submit_root"] + + +def test_non_video_task_passes_regardless_of_marker() -> None: + task = SimpleNamespace(source="code", orchestration_markers=None) + assert tr._check_render_verified(task, tr.GateContext()) == [] + + +def test_video_task_without_preview_fails() -> None: + task = SimpleNamespace(source=markers.VIDEO_TASK_SOURCE, orchestration_markers=None) + assert tr._check_render_verified(task, tr.GateContext()) == ["render_preview"] + + +def test_video_task_with_preview_passes() -> None: + task = SimpleNamespace( + source=markers.VIDEO_TASK_SOURCE, + orchestration_markers={markers.RENDER_PREVIEW: {"frames": ["a.png"]}}, + ) + assert tr._check_render_verified(task, tr.GateContext()) == [] + + +def test_i_am_done_requirements_include_the_pre_existing_set_too() -> None: + """Adding RENDER_VERIFIED must not have dropped any prior requirement.""" + required = tr.VERB_REQUIREMENTS["i_am_done"] + for expected in ( + tr.Requirement.COMMITS_AT_LEAST_ONE, + tr.Requirement.PR_OPEN, + tr.Requirement.PROGRESS_AT_LEAST_ONE, + tr.Requirement.SELF_VERIFIED, + tr.Requirement.JOURNAL_REFLECT, + tr.Requirement.JOURNAL_DURING_WORK_AT_LEAST_ONE, + tr.Requirement.ACCEPTANCE_CRITERIA_ADDRESSED, + tr.Requirement.DEV_NOTES_MIN_CHARS, + tr.Requirement.FINDINGS_ADDRESSED, + ): + assert expected in required diff --git a/tests/unit/gateway/test_possibilities_matrix.py b/tests/unit/gateway/test_possibilities_matrix.py index a2c9c8cb..464219a1 100644 --- a/tests/unit/gateway/test_possibilities_matrix.py +++ b/tests/unit/gateway/test_possibilities_matrix.py @@ -16,6 +16,7 @@ from unittest.mock import AsyncMock, MagicMock from uuid import uuid4 import pytest +from roboco.foundation.policy.content import markers from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps from roboco.services.gateway.envelope import Envelope @@ -61,6 +62,8 @@ def _t( t.pr_number = 12345 if pr_created else None t.acceptance_criteria = list(criteria) t.acceptance_criteria_status = ac_status + t.source = "code" + t.orchestration_markers = None return t @@ -334,6 +337,44 @@ async def test_fast_path_open_findings_blocks(monkeypatch: pytest.MonkeyPatch) - c.task.submit_qa.assert_not_awaited() +@pytest.mark.asyncio +async def test_fast_path_video_task_without_render_preview_rejects( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A video-authoring task with no request_render preview must not be + able to fast-path around looking at the rendered artifact.""" + c = Choreographer(_deps()) + stubs = _stub_fast_path(c, monkeypatch) + tracing_gap = AsyncMock( + return_value=Envelope.tracing_gap(missing=["render_preview"], remediate="x") + ) + monkeypatch.setattr(c, "_build_tracing_gap", tracing_gap) + ctx = _ctx() + ctx.task = _t() + ctx.task.source = markers.VIDEO_TASK_SOURCE + await c._i_am_done_fast_path(ctx) + stubs.reject.assert_awaited_once() + c.task.submit_qa.assert_not_awaited() + tracing_gap.assert_awaited_once_with( + ctx.agent_id, ctx.task_id, ["render_preview"], task=ctx.task + ) + + +@pytest.mark.asyncio +async def test_fast_path_video_task_with_render_preview_passes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = Choreographer(_deps()) + stubs = _stub_fast_path(c, monkeypatch) + ctx = _ctx() + ctx.task = _t() + ctx.task.source = markers.VIDEO_TASK_SOURCE + ctx.task.orchestration_markers = {markers.RENDER_PREVIEW: {"frames": ["a.png"]}} + await c._i_am_done_fast_path(ctx) + stubs.ok.assert_awaited_once() + c.task.submit_qa.assert_awaited_once() + + @pytest.mark.asyncio async def test_fast_path_conventions_block_rejects( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/unit/gateway/test_request_render_verb.py b/tests/unit/gateway/test_request_render_verb.py new file mode 100644 index 00000000..a9ad4d24 --- /dev/null +++ b/tests/unit/gateway/test_request_render_verb.py @@ -0,0 +1,485 @@ +"""ContentActions.request_render — render a video composition to preview +frames so an agent verifies the RENDERED artifact, not just its source. + +Guard matrix (flag off / no active task / non-video source / renderer +unconfigured / missing composition_id / bad composition_id-orientation- +frame_count / missing composition dir on disk / role gate), the dev and QA +success paths (frame extraction + marker payload shape), and a +VideoRendererError surfacing as a retryable rejection. +""" + +from __future__ import annotations + +import io +import tarfile +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock +from uuid import uuid4 + +import pytest +from roboco.config import settings +from roboco.foundation.policy.content import markers +from roboco.services.gateway.content_actions import ContentActions, ContentActionsDeps +from roboco.services.video_renderer_client import VideoRendererError + + +def _make_actions( + *, + task_obj: MagicMock | None, + role: str = "developer", + team: str | None = "ux_ui", + workspace: MagicMock | None = None, +) -> tuple[ContentActions, MagicMock]: + task = AsyncMock() + task.get_active_task_for_agent.return_value = task_obj + task.session = MagicMock() + task.session.flush = AsyncMock() + task.heartbeat = AsyncMock() + agent = MagicMock() + agent.role = role + agent.team = team + task.agent_for = AsyncMock(return_value=agent) + deps = ContentActionsDeps( + task=task, + git=MagicMock(), + a2a=MagicMock(), + journal=MagicMock(), + workspace=workspace or MagicMock(), + notifications=MagicMock(), + ) + return ContentActions(deps), task + + +def _task( + *, + project_id: object | None = uuid4(), + source: str = markers.VIDEO_TASK_SOURCE, + branch_name: str | None = "feature/ux_ui/ABCD1234", + draft: dict[str, object] | None = None, +) -> MagicMock: + t = MagicMock() + t.id = uuid4() + t.project_id = project_id + t.status = "in_progress" + t.source = source + t.branch_name = branch_name + t.orchestration_markers = {"video_draft": draft} if draft else None + return t + + +def _stub_project( + monkeypatch: pytest.MonkeyPatch, *, slug: str = "demo-project" +) -> MagicMock: + project = MagicMock(slug=slug, git_url="https://example.invalid/demo.git") + project_service = MagicMock() + project_service.get = AsyncMock(return_value=project) + monkeypatch.setattr( + "roboco.services.project.get_project_service", lambda _s: project_service + ) + return project + + +def _stub_renderer( + monkeypatch: pytest.MonkeyPatch, + *, + frames_tar_gz: bytes = b"", + duration: float = 2.5, + error: Exception | None = None, +) -> MagicMock: + renderer = MagicMock() + if error is not None: + renderer.render_frames = AsyncMock(side_effect=error) + else: + renderer.render_frames = AsyncMock(return_value=(frames_tar_gz, duration)) + monkeypatch.setattr( + "roboco.services.video_renderer_client.get_video_renderer", + lambda: renderer, + ) + return renderer + + +def _make_frames_tar(names: list[str]) -> bytes: + """An in-memory tar.gz of a few frame files, matching what render_frames + returns on success — the render loop extracts this straight to disk.""" + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + for name in names: + data = f"fake-png-bytes-{name}".encode() + info = tarfile.TarInfo(name=name) + info.size = len(data) + tar.addfile(info, io.BytesIO(data)) + return buf.getvalue() + + +def _arm( + monkeypatch: pytest.MonkeyPatch, *, renderer_url: str = "http://sidecar:3001" +) -> None: + monkeypatch.setattr(settings, "video_engine_enabled", True) + monkeypatch.setattr(settings, "video_renderer_base_url", renderer_url) + + +# --------------------------------------------------------------------------- # +# guard matrix +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_flag_off_refuses_before_task_lookup( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "video_engine_enabled", False) + actions, task_svc = _make_actions(task_obj=None) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + task_svc.get_active_task_for_agent.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_no_active_task_refused(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "video_engine_enabled", True) + actions, _task_svc = _make_actions(task_obj=None) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + + +@pytest.mark.asyncio +async def test_non_video_source_refused(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "video_engine_enabled", True) + t = _task(source="chore") + actions, _task_svc = _make_actions(task_obj=t) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + assert "video-authoring" in (env.message or "") + (env.remediate or "") + + +@pytest.mark.asyncio +async def test_renderer_unconfigured_refused(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "video_engine_enabled", True) + monkeypatch.setattr(settings, "video_renderer_base_url", "") + t = _task() + actions, _task_svc = _make_actions(task_obj=t) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + assert "ROBOCO_VIDEO_RENDERER_BASE_URL" in (env.remediate or "") + + +@pytest.mark.asyncio +async def test_missing_composition_id_is_incomplete_input( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _arm(monkeypatch) + t = _task(draft=None) + actions, _task_svc = _make_actions(task_obj=t) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "incomplete_input" + assert "composition_id" in (env.missing or []) + + +@pytest.mark.asyncio +async def test_bad_composition_id_regex_rejected( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _arm(monkeypatch) + t = _task() + actions, _task_svc = _make_actions(task_obj=t) + + env = await actions.request_render(agent_id=uuid4(), composition_id="bad id!") + + assert env.error == "invalid_state" + assert "not renderable" in (env.message or "") + + +@pytest.mark.asyncio +async def test_bad_orientation_rejected(monkeypatch: pytest.MonkeyPatch) -> None: + _arm(monkeypatch) + t = _task() + actions, _task_svc = _make_actions(task_obj=t) + + env = await actions.request_render( + agent_id=uuid4(), composition_id="release-v1", orientation="landscape" + ) + + assert env.error == "invalid_state" + + +@pytest.mark.asyncio +async def test_frame_count_out_of_range_rejected( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _arm(monkeypatch) + t = _task() + actions, _task_svc = _make_actions(task_obj=t) + + too_low = await actions.request_render( + agent_id=uuid4(), composition_id="release-v1", frame_count=0 + ) + too_high = await actions.request_render( + agent_id=uuid4(), composition_id="release-v1", frame_count=33 + ) + + assert too_low.error == "invalid_state" + assert too_high.error == "invalid_state" + + +@pytest.mark.asyncio +async def test_missing_composition_dir_on_disk_rejected( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _arm(monkeypatch) + monkeypatch.setattr(settings, "workspaces_root", str(tmp_path / "workspaces")) + _stub_project(monkeypatch) + clone_root = tmp_path / "clone" + clone_root.mkdir(parents=True) # no motion/ tree at all + workspace = MagicMock() + workspace.get_clone_root_path.return_value = clone_root + workspace.get_worktree_path.return_value = clone_root / ".worktrees" / "x" + + t = _task(draft={"composition_id": "release-v1"}) + actions, _task_svc = _make_actions(task_obj=t, workspace=workspace) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + assert "motion/compositions/release-v1" in (env.message or "") + + +@pytest.mark.asyncio +async def test_other_role_not_authorized(monkeypatch: pytest.MonkeyPatch) -> None: + _arm(monkeypatch) + _stub_project(monkeypatch) + t = _task(draft={"composition_id": "release-v1"}) + actions, _task_svc = _make_actions(task_obj=t, role="documenter", team="ux_ui") + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "not_authorized" + + +# --------------------------------------------------------------------------- # +# dev happy path — own working tree +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_dev_happy_path_extracts_frames_and_stamps_marker( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _arm(monkeypatch) + monkeypatch.setattr(settings, "workspaces_root", str(tmp_path / "workspaces")) + _stub_project(monkeypatch, slug="demo-project") + + clone_root = tmp_path / "clone" + (clone_root / "motion" / "compositions" / "release-v1").mkdir(parents=True) + workspace = MagicMock() + workspace.get_clone_root_path.return_value = clone_root + workspace.get_worktree_path.return_value = clone_root / ".worktrees" / "deadbeef" + + t = _task(draft={"composition_id": "release-v1"}) + actions, task_svc = _make_actions( + task_obj=t, role="developer", team="ux_ui", workspace=workspace + ) + expected_frame_count = 2 + expected_duration = 3.2 + default_frame_count = 8 + tar_bytes = _make_frames_tar(["frame-0.png", "frame-1.png"]) + _stub_renderer(monkeypatch, frames_tar_gz=tar_bytes, duration=expected_duration) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error is None + assert env.status == t.status + assert env.task_id == str(t.id) + assert env.evidence is not None + frames = env.evidence["frames"] + assert len(frames) == expected_frame_count + for p in frames: + assert Path(p).is_file() + assert env.evidence["duration_seconds"] == expected_duration + assert env.evidence["source"] == "workspace" + assert env.evidence["dirty"] is False + assert env.evidence["rendered_by"] + assert "note" in env.evidence + assert "frames[]" in (env.next or "") + + payload = markers.get_render_preview(t) + assert payload is not None + assert payload["composition_id"] == "release-v1" + assert payload["orientation"] == "vertical" + assert payload["frame_count"] == default_frame_count + assert payload["duration_seconds"] == expected_duration + assert payload["source"] == "workspace" + assert payload["dirty"] is False + assert payload["frames"] == frames + assert "at" in payload + task_svc.session.flush.assert_awaited() + task_svc.heartbeat.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_explicit_composition_id_backfills_video_draft( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A dev who never called propose_video must still leave + video_draft.composition_id stamped — the post-completion render loop + keys on it and skips silently when absent (proven live).""" + _arm(monkeypatch) + monkeypatch.setattr(settings, "workspaces_root", str(tmp_path / "workspaces")) + _stub_project(monkeypatch, slug="demo-project") + + clone_root = tmp_path / "clone" + (clone_root / "motion" / "compositions" / "release-v1").mkdir(parents=True) + workspace = MagicMock() + workspace.get_clone_root_path.return_value = clone_root + workspace.get_worktree_path.return_value = clone_root / ".worktrees" / "deadbeef" + + t = _task(draft=None) + actions, _ = _make_actions( + task_obj=t, role="developer", team="ux_ui", workspace=workspace + ) + _stub_renderer( + monkeypatch, frames_tar_gz=_make_frames_tar(["frame-0.png"]), duration=1.0 + ) + + env = await actions.request_render(agent_id=uuid4(), composition_id="release-v1") + + assert env.error is None + draft = markers.get_video_draft(t) + assert draft is not None + assert draft["composition_id"] == "release-v1" + + +@pytest.mark.asyncio +async def test_existing_video_draft_composition_id_not_overwritten( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _arm(monkeypatch) + monkeypatch.setattr(settings, "workspaces_root", str(tmp_path / "workspaces")) + _stub_project(monkeypatch, slug="demo-project") + + clone_root = tmp_path / "clone" + for comp in ("release-v1", "release-v2"): + (clone_root / "motion" / "compositions" / comp).mkdir(parents=True) + workspace = MagicMock() + workspace.get_clone_root_path.return_value = clone_root + workspace.get_worktree_path.return_value = clone_root / ".worktrees" / "deadbeef" + + t = _task(draft={"composition_id": "release-v1", "occasion": "r1"}) + actions, _ = _make_actions( + task_obj=t, role="developer", team="ux_ui", workspace=workspace + ) + _stub_renderer( + monkeypatch, frames_tar_gz=_make_frames_tar(["frame-0.png"]), duration=1.0 + ) + + env = await actions.request_render(agent_id=uuid4(), composition_id="release-v2") + + assert env.error is None + draft = markers.get_video_draft(t) + assert draft is not None + assert draft["composition_id"] == "release-v1" + assert draft["occasion"] == "r1" + + +# --------------------------------------------------------------------------- # +# QA happy path — read-only branch export, never a working tree +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_qa_happy_path_uses_branch_export( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _arm(monkeypatch) + monkeypatch.setattr(settings, "workspaces_root", str(tmp_path / "workspaces")) + _stub_project(monkeypatch, slug="demo-project") + + scratch = tmp_path / "scratch" + (scratch / "motion" / "compositions" / "release-v1").mkdir(parents=True) + read_clone = tmp_path / "readclone" + read_clone.mkdir() + + workspace = MagicMock() + workspace.export_branch_motion = AsyncMock(return_value=scratch) + workspace.ensure_read_clone = AsyncMock(return_value=read_clone) + + t = _task( + draft={"composition_id": "release-v1"}, + branch_name="feature/ux_ui/ABCD1234", + ) + actions, task_svc = _make_actions( + task_obj=t, role="qa", team=None, workspace=workspace + ) + tar_bytes = _make_frames_tar(["frame-0.png"]) + _stub_renderer(monkeypatch, frames_tar_gz=tar_bytes, duration=1.0) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error is None + assert env.evidence is not None + assert env.evidence["source"] == "branch" + assert env.evidence["dirty"] is False + assert len(env.evidence["frames"]) == 1 + workspace.export_branch_motion.assert_awaited_once() + called_project, called_branch = workspace.export_branch_motion.call_args.args + assert called_project.slug == "demo-project" + assert called_branch == "feature/ux_ui/ABCD1234" + payload = markers.get_render_preview(t) + assert payload is not None + assert payload["source"] == "branch" + task_svc.heartbeat.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_qa_without_branch_name_rejected( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _arm(monkeypatch) + _stub_project(monkeypatch) + t = _task(draft={"composition_id": "release-v1"}, branch_name=None) + actions, _task_svc = _make_actions(task_obj=t, role="qa", team=None) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + + +# --------------------------------------------------------------------------- # +# renderer failure +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_video_renderer_error_is_retryable_invalid_state( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + _arm(monkeypatch) + monkeypatch.setattr(settings, "workspaces_root", str(tmp_path / "workspaces")) + _stub_project(monkeypatch, slug="demo-project") + + clone_root = tmp_path / "clone" + (clone_root / "motion" / "compositions" / "release-v1").mkdir(parents=True) + workspace = MagicMock() + workspace.get_clone_root_path.return_value = clone_root + workspace.get_worktree_path.return_value = clone_root / ".worktrees" / "x" + + t = _task(draft={"composition_id": "release-v1"}) + actions, _task_svc = _make_actions( + task_obj=t, role="developer", team="ux_ui", workspace=workspace + ) + _stub_renderer(monkeypatch, error=VideoRendererError("sidecar unreachable")) + + env = await actions.request_render(agent_id=uuid4()) + + assert env.error == "invalid_state" + assert "retry" in (env.remediate or "").lower() + assert markers.get_render_preview(t) is None diff --git a/tests/unit/gateway/test_tracing_gap_hints.py b/tests/unit/gateway/test_tracing_gap_hints.py index a0dee321..81fab109 100644 --- a/tests/unit/gateway/test_tracing_gap_hints.py +++ b/tests/unit/gateway/test_tracing_gap_hints.py @@ -64,6 +64,7 @@ def _build_choreographer() -> Choreographer: "commits>=1", "pr_open", "self_verified", + "render_preview", ], ) def test_hint_registered_for_previously_unhinted_token(token: str) -> None: @@ -93,6 +94,13 @@ def test_during_work_hint_warns_reflect_doesnt_count() -> None: ), f"during_work hint must warn that scope='reflect' doesn't satisfy this: {hint!r}" +def test_render_preview_hint_mentions_request_render() -> None: + """The render_preview hint must name the exact next call (request_render).""" + hint = Choreographer._hint_for_missing_key("render_preview", uuid4()) + assert hint is not None + assert "request_render" in hint + + # --------------------------------------------------------------------------- # Multi-hint remediate — numbered list, not semicolon-joined # --------------------------------------------------------------------------- diff --git a/tests/unit/gateway/test_video_context_in_qa_evidence.py b/tests/unit/gateway/test_video_context_in_qa_evidence.py new file mode 100644 index 00000000..8580d372 --- /dev/null +++ b/tests/unit/gateway/test_video_context_in_qa_evidence.py @@ -0,0 +1,67 @@ +"""QA claim_review evidence carries a video-artifact context for +video-source tasks — pointing QA at the rendered artifact, not just source.""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from roboco.foundation.policy.content import markers +from roboco.services.gateway.choreographer import Choreographer +from roboco.services.gateway.evidence_builder import build_evidence_for_task + + +def _stub_task() -> MagicMock: + task = MagicMock() + task.pr_number = None + task.pr_url = None + task.commits = [] + task.dev_notes = None + task.acceptance_criteria_status = [] + task.source = "code" + task.orchestration_markers = None + return task + + +def test_video_context_none_for_non_video_task() -> None: + assert Choreographer._qa_video_context(_stub_task()) is None + + +def test_video_context_present_for_video_task_with_render_preview() -> None: + task = _stub_task() + task.source = markers.VIDEO_TASK_SOURCE + task.orchestration_markers = { + markers.VIDEO_DRAFT: {"composition_id": "intro-v1"}, + markers.RENDER_PREVIEW: {"frames": ["a.png", "b.png"]}, + } + ctx = Choreographer._qa_video_context(task) + assert ctx is not None + assert ctx["composition_id"] == "intro-v1" + assert ctx["render_preview"] == {"frames": ["a.png", "b.png"]} + assert "request_render" in ctx["note"] + + +def test_video_context_render_preview_none_without_marker() -> None: + task = _stub_task() + task.source = markers.VIDEO_TASK_SOURCE + task.orchestration_markers = {markers.VIDEO_DRAFT: {"composition_id": "intro-v1"}} + ctx = Choreographer._qa_video_context(task) + assert ctx is not None + assert ctx["composition_id"] == "intro-v1" + assert ctx["render_preview"] is None + + +def test_evidence_payload_includes_video_context() -> None: + video_context = {"composition_id": "x", "render_preview": None, "note": "n"} + ev = build_evidence_for_task( + _stub_task(), + journal_highlights=[], + files_changed=[], + video_context=video_context, + ) + assert ev.as_dict()["video_context"] == video_context + + +def test_evidence_payload_video_context_default_absent() -> None: + ev = build_evidence_for_task(_stub_task(), journal_highlights=[], files_changed=[]) + assert ev.video_context is None + assert "video_context" not in ev.as_dict() diff --git a/tests/unit/runtime/test_video_task_prompt.py b/tests/unit/runtime/test_video_task_prompt.py new file mode 100644 index 00000000..8b2dee5b --- /dev/null +++ b/tests/unit/runtime/test_video_task_prompt.py @@ -0,0 +1,55 @@ +"""Video-authoring dev prompt block: the request_render/propose_video +verification instructions appear only for ``source=VIDEO_SOURCE`` tasks, +never for anything else — mirrors test_possibilities_matrix_prompt.py's +bare-instance ``_build_dev_prompt`` idiom. +""" + +from __future__ import annotations + +from typing import Any +from uuid import uuid4 + +import pytest +from roboco.runtime.orchestrator import AgentOrchestrator +from roboco.services.task import VIDEO_SOURCE + + +def _orch() -> AgentOrchestrator: + orch = object.__new__(AgentOrchestrator) + orch._instances = {} + return orch + + +def _task(**over: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "id": str(uuid4()), + "title": "Video: release v1.0.0", + "status": "claimed", + "plan": None, + } + base.update(over) + return base + + +@pytest.mark.asyncio +async def test_video_source_prompt_includes_request_render_instruction() -> None: + prompt = await _orch()._build_dev_prompt(_task(source=VIDEO_SOURCE)) + assert "request_render" in prompt + assert "propose_video" in prompt + assert "render preview" in prompt + + +@pytest.mark.asyncio +async def test_non_video_source_prompt_omits_request_render_instruction() -> None: + prompt = await _orch()._build_dev_prompt(_task(source="code")) + assert "request_render" not in prompt + + +@pytest.mark.asyncio +async def test_missing_source_prompt_omits_request_render_instruction() -> None: + prompt = await _orch()._build_dev_prompt(_task()) + assert "request_render" not in prompt + + +if __name__ == "__main__": + pytest.main([__file__, "-q"]) diff --git a/tests/unit/services/test_video_engine.py b/tests/unit/services/test_video_engine.py index ad6b7b56..4ec6a6f0 100644 --- a/tests/unit/services/test_video_engine.py +++ b/tests/unit/services/test_video_engine.py @@ -35,6 +35,7 @@ SLUG = "roboco" ONE = 1 TWO = 2 THREE = 3 +FOUR = 4 async def _seed(session: AsyncSession) -> None: @@ -138,9 +139,11 @@ async def test_open_video_task_creates_assigned_authoring_task( assert task.estimated_complexity == Complexity.LOW assert task.acceptance_criteria # non-empty # Third AC line: composition follows the design bar / demo-kit register. - assert len(task.acceptance_criteria) == THREE + assert len(task.acceptance_criteria) == FOUR assert "motion/README.md" in task.acceptance_criteria[2] assert "panel-demo" in task.acceptance_criteria[2] + # Fourth AC line: request_render preview frames must be verified. + assert "request_render" in task.acceptance_criteria[3] project = await db_session.get(ProjectTable, task.project_id) assert project is not None assert project.slug == SLUG @@ -154,6 +157,7 @@ async def test_open_video_task_creates_assigned_authoring_task( assert "motion/README.md" in draft["brief"] assert "motion/kit/README.md" in draft["brief"] assert "compositions/panel-demo/" in draft["brief"] + assert "request_render" in draft["brief"] # verify-the-render pointer assert "Brand voice" not in draft["brief"] # unset -> omitted assert draft["suggested_input_props"] == {} # none supplied assert task.description == draft["brief"] @@ -715,9 +719,10 @@ async def test_open_video_task_acceptance_criteria_has_design_bar_line( occasion="release v9.9.9", script="s", platforms=["x"], brief="b" ) assert task is not None - assert len(task.acceptance_criteria) == THREE + assert len(task.acceptance_criteria) == FOUR assert "motion/README.md" in task.acceptance_criteria[2] assert "panel-demo" in task.acceptance_criteria[2] + assert "request_render" in task.acceptance_criteria[3] @pytest.mark.asyncio @@ -818,3 +823,76 @@ async def test_rerender_none_for_missing_task(db_session: AsyncSession) -> None: assert result is None open_tasks = await get_task_service(db_session).list_open_video_posts() assert open_tasks == [] + + +# --------------------------------------------------------------------------- # +# reauthor_from_rejection — CEO reject feedback loop +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_reauthor_from_rejection_opens_revision_with_reason_and_pointer( + db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> None: + await _seed(db_session) + _enable(monkeypatch) + engine = video_engine_module.VideoEngine(db_session) + source_task = await engine.open_video_task( + occasion="release v1.0.0", + script="Here's what shipped", + platforms=["x", "tiktok"], + brief="Announce the release", + ) + assert source_task is not None + draft = markers.get_video_draft(source_task) or {} + markers.set_video_draft(source_task, {**draft, "composition_id": "ReleaseIntro"}) + # Mirrors production: the render loop only fires once the authoring task + # has gone through the full delivery lifecycle to COMPLETED — an open + # source task with the same occasion would otherwise dedupe the revision + # reauthor is about to open right back against itself. + source_task.status = TS.COMPLETED + await db_session.flush() + + post_task = await engine._originate_video_post( + source_task=source_task, + mp4_paths={"vertical": "a.mp4", "square": "b.mp4"}, + captions={"x": "cap", "tiktok": "cap2"}, + platforms=["x", "tiktok"], + ) + # Mirrors VideoPostService.reject: the draft is already CANCELLED by the + # time reauthor_from_rejection runs, so open_video_task's own-occasion + # dedup (which only scans OPEN drafts) doesn't block against it. + post_task.status = TS.CANCELLED + await db_session.flush() + + revision = await engine.reauthor_from_rejection( + post_task, "Logo is cut off in the second scene" + ) + assert revision is not None + assert revision.id != post_task.id + assert revision.source == VIDEO_SOURCE + revision_draft = markers.get_video_draft(revision) + assert revision_draft is not None + assert revision_draft["occasion"] == "release v1.0.0" # SAME occasion + assert revision_draft["platforms"] == ["x", "tiktok"] + assert "Logo is cut off in the second scene" in revision_draft["brief"] + assert "motion/compositions/ReleaseIntro/" in revision_draft["brief"] + assert "do not start a new composition" in revision_draft["brief"] + + +@pytest.mark.asyncio +async def test_reauthor_from_rejection_missing_draft_returns_none( + db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> None: + await _seed(db_session) + _enable(monkeypatch) + engine = video_engine_module.VideoEngine(db_session) + task = await engine.open_video_task( + occasion="no-draft", script="s", platforms=["x"], brief="b" + ) + assert task is not None + task.orchestration_markers = {} # strip the video_draft marker + await db_session.flush() + + result = await engine.reauthor_from_rejection(task, "some reason") + assert result is None diff --git a/tests/unit/services/test_video_post_service.py b/tests/unit/services/test_video_post_service.py index 361fce1e..fbf35cd8 100644 --- a/tests/unit/services/test_video_post_service.py +++ b/tests/unit/services/test_video_post_service.py @@ -509,6 +509,80 @@ async def test_reject_records_reason_and_cancels(db_session: AsyncSession) -> No assert markers.get_video_reject_reason(updated) == "Doesn't match the release" +@pytest.mark.asyncio +async def test_reject_with_reason_calls_reauthor_with_cancelled_task( + db_session: AsyncSession, +) -> None: + """A non-blank reject reason routes into VideoEngine.reauthor_from_rejection, + called with the just-cancelled task and the verbatim reason.""" + task = await _seed_video_post(db_session) + fake_engine = MagicMock() + fake_engine.reauthor_from_rejection = AsyncMock(return_value=None) + with ( + _LOCKED[0], + _LOCKED[1], + patch( + "roboco.services.video_engine.get_video_engine", + return_value=fake_engine, + ), + ): + updated = await _svc( + db_session, x_poster=_StubXPoster(), tiktok_poster=_StubTikTokPoster() + ).reject(_id(task), "Doesn't match the release") + assert updated is not None + assert updated.status == TS.CANCELLED + fake_engine.reauthor_from_rejection.assert_awaited_once() + called_task, called_reason = fake_engine.reauthor_from_rejection.await_args.args + assert called_task.id == task.id + assert called_reason == "Doesn't match the release" + + +@pytest.mark.asyncio +async def test_reject_succeeds_even_when_reauthor_raises( + db_session: AsyncSession, +) -> None: + """A reauthor failure must never fail or roll back the reject — the + cancel already committed before this best-effort seam runs.""" + task = await _seed_video_post(db_session) + fake_engine = MagicMock() + fake_engine.reauthor_from_rejection = AsyncMock(side_effect=RuntimeError("boom")) + with ( + _LOCKED[0], + _LOCKED[1], + patch( + "roboco.services.video_engine.get_video_engine", + return_value=fake_engine, + ), + ): + updated = await _svc( + db_session, x_poster=_StubXPoster(), tiktok_poster=_StubTikTokPoster() + ).reject(_id(task), "Doesn't match the release") + assert updated is not None + assert updated.status == TS.CANCELLED + assert markers.get_video_reject_reason(updated) == "Doesn't match the release" + + +@pytest.mark.asyncio +async def test_reject_blank_reason_skips_reauthor(db_session: AsyncSession) -> None: + task = await _seed_video_post(db_session) + fake_engine = MagicMock() + fake_engine.reauthor_from_rejection = AsyncMock() + with ( + _LOCKED[0], + _LOCKED[1], + patch( + "roboco.services.video_engine.get_video_engine", + return_value=fake_engine, + ), + ): + updated = await _svc( + db_session, x_poster=_StubXPoster(), tiktok_poster=_StubTikTokPoster() + ).reject(_id(task), " ") + assert updated is not None + assert updated.status == TS.CANCELLED + fake_engine.reauthor_from_rejection.assert_not_awaited() + + @pytest.mark.asyncio async def test_reject_takes_the_same_lock_approve_holds( db_session: AsyncSession, diff --git a/tests/unit/services/test_video_renderer_client.py b/tests/unit/services/test_video_renderer_client.py index 04777d1e..bab6cac9 100644 --- a/tests/unit/services/test_video_renderer_client.py +++ b/tests/unit/services/test_video_renderer_client.py @@ -152,6 +152,130 @@ async def test_null_renderer_raises_without_network_call(tmp_path: Path) -> None ) +@pytest.mark.asyncio +async def test_render_frames_posts_frames_field_and_parses_duration( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source = _make_source(tmp_path) + monkeypatch.setattr(cfg, "video_request_timeout_seconds", 5.0) + monkeypatch.setattr(cfg, "video_render_timeout_seconds", 30.0) + captured: dict[str, bytes] = {} + + expected_duration = 12.5 + + def handler(request: httpx.Request) -> httpx.Response: + captured["body"] = request.content + return httpx.Response( + 200, + content=b"fake-frames-tar-gz", + headers={"X-Video-Duration": str(expected_duration)}, + ) + + transport = httpx.MockTransport(handler) + http_client = httpx.AsyncClient(transport=transport) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) + + tar_bytes, duration = await renderer.render_frames( + str(source), + composition_id="Intro", + input_props={"title": "hello"}, + orientation="vertical", + frame_count=8, + ) + await http_client.aclose() + + assert tar_bytes == b"fake-frames-tar-gz" + assert duration == expected_duration + body = captured["body"] + assert isinstance(body, bytes) + assert b'name="frames"' in body + assert b"8" in body + + +@pytest.mark.asyncio +async def test_render_frames_missing_duration_header_returns_zero( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source = _make_source(tmp_path) + monkeypatch.setattr(cfg, "video_request_timeout_seconds", 5.0) + monkeypatch.setattr(cfg, "video_render_timeout_seconds", 30.0) + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content=b"fake-frames-tar-gz") + + transport = httpx.MockTransport(handler) + http_client = httpx.AsyncClient(transport=transport) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) + + tar_bytes, duration = await renderer.render_frames( + str(source), + composition_id="Intro", + input_props={}, + orientation="square", + frame_count=4, + ) + await http_client.aclose() + + assert tar_bytes == b"fake-frames-tar-gz" + assert duration == 0.0 + + +@pytest.mark.asyncio +async def test_render_frames_non_success_response_raises_clear_error( + tmp_path: Path, +) -> None: + source = _make_source(tmp_path) + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(400, text="frames out of bounds") + + transport = httpx.MockTransport(handler) + http_client = httpx.AsyncClient(transport=transport) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) + + with pytest.raises(VideoRendererError, match="400"): + await renderer.render_frames( + str(source), + composition_id="Intro", + input_props={}, + orientation="square", + frame_count=4, + ) + await http_client.aclose() + + +@pytest.mark.asyncio +async def test_render_frames_unconfigured_renderer_raises_without_network_call( + tmp_path: Path, +) -> None: + source = _make_source(tmp_path) + renderer = VideoRenderer(base_url="") + with pytest.raises(VideoRendererError, match="not configured"): + await renderer.render_frames( + str(source), + composition_id="Intro", + input_props={}, + orientation="vertical", + frame_count=4, + ) + + +@pytest.mark.asyncio +async def test_render_frames_null_renderer_raises_without_network_call( + tmp_path: Path, +) -> None: + source = _make_source(tmp_path) + renderer = NullVideoRenderer() + with pytest.raises(VideoRendererError, match="not configured"): + await renderer.render_frames( + str(source), + composition_id="Intro", + input_props={}, + orientation="vertical", + frame_count=4, + ) + + def test_get_video_renderer_returns_null_when_unset( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/video-renderer/render.js b/video-renderer/render.js index e71c27f2..39914333 100644 --- a/video-renderer/render.js +++ b/video-renderer/render.js @@ -8,13 +8,17 @@ // render call produces its own temp file the caller cleans up once it has // streamed the response. import { createRenderJob, executeRenderJob } from "@hyperframes/producer"; +import { execFile } from "node:child_process"; import { existsSync } from "node:fs"; import { cp, mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { Readable } from "node:stream"; +import { promisify } from "node:util"; import * as tar from "tar"; +const execFileP = promisify(execFile); + // @hyperframes/producer reads each cut's dimensions from the composition HTML // itself (data-width/data-height on the stage), so the sidecar no longer // passes width/height — it only picks the quality tier. @@ -247,4 +251,85 @@ export async function renderComposition({ } throw err; } +} + +export const MAX_PREVIEW_FRAMES = 32; + +// Downscaled so agents reading the frames as images get a small file that +// still keeps on-screen copy legible (720 wide ≈ half of a 1080 cut). +const PREVIEW_FRAME_WIDTH = 720; + +/** + * Render one cut and extract `frameCount` evenly spaced keyframes from it — + * the preview surface behind the fleet's `request_render` verb, so an agent + * can verify the actual artifact instead of the composition source. Samples + * scene MIDPOINTS (duration * (i + 0.5) / N), never t=0 or t=duration, so a + * fade-in first frame or an EOF seek can't produce a blank/missing frame. + * Returns the frames tarball path, the probed real duration, and a cleanup + * callback the caller MUST invoke after streaming. + */ +export async function renderFrames({ + tarBuffer, + compositionId, + inputProps, + orientation, + frameCount, +}) { + const { outputLocation, cleanup: cleanupRender } = await renderComposition({ + tarBuffer, + compositionId, + inputProps, + orientation, + }); + let framesDir; + try { + // ffprobe the RENDERED file rather than trusting the composition's + // data-duration attribute — the whole point is ground truth. + const { stdout } = await execFileP("ffprobe", [ + "-v", "error", + "-show_entries", "format=duration", + "-of", "csv=p=0", + outputLocation, + ]); + const duration = Number(stdout.trim()); + if (!Number.isFinite(duration) || duration <= 0) { + throw new Error("could not probe rendered video duration"); + } + + framesDir = await mkdtemp(path.join(tmpdir(), "hyperframes-frames-")); + const files = []; + for (let i = 0; i < frameCount; i++) { + const t = (duration * (i + 0.5)) / frameCount; + // Timestamp in the filename — self-describing, no manifest needed. + const name = `frame-${String(i + 1).padStart(2, "0")}-of-${frameCount}-at-${t.toFixed(1)}s.png`; + await execFileP("ffmpeg", [ + "-v", "error", + "-ss", t.toFixed(3), + "-i", outputLocation, + "-frames:v", "1", + "-vf", `scale=${PREVIEW_FRAME_WIDTH}:-2`, + "-y", + path.join(framesDir, name), + ]); + files.push(name); + } + + const tarPath = path.join(framesDir, "frames.tar.gz"); + await tar.create({ gzip: true, cwd: framesDir, file: tarPath }, files); + return { + tarPath, + duration, + cleanup: () => + Promise.all([ + cleanupRender(), + rm(framesDir, { recursive: true, force: true }).catch(() => {}), + ]), + }; + } catch (err) { + await cleanupRender(); + if (framesDir) { + await rm(framesDir, { recursive: true, force: true }).catch(() => {}); + } + throw err; + } } \ No newline at end of file diff --git a/video-renderer/server.js b/video-renderer/server.js index 4cde0244..71bcbac1 100644 --- a/video-renderer/server.js +++ b/video-renderer/server.js @@ -10,9 +10,12 @@ import express from "express"; import multer from "multer"; import rateLimit from "express-rate-limit"; import { createReadStream } from "node:fs"; +import { fileURLToPath } from "node:url"; import { renderComposition, + renderFrames, ExtractedSizeExceededError, + MAX_PREVIEW_FRAMES, RenderTimeoutError, UnknownCompositionError, } from "./render.js"; @@ -58,6 +61,61 @@ app.get("/health", (_req, res) => { res.status(200).json({ status: "ok" }); }); +/** + * Validate the optional 'frames' form field. Absent/empty keeps the + * existing MP4 behavior (`count: null`); present must be an integer in + * 1..MAX_PREVIEW_FRAMES or `error` names the bound for the 400 response. + * Pure + exported so the branch is unit-testable without a live server. + */ +export function parseFramesField(raw) { + if (raw === undefined || raw === null || raw === "") return { count: null }; + const count = Number(raw); + if (!Number.isInteger(count) || count < 1 || count > MAX_PREVIEW_FRAMES) { + return { + error: `'frames' must be an integer between 1 and ${MAX_PREVIEW_FRAMES}`, + }; + } + return { count }; +} + +/** + * Stream `filePath` as the response body and run `cleanup` once the + * response is done — shared by both the MP4 and the frames-tar branches of + * /render. See the inline comment below on why cleanup fires from both the + * source stream's own "close" and the response's "close" (client abort). + */ +function streamFileWithCleanup(res, filePath, { contentType, headers, cleanup }) { + res.status(200); + res.setHeader("Content-Type", contentType); + for (const [name, value] of Object.entries(headers ?? {})) { + res.setHeader(name, value); + } + const stream = createReadStream(filePath); + stream.on("error", (err) => { + console.error("video-renderer: stream error", err); + if (!res.headersSent) { + res.status(500); + } + res.end(); + cleanup(); + }); + stream.on("close", () => { + cleanup(); + }); + // stream.pipe() never propagates a DESTINATION close back to the + // source: if the client aborts, or the orchestrator's retry-on-timeout + // hangs up mid-download, `res` closes but the source stream's own + // "close" above never fires — leaking this request's render-output + // temp dir on every such disconnect. Destroying the still-open source + // releases its fd immediately; cleanup() is idempotent (rm force:true) + // so also landing here on a normal end-of-stream close is harmless. + res.on("close", () => { + stream.destroy(); + cleanup(); + }); + stream.pipe(res); +} + app.post("/render", renderLimiter, upload.single("source"), async (req, res) => { const body = req.body ?? {}; const compositionId = body.composition_id; @@ -96,40 +154,39 @@ app.post("/render", renderLimiter, upload.single("source"), async (req, res) => return; } + const framesField = parseFramesField(body.frames); + if (framesField.error) { + res.status(400).json({ error: framesField.error }); + return; + } + try { + if (framesField.count !== null) { + const { tarPath, duration, cleanup } = await renderFrames({ + tarBuffer: req.file.buffer, + compositionId, + inputProps, + orientation, + frameCount: framesField.count, + }); + streamFileWithCleanup(res, tarPath, { + contentType: "application/gzip", + headers: { "X-Video-Duration": String(duration) }, + cleanup, + }); + return; + } + const { outputLocation, cleanup } = await renderComposition({ tarBuffer: req.file.buffer, compositionId, inputProps, orientation, }); - - res.status(200); - res.setHeader("Content-Type", "video/mp4"); - const stream = createReadStream(outputLocation); - stream.on("error", (err) => { - console.error("video-renderer: stream error", err); - if (!res.headersSent) { - res.status(500); - } - res.end(); - cleanup(); + streamFileWithCleanup(res, outputLocation, { + contentType: "video/mp4", + cleanup, }); - stream.on("close", () => { - cleanup(); - }); - // stream.pipe() never propagates a DESTINATION close back to the - // source: if the client aborts, or the orchestrator's retry-on-timeout - // hangs up mid-download, `res` closes but the source stream's own - // "close" above never fires — leaking this request's render-output - // temp dir on every such disconnect. Destroying the still-open source - // releases its fd immediately; cleanup() is idempotent (rm force:true) - // so also landing here on a normal end-of-stream close is harmless. - res.on("close", () => { - stream.destroy(); - cleanup(); - }); - stream.pipe(res); } catch (err) { if ( err instanceof UnknownCompositionError || @@ -171,6 +228,10 @@ app.use((err, req, res, _next) => { res.status(500).json({ error: "internal error" }); }); -app.listen(PORT, () => { - console.log(`video-renderer listening on :${PORT}`); -}); +// Guarded so a test can `import` this module (for parseFramesField) without +// also binding the real port — only `node server.js` triggers the listen. +if (process.argv[1] === fileURLToPath(import.meta.url)) { + app.listen(PORT, () => { + console.log(`video-renderer listening on :${PORT}`); + }); +} diff --git a/video-renderer/server.test.js b/video-renderer/server.test.js new file mode 100644 index 00000000..1534703a --- /dev/null +++ b/video-renderer/server.test.js @@ -0,0 +1,42 @@ +// node --test: exercises the pure 'frames' form-field validation only — +// server.js's app.listen is guarded (see server.js) so importing it here +// never binds the real port. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { parseFramesField } from "./server.js"; +import { MAX_PREVIEW_FRAMES } from "./render.js"; + +test("parseFramesField treats an absent field as the existing MP4 path", () => { + assert.deepEqual(parseFramesField(undefined), { count: null }); +}); + +test("parseFramesField treats an empty string as the existing MP4 path", () => { + assert.deepEqual(parseFramesField(""), { count: null }); +}); + +test("parseFramesField accepts an in-bounds integer string", () => { + assert.deepEqual(parseFramesField("8"), { count: 8 }); +}); + +test("parseFramesField accepts the boundary values 1 and MAX_PREVIEW_FRAMES", () => { + assert.deepEqual(parseFramesField("1"), { count: 1 }); + assert.deepEqual(parseFramesField(String(MAX_PREVIEW_FRAMES)), { + count: MAX_PREVIEW_FRAMES, + }); +}); + +test("parseFramesField rejects zero", () => { + assert.ok(parseFramesField("0").error); +}); + +test("parseFramesField rejects above MAX_PREVIEW_FRAMES", () => { + assert.ok(parseFramesField(String(MAX_PREVIEW_FRAMES + 1)).error); +}); + +test("parseFramesField rejects a non-integer value", () => { + assert.ok(parseFramesField("4.5").error); +}); + +test("parseFramesField rejects a non-numeric value", () => { + assert.ok(parseFramesField("abc").error); +});