fix(media-proof): time out hung ffmpeg and ffprobe

[SebTardif]

What Problem This Solves

Fixes an issue where review workers would hang forever when a pull request linked a corrupt or huge video as proof. curl already uses --max-time 90, but ffprobe and ffmpeg ran through spawnSync with no process deadline. One stalled decoder blocked the worker until an outer job timeout (or a human) killed it.

Why This Change Was Made

The media-proof helper now passes timeout: 120000 to spawnSync. That bound covers curl, ffprobe, and ffmpeg. When the child exceeds it, Node sets error.code to ETIMEDOUT and a non-zero/null status. The existing failed-artifact path records that detail and the review continues. Git fetch and webhook spawnSync callers are unchanged.

User Impact

A bad video proof no longer stalls a review worker. The worker records a failed media-proof artifact (ffprobe failed: spawnSync ... ETIMEDOUT) and moves on.

OpenClaw Bay Impact

Unaffected. This change is review-worker media preparation only. It does not alter Bay lifecycle, queue, status, telemetry, or dashboard data contracts.

Documentation Lifecycle

No documentation lifecycle changes.

Evidence

Before this change, the default media-proof spawnSync options had no timeout field. After the change, a live node import of the compiled helper prints timeout: 120000, and sleep 30 with a 50ms runner timeout returns ETIMEDOUT instead of running to completion.

$ node --input-type=module
default spawn options:
{
  "encoding": "utf8",
  "maxBuffer": 16777216,
  "timeout": 120000
}

hung sleep 30 with timeout 50ms:
{
  "status": null,
  "signal": "SIGTERM",
  "error": {
    "name": "Error",
    "message": "spawnSync sleep ETIMEDOUT",
    "code": "ETIMEDOUT"
  },
  "elapsedMs": 52
}

prepared artifact after hung ffprobe:
{
  "status": "failed",
  "kind": "video",
  "detail": "ffprobe failed: spawnSync sleep ETIMEDOUT",
  "downloadedPath": "set",
  "metadataPath": null,
  "contactSheetPath": null
}

Related: video inspection landed in #144 (2026-05-20). The unbounded spawnSync helper was extracted in #998 (2026-08-01). Git helpers already have the same class of bound in #508.

Real behavior proof

• Behavior or issue addressed: Review-worker media proof could hang forever on ffprobe/ffmpeg because the shared spawnSync helper had no timeout. A corrupt or huge video blocked the worker.

• Real environment tested: macOS, Node v26.7.0, worktree /tmp/oc-impl-clawsweeper-ffmpeg at the patched commit.

• Exact steps or command run after this patch:

  node --input-type=module

  The script imported createMediaProofCommandRunnerForTest and prepareMediaProofArtifactsForTest from ./dist/clawsweeper.js, printed default spawn options, ran sleep 30 with a 50ms timeout, then prepared a .mov URL whose ffprobe step reused that hung sleep.

• Evidence after fix: terminal output from the live node command above. Default options include timeout: 120000. The hung sleep 30 returns status: null, signal: SIGTERM, error.code: ETIMEDOUT. prepareMediaProofArtifacts records status: "failed" with detail: "ffprobe failed: spawnSync sleep ETIMEDOUT".

• Observed result after fix: The helper no longer waits for a 30 second sleep. Node kills the child and the media-proof artifact is failed, which is the same path a hung ffprobe would take on the review worker.

• What was not tested: A real corrupt video file through host ffmpeg/ffprobe on a live review-worker VM. Git fetch and webhook spawnSync callers were left unchanged on purpose.

Keep Allow edits from maintainers enabled.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 29, 2026, 9:59 PM ET / August 30, 2026, 01:59 UTC.

ClawSweeper review

What this changes

Adds a two-minute default deadline to media-proof subprocesses and tests that a timed-out video probe becomes a failed artifact.

Merge readiness

⛔ Blocked until stronger real behavior proof is added - 5 items remain

Keep open: the per-command timeout bounds a single hung decoder, but the full sequential media pipeline can still exceed the review worker’s preprocessing allowance; the current proof also does not meet the repository’s required Crabbox provenance.

