Skip to content

Fix M5 feedback: bound cleanup interval to prevent overflow#6925

Merged
Jasonnnz merged 1 commit into
feature/qa-video-automationfrom
swarm/qa-video-auto/task-11
Feb 23, 2026
Merged

Fix M5 feedback: bound cleanup interval to prevent overflow#6925
Jasonnnz merged 1 commit into
feature/qa-video-automationfrom
swarm/qa-video-auto/task-11

Conversation

@Jasonnnz
Copy link
Copy Markdown
Contributor

@Jasonnnz Jasonnnz commented Feb 23, 2026

Clamps cleanupIntervalMs to 2^31-1 ms before passing to setInterval to prevent near-continuous execution from timer overflow. Part of #6899.


Open with Devin

Co-Authored-By: Claude <noreply@anthropic.com>
@Jasonnnz Jasonnnz self-assigned this Feb 23, 2026
@Jasonnnz Jasonnnz merged commit bb9e1d3 into feature/qa-video-automation Feb 23, 2026
@Jasonnnz Jasonnnz deleted the swarm/qa-video-auto/task-11 branch February 23, 2026 16:41
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Log message reports original intervalMs instead of clamped safeInterval

When startRecordingCleanup is called with an intervalMs value exceeding 2^31-1, the runtime clamp on line 81 correctly limits the actual interval passed to setInterval. However, the log message on line 93 still reports the original unclamped intervalMs value, misleading operators into thinking the timer is running at the requested (overflowing) interval.

Detailed Explanation

On line 81, safeInterval is computed as Math.min(intervalMs, MAX_INTERVAL_MS), and this clamped value is correctly passed to setInterval on line 89. But the log on line 93 logs { intervalMs } — the original parameter — not the actual value used:

log.info({ intervalMs }, 'Recording cleanup worker started');

If clamping occurs, the log would say e.g. intervalMs: 3000000000 while the real interval is 2147483647. This makes debugging timer behavior harder.

In practice, the schema validation (.max(2_147_483_647) at assistant/src/config/schema.ts:1067) prevents out-of-range values from reaching this code path via config, so this only affects direct callers bypassing config validation.

Impact: Misleading log output when clamping is triggered; low practical impact given schema validation.

(Refers to line 93)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Jasonnnz added a commit that referenced this pull request Feb 23, 2026
Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude <noreply@anthropic.com>
Jasonnnz added a commit that referenced this pull request Feb 23, 2026
Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude <noreply@anthropic.com>
Jasonnnz added a commit that referenced this pull request Feb 23, 2026
Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude <noreply@anthropic.com>
Jasonnnz added a commit that referenced this pull request Feb 23, 2026
Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude <noreply@anthropic.com>
Jasonnnz added a commit that referenced this pull request Feb 23, 2026
Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude <noreply@anthropic.com>
Jasonnnz added a commit that referenced this pull request Feb 23, 2026
Co-authored-by: Vellum Assistant <assistant@vellum.ai>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant