fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(worker): Fix issues with gracefully shutting down - #612

Merged
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround
Nov 12, 2025
Merged

fix(worker): Fix issues with gracefully shutting down#612
brendan-kellam merged 8 commits into
mainfrom
bkellam/groupmq_workaround

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

This PR resolves a couple of issues:

  • During termination, we call worker.close() to terminate any in progress jobs. This takes a gracefulTimeout param (defaults 30 seconds). If there are any jobs still in progress after that duration, there is a bug in groupmq where the job's group lock will not be released (see [bug] Group lock is not released after graceful shutdown, resulting in stalling jobs Openpanel-dev/groupmq#8). This resulted in jobs (especially long running connection jobs) being left in PENDING or IN_PROGRESS indefinitely, and new jobs not being queued until the timeout expires. The current workaround is to explicitly free the locks when calling dispose.
  • Improved the graceful shutdown handling in index.ts

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Implemented timeout handling for background jobs; timed-out jobs are now properly marked as failed with status updates and metrics.
  • Added job state validation to prevent execution of jobs in invalid states.

Improvements

  • Enhanced graceful shutdown process with comprehensive error handling and error tracking integration.
  • Improved lock management during shutdown to prevent deadlocks.
  • Extended process signal handling for more reliable termination.

@coderabbitai

coderabbitaiBot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

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

The pull request introduces graceful shutdown handling for job queues, implements job state validation before execution, enriches job payloads with metadata, adds a reusable signal listener for shutdown orchestration, and updates method signatures to pass AbortSignal instead of AbortController instances.

Changes

Cohort / File(s)Summary
Graceful timeout and job validation
packages/backend/src/connectionManager.ts, packages/backend/src/repoIndexManager.ts
Added graceful-timeout event handlers that mark timed-out jobs as FAILED with status updates and metric emissions. Implemented job state validation in runJob to fetch and verify job status is PENDING or IN_PROGRESS before execution. Added manual Redis lock cleanup during shutdown to release group locks for in-progress jobs. Extended constructors to store Redis instance as private field for lock management.
Shutdown signal handling
packages/backend/src/index.ts
Refactored cleanup flow into a new listenToShutdownSignals function that sequentially disposes managers, disconnects services, and handles errors with Sentry. Replaced hard-coded cleanup with signal handler registration loop over SHUTDOWN_SIGNALS and added uncaught exception/rejection handlers.
Shutdown constants
packages/backend/src/constants.ts
Added GROUPMQ_WORKER_STOP_GRACEFUL_TIMEOUT_MS (5 seconds) for worker shutdown timeout and SHUTDOWN_SIGNALS array listing Node.js termination signals.
Parameter type refinement
packages/backend/src/repoCompileUtils.ts
Changed compileGithubConfig signature to accept signal: AbortSignal instead of abortController: AbortController, forwarding the signal to getGitHubReposFromConfig.

Sequence Diagram(s)

sequenceDiagram
participant Worker
participant JobQueue
participant DB as Prisma/Redis
participant Handler
rect rgb(200, 220, 255)
note over Worker: Job Lifecycle with Graceful Timeout
end
Worker->>Worker: runJob called
Worker->>DB: Fetch current job status
DB-->>Worker: Job status
alt Status is PENDING or IN_PROGRESS
Worker->>Worker: Execute job
alt Normal Completion
Worker->>Handler: Job success
else Graceful Timeout
Handler->>Worker: onJobGracefulTimeout
Worker->>DB: Mark job as FAILED
Worker->>Handler: Update metrics & log
end
else Status is other
Worker->>Handler: Throw error (skip invalid state)
end
Loading
sequenceDiagram
participant System
participant Index as index.ts
participant Managers as ConnectionManager<br/>RepoIndexManager
participant Services as Prisma<br/>Redis<br/>API<br/>Posthog
participant Signals
rect rgb(255, 220, 200)
note over System: Graceful Shutdown Flow
end
Signals->>Index: SIGTERM/SIGINT/etc.
Index->>Index: listenToShutdownSignals
Index->>Managers: dispose() each manager
par Manager Cleanup
Managers->>Managers: Worker.gracefulStop(timeout)
Managers->>Services: Delete Redis lock keys
Managers->>Managers: Log lock release
end
Index->>Services: Disconnect each service
Services-->>Index: Disconnected
Index->>Index: Exit gracefully
Index->>Signals: Remove handlers
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Lock cleanup logic in connectionManager.ts and repoIndexManager.ts: Manual Redis key deletion during disposal—verify error handling is robust and doesn't prevent shutdown
  • Job state validation logic: Ensure the early-exit pattern for invalid job states doesn't suppress legitimate errors or cause jobs to be lost
  • Signal handler guard mechanism: Verify the guard against repeated signal processing works correctly and doesn't prevent graceful shutdown if triggered multiple times
  • Payload enrichment impact: Confirm job payload expansion (jobId, connectionId, connectionName, orgId) doesn't break existing job consumers or queue serialization
  • Parameter type change in repoCompileUtils.ts: Ensure all call sites pass AbortSignal correctly and the change is backward-compatible with consumers

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: implementing graceful shutdown fixes for workers, which is the core objective of the pull request.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions

This comment has been minimized.

@brendan-kellam

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitaicoderabbitaiBot left a comment

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.

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 18fad64 and d43f35b.

📒 Files selected for processing (5)
  • packages/backend/src/connectionManager.ts (9 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/index.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (1 hunks)
  • packages/backend/src/repoIndexManager.ts (6 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/constants.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/connectionManager.ts

Comment threadpackages/backend/src/index.ts
@brendan-kellam
brendan-kellam merged commit 903d15a into mainNov 12, 2025
9 checks passed
@brendan-kellam
brendan-kellam deleted the bkellam/groupmq_workaround branch November 12, 2025 04:12
@github-actionsgithub-actionsBot mentioned this pull request Nov 12, 2025
Sign up for freeto 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

@brendan-kellam