Priority: P2
Reviewed head: b40dc57663b3b4fb1f06e5fc6afa752a635f104b

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The PR has a focused, relevant implementation and terminal evidence, but the remaining pipeline-budget defect and incomplete required proof block readiness.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: The PR body contains a current-head macOS terminal trace using the production media runner with a real timed-out sleep and a simulated ffprobe call into media preparation; it demonstrates ETIMEDOUT conversion but not the complete curl-to-ffprobe-to-ffmpeg pipeline or the repository-required Docker-backed Crabbox provider, image, lease, artifact, and limits record. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The PR body contains a current-head macOS terminal trace using the production media runner with a real timed-out sleep and a simulated ffprobe call into media preparation; it demonstrates ETIMEDOUT conversion but not the complete curl-to-ffprobe-to-ffmpeg pipeline or the repository-required Docker-backed Crabbox provider, image, lease, artifact, and limits record. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	7 items	Introduced timeout: The PR applies a 120-second default timeout to every media-proof subprocess invocation. Sequential pipeline remains unshared: Each selected video is processed sequentially through curl, ffprobe, then ffmpeg; a probe failure skips conversion, but a successful slow probe can still be followed by a separate 120-second conversion budget. Scheduler budget mismatch: Adaptive review budgeting assigns 120 seconds per video, while the media helper can spend up to 90 seconds downloading plus two independent 120-second subprocess windows per video.
Findings	1 actionable finding	[P1] Enforce one deadline across each video pipeline
Security	None	None.

How this fits together

Review workers extract linked image and video evidence before giving it to Codex. Media downloads, probing, and contact-sheet generation feed the review prompt and must complete within the worker’s reserved preprocessing time.

flowchart LR
  A[Review context] --> B[Media URL selection]
  B --> C[Download proof]
  C --> D[Probe video metadata]
  D --> E[Create contact sheet]
  E --> F[Review prompt evidence]
  G[Review worker time budget] --> C
  G --> D
  G --> E

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The PR body contains a current-head macOS terminal trace using the production media runner with a real timed-out sleep and a simulated ffprobe call into media preparation; it demonstrates ETIMEDOUT conversion but not the complete curl-to-ffprobe-to-ffmpeg pipeline or the repository-required Docker-backed Crabbox provider, image, lease, artifact, and limits record. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Enforce one deadline across each video pipeline (P1) - Each video can still spend up to 90 seconds downloading, then up to 120 seconds in ffprobe and another 120 seconds in ffmpeg. With four URLs that can reach 1,320 seconds, exceeding the 480-second preprocessing reservation before failed artifacts are recorded; carry a diminishing shared deadline through the stages or align the reservation and cover that bound.
• Resolve merge risk (P2) - Four video URLs can consume up to 1,320 seconds across download, probe, and contact-sheet stages, while the exact worker reserves 480 seconds for preprocessing; the outer timeout can terminate the worker before it records all failures and continues.
• Resolve merge risk (P2) - The supplied macOS terminal trace exercises timeout conversion but lacks the current-head Docker-backed Crabbox provenance required by repository policy and does not cover the full multi-stage media pipeline.
• Complete next step (P2) - A narrow mechanical repair can make the media pipeline honor its already-declared preprocessing budget; contributor proof remains required after that code repair.

Findings

• [P1] Enforce one deadline across each video pipeline — src/clawsweeper-media-proof.ts:42-45

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +34/-7, tests +44 across 3 files	The focused implementation is well-covered by additions, but the new tests do not establish the full scheduler-aligned pipeline bound.

Merge-risk options

Maintainer options:

1. Bound each complete video pipeline (recommended)
   Use a shared per-video deadline across download, probing, and conversion, then cover the maximum supported URL count before merging.
2. Accept outer-timeout failures
   Maintainers may retain independent command deadlines if ending a review during media preparation is acceptable.

Technical review

Best possible solution:

Use one diminishing deadline per video pipeline that fits the scheduler allowance, record a failed artifact at exhaustion, cover the multi-video bound, and publish current-head Crabbox proof.

Do we have a high-confidence way to reproduce the issue?

Yes. The supplied terminal trace demonstrates Node’s timeout behavior, and current source establishes the remaining sequential deadline mismatch without requiring a live review worker.

