fix(worker): Run setInterval as blocking

[brendan-kellam]

setInterval is not blocking, so if a callback takes longer than the polling interval to execute, than we can run into situations where multiple setInterval callbacks run in parallel.

This change adds setIntervalAsync that handles this. See: https://mottaquikarim.github.io/dev/posts/setinterval-that-blocks-on-await/

Summary by CodeRabbit

Bug Fixes

• Fixed race condition in job schedulers that could cause concurrent execution conflicts, resulting in data inconsistencies and scheduling failures. Significantly improved backend system stability and reliability of background job processing. Ensures more dependable scheduler operation across all components, particularly during simultaneous operations and peak-load situations.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This PR fixes a race condition in job schedulers by introducing setIntervalAsync, a utility that prevents concurrent executions of async callbacks. The utility is then applied across multiple scheduler implementations to ensure safe periodic task execution.

Changes

Cohort / File(s)	Change Summary
Documentation CHANGELOG.md	Added "Fixed race condition in job schedulers" entry under Unreleased section
Async Interval Utility packages/backend/src/utils.ts	Added setIntervalAsync function that wraps async callbacks with a concurrency guard using a per-target isRunning flag to prevent overlapping executions
Scheduler Implementations packages/backend/src/connectionManager.ts, packages/backend/src/ee/accountPermissionSyncer.ts, packages/backend/src/ee/repoPermissionSyncer.ts, packages/backend/src/repoIndexManager.ts	Replaced setInterval with setIntervalAsync imported from utils to apply the concurrency guard across all periodic schedulers

Sequence Diagram

sequenceDiagram participant Caller participant setIntervalAsync participant isRunning Flag participant Async Task Note over Caller,Async Task: setIntervalAsync prevents concurrent executions rect rgb(230, 245, 250) Note over setIntervalAsync,isRunning Flag: First interval tick Caller->>setIntervalAsync: Interval fires (tick 1) setIntervalAsync->>isRunning Flag: Check isRunning isRunning Flag-->>setIntervalAsync: false setIntervalAsync->>isRunning Flag: Set isRunning = true setIntervalAsync->>Async Task: Await task (500ms) Async Task-->>setIntervalAsync: Task completes setIntervalAsync->>isRunning Flag: Set isRunning = false end rect rgb(255, 240, 245) Note over setIntervalAsync,isRunning Flag: Second interval tick (overlapping) Caller->>setIntervalAsync: Interval fires (tick 2) setIntervalAsync->>isRunning Flag: Check isRunning isRunning Flag-->>setIntervalAsync: true setIntervalAsync->>setIntervalAsync: Return early (skip execution) Note over setIntervalAsync: Prevents race condition end 

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

• Focus areas:
  • Verify the setIntervalAsync concurrency guard logic is correct and thread-safe
  • Confirm all scheduler files correctly imported and applied setIntervalAsync
  • Ensure no timing-dependent behavior was inadvertently changed by the interval mechanism swap

Possibly related PRs

• #600: Refactors schedulers in accountPermissionSyncer and repoPermissionSyncer to account-based jobs, overlapping with scheduler modifications in this PR.

Suggested reviewers

• msukkari

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing a race condition by making setInterval blocking (via setIntervalAsync), which aligns with the primary objective of preventing concurrent executions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[brendan-kellam]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d63da4b and e70ca49.

📒 Files selected for processing (6)

• CHANGELOG.md (1 hunks)
• packages/backend/src/connectionManager.ts (2 hunks)
• packages/backend/src/ee/accountPermissionSyncer.ts (2 hunks)
• packages/backend/src/ee/repoPermissionSyncer.ts (2 hunks)
• packages/backend/src/repoIndexManager.ts (2 hunks)
• packages/backend/src/utils.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*

📄 CodeRabbit inference engine (.cursor/rules/style.mdc)

Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.

Files:

• packages/backend/src/utils.ts
• packages/backend/src/repoIndexManager.ts
• CHANGELOG.md
• packages/backend/src/connectionManager.ts
• packages/backend/src/ee/accountPermissionSyncer.ts
• packages/backend/src/ee/repoPermissionSyncer.ts

🔇 Additional comments (9)

CHANGELOG.md (1)

10-12: LGTM - Changelog entry is appropriate.

The changelog entry accurately describes the race condition fix and follows the project's changelog format.

packages/backend/src/ee/accountPermissionSyncer.ts (2)

10-10: LGTM - Import is correct.

The import path ../utils.js correctly references the new utility function.

---

50-50: LGTM - Correctly prevents overlapping scheduler executions.

Replacing setInterval with setIntervalAsync ensures that if the permission sync query and job scheduling takes longer than 5 seconds, a new execution won't start until the current one completes, preventing the race condition.

packages/backend/src/repoIndexManager.ts (2)

15-15: LGTM - Import is correct.

The import correctly includes setIntervalAsync from the utils module.

---

75-77: LGTM - Correctly prevents overlapping scheduler executions.

The changes are well-implemented:

• Removing async from the function signature is correct since startScheduler no longer awaits any operations
• Using setIntervalAsync ensures that if scheduleIndexJobs or scheduleCleanupJobs takes longer than the polling interval, a new execution won't start until the current one completes, preventing the race condition

packages/backend/src/connectionManager.ts (2)

10-10: LGTM - Import is correct.

The import correctly includes setIntervalAsync from the utils module.

---

69-69: LGTM - Correctly prevents overlapping scheduler executions.

Replacing setInterval with setIntervalAsync ensures that if the connection sync query and job creation takes longer than the polling interval, a new execution won't start until the current one completes, preventing the race condition.

packages/backend/src/ee/repoPermissionSyncer.ts (2)

11-11: LGTM - Import is correct.

The import path ../utils.js correctly references the new utility function.

---

51-51: LGTM - Correctly prevents overlapping scheduler executions.

Replacing setInterval with setIntervalAsync ensures that if the repo permission sync query and job scheduling takes longer than 5 seconds, a new execution won't start until the current one completes, preventing the race condition.