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 15, 2026, 5:17 PM ET / 21:17 UTC.

ClawSweeper review

What this changes

The PR adds a two-minute timeout to the shared media-proof subprocess runner and tests timeout handling for video inspection failures.

Merge readiness

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

This PR addresses a real review-worker availability gap, but its per-command timeout can still exhaust the exact-review job deadline when multiple video proofs are processed sequentially.

Priority: P2
Reviewed head: b63b38b06ed94ef97c265b034e540447703e7898

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The direct timeout trace is useful, but a deadline mismatch leaves a blocking availability defect and the required controlled proof is incomplete.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: The PR body includes a meaningful after-fix terminal trace, but the required current-head Crabbox proof record (provider, lease, image, artifact, and limits) is absent. 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	🦪 silver shellfish (2/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The PR body includes a meaningful after-fix terminal trace, but the required current-head Crabbox proof record (provider, lease, image, artifact, and limits) is absent. 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	5 items	Current unbounded implementation: Current main invokes curl, ffprobe, and ffmpeg through one synchronous runner with no timeout; the latter two calls are reached after download succeeds. PR applies timeout per subprocess: The PR routes the shared runner through options with timeout: 120000, so the bound applies independently to curl, ffprobe, and ffmpeg. Outer review deadline is too short for staged processing: The workflow reserves 480 seconds for media preprocessing while allowing up to four URLs. Four videos can consume up to 90 seconds of curl plus 120 seconds each of ffprobe and ffmpeg, or 1320 seconds before Codex begins.
Findings	1 actionable finding	[P1] Budget the full sequential media-processing window
Security	None	None.

How this fits together

Media-proof preparation downloads linked images and videos, then invokes curl, ffprobe, and ffmpeg before the review model runs. It produces inspectable artifacts or failed-artifact details that feed the review workflow.

flowchart LR
  A[PR proof links] --> B[Media-proof preparation]
  B --> C[Download and inspect media]
  C --> D{Each command finishes?}
  D -->|Yes| E[Prepared artifacts]
  D -->|Timeout or error| F[Failed artifact detail]
  E --> G[Codex review]
  F --> G

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The PR body includes a meaningful after-fix terminal trace, but the required current-head Crabbox proof record (provider, lease, image, artifact, and limits) is absent. 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.
• Budget the full sequential media-processing window (P1) - The new 120-second limit applies separately to curl, ffprobe, and ffmpeg, but exact reviews reserve only 480 seconds for all media processing. With the allowed four videos, up to 1320 seconds of bounded media work can occur before Codex runs, so the outer timeout can kill the review before failed artifacts or a result are published. Update the total deadline/budget together with this per-command limit and cover the multi-video case.
• Resolve merge risk (P1) - For two or more slow video proofs, sequential curl, ffprobe, and ffmpeg stages can consume the exact-review deadline before a failed artifact or review result is published.
• Resolve merge risk (P1) - The supplied terminal proof is useful but does not meet the repository's current Crabbox proof contract for code-bearing changes.
• Complete next step (P2) - A focused repair can align the media-processing deadline with the new per-command limit, but the contributor must still provide current-head Crabbox proof before merge.
• Improve patch quality - Adjust the exact-review media budget or add a shared media deadline and cover multiple video URLs.
• Improve patch quality - Add current-head Crabbox proof showing timeout recovery and failed-artifact continuation, with sensitive data redacted.

Findings

• [P1] Budget the full sequential media-processing window — src/clawsweeper-media-proof.ts:19-25

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	3 files affected; 65 added, 1 removed	The implementation is narrowly focused on media subprocess options, test exports, and regression coverage.
Worst-case media time	4 videos × (90s curl + 120s ffprobe + 120s ffmpeg) = 1320s	This exceeds the default 1260-second exact-review deadline before model execution.

Merge-risk options

Maintainer options:

1. Align the review deadline with staged media limits (recommended)
   Derive or expand the media preprocessing allowance so the maximum accepted set of video proofs can complete their bounded stages and still reach the review result.
2. Use a single total media deadline
   Keep the existing outer allowance by enforcing a shared per-review media deadline rather than granting each subprocess a separate two-minute window.

Technical review

Best possible solution:

Use one bounded media-processing budget that covers every sequential stage and all accepted proof URLs, then demonstrate the current-head timeout and failed-artifact recovery in the required Crabbox environment.

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

Yes, from source: use two or more accepted video URLs whose ffprobe or ffmpeg stage reaches its new 120-second limit; the sequential preprocessing plus Codex allowance exceeds the exact-review deadline.

Is this the best way to solve the issue?

No. A per-process timeout is useful, but the outer review budget must also cover every sequential media stage or impose one shared media deadline.

Full review comments:

• [P1] Budget the full sequential media-processing window — src/clawsweeper-media-proof.ts:19-25
  The new 120-second limit applies separately to curl, ffprobe, and ffmpeg, but exact reviews reserve only 480 seconds for all media processing. With the allowed four videos, up to 1320 seconds of bounded media work can occur before Codex runs, so the outer timeout can kill the review before failed artifacts or a result are published. Update the total deadline/budget together with this per-command limit and cover the multi-video case.
  Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add P2: The change targets a bounded review-worker availability defect, but its known failure case is limited to media-heavy proof processing.
• add merge-risk: 🚨 availability: Merging the current timeout model can still terminate exact reviews before they publish a result when several video proofs consume their sequential limits.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦐 gold shrimp and patch quality is 🦪 silver shellfish.
• add 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 includes a meaningful after-fix terminal trace, but the required current-head Crabbox proof record (provider, lease, image, artifact, and limits) is absent. 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.

Label justifications:

• P2: The change targets a bounded review-worker availability defect, but its known failure case is limited to media-heavy proof processing.
• merge-risk: 🚨 availability: Merging the current timeout model can still terminate exact reviews before they publish a result when several video proofs consume their sequential limits.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦐 gold shrimp and patch quality is 🦪 silver shellfish.
• 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 includes a meaningful after-fix terminal trace, but the required current-head Crabbox proof record (provider, lease, image, artifact, and limits) is absent. 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] pnpm run build.
• [P1] node --test test/review-prompt-policy.test.ts test/repair/adaptive-review-budget.test.ts test/sweep-workflow.test.ts.
• [P1] pnpm run check.
• [P2] Crabbox current-head proof of staged video timeout recovery and continuation.