Is this the best way to solve the issue?

No. The per-command default removes an infinite wait for one child process, but a shared deadline across each video pipeline is needed to honor the existing worker budget.

Full review comments:

• [P1] Enforce one deadline across each video pipeline — src/clawsweeper-media-proof.ts:42-45
  Each video can still spend up to 90 seconds downloading, then up to 120 seconds in ffprobe and another 120 seconds in ffmpeg. With four URLs that can reach 1,320 seconds, exceeding the 480-second preprocessing reservation before failed artifacts are recorded; carry a diminishing shared deadline through the stages or align the reservation and cover that bound.
  Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against fa535d54a032.

Labels

Label justifications:

• P2: This is a bounded review-worker availability repair with limited user-facing blast radius.
• merge-risk: 🚨 availability: The independent stage limits can exceed the worker preprocessing reservation and cause the outer review timeout to end the run.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body contains a current-head macOS terminal trace using the production media runner with a real timed-out sleep and a simulated ffprobe call into media preparation; it demonstrates ETIMEDOUT conversion but not the complete curl-to-ffprobe-to-ffmpeg pipeline or the repository-required Docker-backed Crabbox provider, image, lease, artifact, and limits record. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Acceptance criteria:

• [P1] node --test test/review-prompt-policy.test.ts.
• [P1] pnpm run check.
• [P1] Current-head Docker-backed Crabbox media-pipeline proof with recorded provider, image, lease, artifact, and limits.

What I checked:

• Introduced timeout: The PR applies a 120-second default timeout to every media-proof subprocess invocation. (src/clawsweeper-media-proof.ts:42, b40dc57663b3)
• Sequential pipeline remains unshared: Each selected video is processed sequentially through curl, ffprobe, then ffmpeg; a probe failure skips conversion, but a successful slow probe can still be followed by a separate 120-second conversion budget. (src/clawsweeper-media-proof.ts:170, b40dc57663b3)
• Scheduler budget mismatch: Adaptive review budgeting assigns 120 seconds per video, while the media helper can spend up to 90 seconds downloading plus two independent 120-second subprocess windows per video. (src/repair/adaptive-review-budget.ts:47, fa535d54a032)
• Outer reservation: The exact-review workflow reserves 480 seconds for preprocessing and does not add its detected media allowance to the actual timeout calculation. (.github/workflows/sweep.yml:1263, fa535d54a032)
• Current-main necessity: The PR base’s media command runner passes only an optional caller timeout, so current main has no default deadline for ffprobe or ffmpeg. (src/clawsweeper-media-proof.ts:20, fa535d54a032)
• Repository proof policy: The checked-out policy requires Docker-backed Crabbox proof with current-head provider, image, lease, artifact, and limits details for this runtime change. (AGENTS.md:67, b40dc57663b3)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Takhoffman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Apply one shared deadline to each complete video pipeline and add a maximum-URL regression case.
• Add redacted current-head Docker-backed Crabbox proof with provider, image, lease, artifact, and limits details.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-08-15T21:17:22.088Z sha b63b38b :: needs real behavior proof before merge. :: [P1] Budget the full sequential media-processing window
• reviewed 2026-08-24T02:24:23.818Z sha b63b38b :: needs real behavior proof before merge. :: [P1] Bound the whole media-preparation sequence
• reviewed 2026-08-25T09:17:47.731Z sha 3294904 :: needs real behavior proof before merge. :: [P1] Share the media deadline across each video pipeline

[brokemac79]

@SebTardif, thank you—your contribution is welcome. If you would like to continue with this PR, please bring it up to date and ready for maintainer look. If you would prefer Martin to take it over as a maintainer, please say so. This note does not indicate approval or a merge decision.

[SebTardif]

@brokemac79

If you would like to continue with this PR, please bring it up to date and ready for maintainer look. If you would prefer Martin to take it over as a maintainer, please say so.

We want to continue. The branch is now replayed onto current main (cherry-pick of the media-proof timeout commit, not a rebase of the old history). The shared spawnSync helper still defaults to timeout: 120000 for curl, ffprobe, and ffmpeg. A hung probe is recorded as a failed artifact and the review worker continues.

Please take another look when you have time.