Skip to content

fix(video): skip delivering tasks in active-task prompt guard#96018

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
palomyates516-alt:fix/95701-video-generate-deadlock
Jun 24, 2026
Merged

fix(video): skip delivering tasks in active-task prompt guard#96018
vincentkoc merged 2 commits into
openclaw:mainfrom
palomyates516-alt:fix/95701-video-generate-deadlock

Conversation

@palomyates516-alt

@palomyates516-alt palomyates516-alt commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #95701.

What Problem This Solves

Sequential video_generate workflows blocked the next segment for ~4-5 min after each clip's video was ready. The active-task prompt guard treated the still-running task as blocking until the completion wake finished (self-deadlock), so crews producing clips 01, 02, 03 back-to-back paid a ~3.5-4 min "release tax" per segment on top of actual generation (~60-90s). The task only flipped running -> succeeded after the woken agent turn gave up and yielded.

Evidence

  • Reporter production data (issue video_generate background task stays "running" for minutes after media ready — self-deadlock between wake (expectFinal) and completeTaskRun #95701): state/openclaw.sqlite task_runs for one session - task c6043e3f ran 12:54:07 -> 12:59:25 (5m18s) with provider byteplus/doubao-seedance, where actual generation+download is ~60-90s. The requester trajectory shows the woken turn (12:54:35 -> 12:59:23, 4m48s) reporting "stale state machine, 5 retries intercepted", and the task flips to succeeded only at 12:59:25 - after the woken turn ended, not when the video was ready.
  • Root cause verified against current main (a182811070): scheduleMediaGenerationTaskCompletion (src/agents/tools/media-generate-background-shared.ts:382) gates completeTaskRun (:487) behind wakeTaskCompletion(expectFinal:true) (:431); the label-less prompt guard (buildActiveMediaGenerationTaskPromptContextForSession at media-generation-task-status-shared.ts:447) suppresses the next segment's video_generate while the task is still running.
  • Unit tests (new + existing, 52 green): listActive excludes delivering tasks when the flag is set; buildActive returns no prompt when the only active task is delivering, still warns when mid-generation; existing video/image generation suites unchanged.

Fix

  • Extract the existing "Generated media; delivering completion" progress marker (set at media-generate-background-shared.ts:418 since the wake gates completeTaskRun) into a shared constant MEDIA_GENERATION_DELIVERING_COMPLETION_PROGRESS.
  • listActiveMediaGenerationTasksForSession / findActiveMediaGenerationTaskForSession gain an optional excludeDeliveringCompletion flag; the prompt guard buildActiveMediaGenerationTaskPromptContextForSession passes true, so a distinct next segment is no longer suppressed once media is ready.
  • Duplicate guard (findDuplicateGuardMediaGenerationTaskForSession, already label-aware) and the completeTaskRun delivery-confirmation gate are unchanged.

Scope / non-goals

  • No change to the task status machine (no new delivering state), TaskRecord schema, duplicate guard, or the completeTaskRun safety contract ("generated result without confirmed delivery is terminally unsafe for closeout").

Test plan

  • New unit tests (media-generation-task-status-shared.test.ts)
  • Existing suites green (52 tests)
  • CI: pnpm tsgo + pnpm check (oxlint/oxfmt)
  • Live regression (maintainer env, needs byteplus/LM Studio key): sequential video_generate of 2 distinct prompts - confirm 2nd isn't suppressed, task A flips to succeeded after the wake.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 23, 2026
@palomyates516-alt
palomyates516-alt force-pushed the fix/95701-video-generate-deadlock branch from faeac9b to e334d55 Compare June 23, 2026 08:48
@palomyates516-alt palomyates516-alt mentioned this pull request Jun 23, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 23, 2026
Closed
@clawsweeper

clawsweeper Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 8:35 AM ET / 12:35 UTC.

Summary
The PR extracts a shared media delivery-completion progress marker, skips that phase in active media prompt guards, passes prompt metadata into video/music duplicate guards, and adds focused media task-status tests.

PR surface: Source +16, Tests +126. Total +142 across 5 files.

Reproducibility: yes. The linked production report plus current source show a generated media task remains running while wakeTaskCompletion awaits the requester turn, and the session prompt/tool guards can still see that active task; I did not run a live provider reproduction.

Review metrics: 1 noteworthy metric.

  • Guard paths covered: 1 prompt guard changed, 2 duplicate guard call sites updated. The reported workflow crosses both prompt-context suppression and early tool duplicate checks, so maintainers should see that both gate types changed.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95701
Summary: This PR is the narrower candidate fix for the canonical sequential video-generation delivery-wake deadlock; a sibling PR targets the same root cause with a broader persisted status design.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted after-fix proof from a sequential video_generate run showing the second distinct prompt is not suppressed and the same prompt remains guarded.

Proof guidance:

  • [P1] Needs real behavior proof before merge: After-fix proof is limited to tests, CI, and maintainer comments; before merge, add redacted live or realistic sequential video_generate logs, terminal output, recording, or linked artifact, then update the PR body so ClawSweeper can re-review automatically or via @clawsweeper re-review.

Risk before merge

  • [P1] After-fix real sequential video proof is still missing; current evidence is source inspection, focused tests, hosted checks, and maintainer validation comments.
  • [P1] The PR intentionally changes how in-flight media tasks participate in session prompt and duplicate guards, so maintainers should explicitly accept the session-state behavior before merge.

Maintainer options:

  1. Require Sequential Video Proof (recommended)
    Before merge, require redacted logs, terminal output, or a linked artifact showing a distinct second video_generate request proceeds during the first task's delivery wake while the same prompt remains guarded.
  2. Accept Unit-Only Proof Deliberately
    Maintainers can waive the real-provider proof gate if credentials remain unavailable and they are comfortable owning the remaining live-session risk.
  3. Choose The Broader Status Design
    If maintainers prefer a persisted delivery phase, pause this PR and resolve the broader sibling candidate instead.

Next step before merge

  • [P1] Human follow-up is needed for real after-fix sequential video proof or an explicit maintainer waiver; there is no narrow code defect left for automated repair.

Security
Cleared: Cleared: the diff only changes TypeScript media task guard code and tests, with no dependency, workflow, permission, secret, download, or package-resolution changes.

Review details

Best possible solution:

Land this narrow progress-marker prompt-guard fix after redacted sequential video_generate proof or an explicit maintainer proof waiver; keep the broader status-machine sibling non-canonical unless maintainers choose that design.

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

Yes. The linked production report plus current source show a generated media task remains running while wakeTaskCompletion awaits the requester turn, and the session prompt/tool guards can still see that active task; I did not run a live provider reproduction.

Is this the best way to solve the issue?

Yes, subject to proof. The narrow progress-marker approach avoids a persisted task-status/schema change while preserving completion-delivery closeout and same-prompt duplicate protection.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded but real sequential video-generation workflow bug with limited blast radius and no evidence of global runtime outage or data loss.
  • merge-risk: 🚨 session-state: The PR changes how in-flight media tasks are interpreted by session prompt and duplicate guards during completion delivery.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: After-fix proof is limited to tests, CI, and maintainer comments; before merge, add redacted live or realistic sequential video_generate logs, terminal output, recording, or linked artifact, then update the PR body so ClawSweeper can re-review automatically or via @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +16, Tests +126. Total +142 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 4 19 3 +16
Tests 1 126 0 +126
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 145 3 +142

What I checked:

Likely related people:

  • vincentkoc: Recent GitHub path history shows shared media task-status refactors, and this PR's two current commits plus maintainer refresh comments are carried by this account. (role: recent area contributor and branch carrier; confidence: high; commits: 5ff724239147, 3881cb34260a, b23022d3af9f; files: src/agents/media-generation-task-status-shared.ts, src/agents/tools/media-generate-background-shared.ts, src/agents/video-generation-task-status.ts)
  • steipete: History shows earlier async media generation unification, media completion routing, and recent media requester-route fixes on the same lifecycle surface. (role: feature-history contributor; confidence: high; commits: fd8afc1dce46, 7f28c8bd07d3, 025db6cf9e80; files: src/agents/tools/media-generate-background-shared.ts, src/agents/tools/video-generate-tool.ts, src/agents/media-generation-task-status-shared.ts)
  • fuller-stack-dev: A prior merged media-completion duplicate-delivery change touched generated-media duplicate guards and completion delivery fallback behavior adjacent to this deadlock. (role: adjacent media completion contributor; confidence: medium; commits: 37a9f58d1b10; files: src/agents/media-generation-task-status-shared.ts, src/agents/tools/media-generate-background-shared.ts, src/agents/tools/video-generate-tool.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 23, 2026
@vincentkoc
vincentkoc force-pushed the fix/95701-video-generate-deadlock branch from e334d55 to 0ce6805 Compare June 24, 2026 01:13
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

maintainer refresh on current main

The narrower existing PR remains the canonical fix for #95701. I refreshed the
maintainer-editable branch onto current main at
4d034639adfdcfd58ac8d793fdea625c88897428; the new head is
0ce6805997d870942da4c370427574221446fe70.

The fix keeps the existing task status schema and completion-delivery ordering:
the prompt guard ignores only the explicit “media ready; delivering completion”
phase, while the duplicate guard still blocks the same prompt and allows a
distinct prompt to proceed.

Validation:

  • focused media/video suites: 6 files, 42/42 passed
  • changed-file oxlint: passed
  • formatting and git diff --check: passed
  • final autoreview: 0 actionable findings
  • no live provider request was run; provider credentials were unavailable

@vincentkoc
vincentkoc force-pushed the fix/95701-video-generate-deadlock branch from 0ce6805 to 1898652 Compare June 24, 2026 01:19
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

rebased again because main advanced during merge preparation.

  • new base: 63874fa0d1194675fa6a96a7178ea187d971afa6
  • new head: 1898652ee1b9382cf1b9cfc0b3c4023426efada2
  • focused media/video proof: 6 files, 42 tests passed

The fix still preserves the existing task status schema and same-prompt
duplicate guard.

@vincentkoc
vincentkoc force-pushed the fix/95701-video-generate-deadlock branch from 1898652 to e1a5228 Compare June 24, 2026 01:27
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

final base refresh: main advanced to 54c0048d6c9 during merge preparation.

The branch is rebased cleanly at e1a5228f9f5ad8d9c4c04cf9619ee9ba297db34a;
focused media/video proof remains 42/42.

@vincentkoc
vincentkoc force-pushed the fix/95701-video-generate-deadlock branch from e1a5228 to 8b7d479 Compare June 24, 2026 01:36
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

rebased onto current main da15cf48bf4813cd64bb7fd64bbd291a8bad9a13; new
head 8b7d4791d9ea55fe57d8e7376bf444dd16690bd3. Focused media/video proof
remains 42/42.

@vincentkoc
vincentkoc force-pushed the fix/95701-video-generate-deadlock branch from 8b7d479 to 8b23552 Compare June 24, 2026 01:53
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Maintainer update: rebased the focused fix onto current origin/main after PR #94154 landed.

Fresh proof on the rebased head:

  • focused media-generation suites: 6 files, 80/80 tests passed
  • git diff --check: clean
  • autoreview: clean, no accepted/actionable findings

The fix remains scoped to delivery-phase prompt guards and prompt-aware duplicate guards. Hosted checks are being rerun on the rebased head before guarded landing.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 24, 2026
@vincentkoc
vincentkoc requested a review from a team as a code owner June 24, 2026 05:44
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: line Channel integration: line channel: matrix Channel integration: matrix channel: mattermost Channel integration: mattermost labels Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: cbf32de95e745ddf3d5c5263f356a6e92da205c8

@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Maintainer update: the merge guard caught a stale PR parent after main advanced. I rebased the maintainer-editable branch onto the exact current main and force-updated it with a lease.

The PR diff is back to the intended five files; the earlier dependency-guard failure came from inherited mainline drift in the stale parent, not from this fix. Fresh hosted checks are running for the corrected exact head before merge.

@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Maintainer update: main advanced again while the guarded lane was running. The PR branch is now rebased onto the exact current main (1069c60e1e2) and force-updated with a lease; the intended five-file diff is preserved.

Fresh exact-head hosted checks are running before the merge retry.

@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Maintainer update: the final merge guard identified one newly landed merge-critical file (scripts/crabbox-wrapper.mjs). The maintainer-editable branch is now rebased onto the exact current main and force-updated with a lease; the five-file product diff is unchanged.

Fresh exact-head hosted checks are running before the final merge retry.

@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Merged via squash.

Thanks @palomyates516-alt!

@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

video_generate background task stays "running" for minutes after media ready — self-deadlock between wake (expectFinal) and completeTaskRun

2 participants

@palomyates516-alt @vincentkoc