What I checked:

• Current unbounded implementation: Current main invokes curl, ffprobe, and ffmpeg through one synchronous runner with no timeout; the latter two calls are reached after download succeeds. (src/clawsweeper-media-proof.ts:20, d389e6addf4d)
• PR applies timeout per subprocess: The PR routes the shared runner through options with timeout: 120000, so the bound applies independently to curl, ffprobe, and ffmpeg. (src/clawsweeper-media-proof.ts:19, b63b38b06ed9)
• Outer review deadline is too short for staged processing: The workflow reserves 480 seconds for media preprocessing while allowing up to four URLs. Four videos can consume up to 90 seconds of curl plus 120 seconds each of ffprobe and ffmpeg, or 1320 seconds before Codex begins. (.github/workflows/sweep.yml:1262, d389e6addf4d)
• Media budget provenance: The existing adaptive budget assigns only 120 seconds per video and caps at four; its timeout constant and calculation were introduced by the queue work in commit 7656448. (src/repair/adaptive-review-budget.ts:11, 765644804756)
• Contributor runtime evidence: The PR body records a live terminal run where a 50ms timeout terminates sleep and the failed-artifact path records ETIMEDOUT, but it does not provide the repository-required Crabbox provider, lease, image, or artifact record. (b63b38b06ed9)

Likely related people:

• steipete: Peter Steinberger authored current-main changes to media-proof handling and the media timeout budget. (role: recent area contributor; confidence: high; commits: 96b8703e4c0b, 765644804756; files: src/clawsweeper-media-proof.ts, src/repair/adaptive-review-budget.ts, .github/workflows/sweep.yml)
• Takhoffman: Introduced the merged video-proof inspection surface that this timeout repair modifies. (role: feature introducer; confidence: medium; commits: 504733a2c0b2; files: src/clawsweeper-media-proof.ts)

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.