Docker: Fix video.sh busy-loop when se:recordVideo=false (#3164)

[barkep]

When a session capability sets se:recordVideo=false, the recorder's polling loop skipped the sleep before looping back to re-check the node status, since the sleep only ran inside the
caps_se_video_record = "true" branch. With recording_started staying false and the same session id still active, this caused a tight loop (curl + jq + python3 video_nodeQuery.py subprocess) with no delay for the entire remaining duration of that session, consuming CPU shared with the browser process (most noticeable with
videoRecorder.sidecarContainer=false).

Add the missing sleep after the if/else, and track the last skipped session id so the script does not keep re-invoking video_nodeQuery.py for a session that was already decided not to be recorded.

Fixes #3164

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

Closes #3164

[CLAassistant]

All committers have signed the CLA.

[qodo-code-review]

PR Summary by Qodo

Fix video.sh busy-loop when se:recordVideo=false

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Prevent tight polling loop when sessions explicitly disable video recording.
• Always sleep between status checks and avoid re-querying already-skipped sessions.

Diagram

graph TD
A["video.sh recorder loop"] --> B["GET node /status"] --> C["Parse session (jq)"] --> D{"New session & not skipped?"}
D -- "yes" --> E["video_nodeQuery.py (caps)"] --> F{"se:recordVideo?"}
F -- "true" --> G["Start ffmpeg recording"] --> H["sleep poll_interval"]
F -- "false" --> I["Remember skipped session"] --> H
D -- "no" --> H

Loading

High-Level Assessment

The PR’s approach is the right minimal fix: ensure the poll loop always sleeps and memoize the last non-recorded session id to avoid repeated expensive capability checks. More invasive alternatives (restructuring the loop into a single sleep-at-end pattern or moving the decision logic into the Python controller) would add churn without improving correctness for this specific bug.

Files changed (1) +6 / -2

Bug fix (1) +6 / -2

video.shAlways sleep during polling and memoize skipped sessions +6/-2

Always sleep during polling and memoize skipped sessions

• Fixes a busy-loop when se:recordVideo=false by moving the poll sleep outside the recording-enabled branch. Adds skipped_session_id tracking so the script does not repeatedly invoke video_nodeQuery.py for a session that was already determined to skip recording.

Video/video.sh

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 [🐛 Bug]: video.sh busy-loops without sleep when se:recordVideo=false, causing high CPU usage for the whole session

1. Misleading skip reason log 🐞 Bug ◔ Observability

Description

In Video/video.sh, the skip log always claims "(se:recordVideo=false)" whenever caps_se_video_record
is not exactly "true", but video_nodeQuery.py can also resolve recording to "false" via defaults
(e.g., SE_RECORD_VIDEO=false) or other non-capability causes. This can mislead operators during
debugging by attributing skipped recordings to the wrong configuration source.

Code

Video/video.sh[R295-297]

+      else
+        echo "$(date -u +"${ts_format}") [${process_name}] - Recording skipped for session: $session_id (se:recordVideo=false)"
+        skipped_session_id="$session_id"

Evidence

The new log line prints a fixed reason string when skipping, but the Python helper can return
"false" based on the global default when the capability is missing, so the log message is not
necessarily true.

Video/video.sh[264-299]
Video/video_nodeQuery.py[27-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`video.sh` logs a hardcoded reason `(se:recordVideo=false)` for any skip decision, even though the resolved `record_video` value may be `false` due to defaults (e.g. `SE_RECORD_VIDEO=false`) or other non-capability reasons. This reduces log accuracy during incident/debugging.

### Issue Context
`video_nodeQuery.py` determines `record_video` from either the capability (`VIDEO_CAP_NAME`, default `se:recordVideo`) or falls back to the `SE_RECORD_VIDEO` environment default when the capability is absent.

### Fix Focus Areas
- Video/video.sh[295-297]
- Video/video_nodeQuery.py[27-56]

### Suggested fix
Update the skip log message to avoid asserting the reason. For example:
- Log the resolved value: `Recording skipped for session: $session_id (recordVideo=$caps_se_video_record)`
- Or log both the capability key and resolved value, without claiming it was explicitly set false.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---