feat(backend): add BullMQ JobManager framework - #1427
Conversation
Introduces a JobManager over BullMQ: work-queue Workloads with a ProcessContext, CronWorkloads multiplexed onto a shared cron worker with a reconcile() sweep helper, a JobProducer that owns the queues and the deduplicated enqueue path, and a Redis-backed read model (status/jobDetail). Wired into the backend entrypoint with a demo workload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughChangesWorkload orchestration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
License Audit
Weak Copyleft Packages (informational)
Resolved Packages (17)
|
|
@brendan-kellam your pull request is missing a changelog! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 65ec093. Configure here.
| { | ||
| every: intervalMs, | ||
| startDate: Date.now() + intervalMs, | ||
| }, |
There was a problem hiding this comment.
Schedulers delay work after upsert
High Severity
upsertJobScheduler always sets startDate to now plus one full interval. Startup reconcile upserts every connection, repo, and permission scheduler, so after each worker restart overdue syncs and reindexes wait an entire interval instead of running soon, unlike the old pollers.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 65ec093. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (21)
packages/shared/src/utils.test.ts (1)
98-112: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for deprecated-key fallback.
Add tests for
experiment_repoDrivenPermissionSyncIntervalMsandexperiment_userDrivenPermissionSyncIntervalMs. Also verify that current keys override deprecated keys when both are configured.The uncovered branches are in
packages/shared/src/utils.ts:82-90.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/utils.test.ts` around lines 98 - 112, Extend the resolveConfigSettings tests to cover fallback from experiment_repoDrivenPermissionSyncIntervalMs and experiment_userDrivenPermissionSyncIntervalMs to their current keys when only deprecated values are provided. Add cases confirming each current key takes precedence when both current and deprecated values are configured, using the existing DEFAULT_CONFIG_SETTINGS expectations where applicable.packages/backend/src/repoIndexWorkload.ts (2)
34-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why this workload sets job state in
processinstead ofonStarted.Every other workload in this cohort marks the job
IN_PROGRESSand updates the parentlatest...JobIdinonStarted. This workload performs the same writes insideprocessthroughprepareRepoIndexJob, so that eligibility checks can skip the job without creating anIN_PROGRESSrow. The deviation is reasonable, but it is not stated in the code.Add a short comment that explains the deviation, so later changes do not move the logic into
onStartedand reintroduce spuriousIN_PROGRESSrows for skipped jobs.As per path instructions for
packages/backend/**/*.{ts,tsx}: "InonStarted, upsert the workload-specific job row asIN_PROGRESS; update a parent resource'slatest...JobIdin the same transaction."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/repoIndexWorkload.ts` around lines 34 - 56, Add a brief comment near the `prepareRepoIndexJob` call in `process` documenting that job-state and parent `latest...JobId` updates intentionally occur there, rather than in `onStarted`, so eligibility checks can skip jobs without creating spurious `IN_PROGRESS` rows.Source: Path instructions
279-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicate repository-root validation.
Line 279 already awaits
isPathAValidGitRepoRootand enters the block only when the path is not a valid git repository root. Line 280 repeats the same check. The second call only addssignal. Movesignalinto the first call and drop the second.♻️ Proposed refactor
- if (existsSync(repoPath) && !(await isPathAValidGitRepoRoot({ path: repoPath }))) { - const isValidGitRepo = await isPathAValidGitRepoRoot({ - path: repoPath, - signal, - }); - - if (!isValidGitRepo && !isReadOnly) { - logger.warn(`${repoPath} is not a valid git repository root. Deleting directory and performing fresh clone.`); - await rm(repoPath, { recursive: true, force: true }); - } - } + if ( + existsSync(repoPath) && + !isReadOnly && + !(await isPathAValidGitRepoRoot({ path: repoPath, signal })) + ) { + logger.warn(`${repoPath} is not a valid git repository root. Deleting directory and performing fresh clone.`); + await rm(repoPath, { recursive: true, force: true }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/repoIndexWorkload.ts` around lines 279 - 289, Update the initial isPathAValidGitRepoRoot call in the repository validation block to pass signal, then reuse its result for the invalid-repository branch. Remove the second isPathAValidGitRepoRoot invocation and preserve the existing isReadOnly guard, warning, and deletion behavior.packages/backend/src/attachmentPruneWorkload.ts (1)
133-143: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBound the
failedIdsexclusion list.
failedIdsgrows for every tombstone whose bytes cannot be deleted. If a storage outage affects many rows, thenotInlist can reach thousands of values in one run. Prisma sends each value as a bind parameter, which can hit database parameter limits and slows each query.Consider stopping the run after a failure threshold, or paginating by a cursor on
idinstead of excluding failed IDs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/attachmentPruneWorkload.ts` around lines 133 - 143, Bound the failedIds handling in the attachment-pruning loop so the notIn filter cannot grow without limit during storage failures. Prefer stopping the run after a defined failure threshold, or replace failedIds exclusion with cursor-based id pagination while preserving retries for eligible DELETING attachments; update the workload’s main pruning function and its findMany query accordingly.packages/backend/src/connectionWorkload.ts (4)
209-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider reporting terminal connection-sync failures to Sentry.
onTerminalFailurerecords the error in the database only. The permission-sync workloads callSentry.captureException(error, { tags: { jobId, queue } })in the same hook (seepackages/backend/src/ee/repoPermissionSyncWorkload.tslines 48-207 in the provided context). Add the same reporting here so connection-sync failures stay visible in Sentry.♻️ Proposed change
onTerminalFailure: async ({ jobId }, error) => { + Sentry.captureException(error, { + tags: { + jobId, + queue: CONNECTION_QUEUE.name, + }, + }); await db.connectionSyncJob.update({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/connectionWorkload.ts` around lines 209 - 220, Update the onTerminalFailure hook to report the received error to Sentry with captureException, including jobId and the connection-sync queue name in the tags, while preserving the existing database failure update.
238-248: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace the O(n²) deduplication with a keyed map.
filterplusfindIndexcompares every repository against every earlier repository. Connection discovery can return thousands of repositories, so this scales quadratically on a hot path. AMapkeyed by the same composite key gives the same first-wins result in linear time.⚡ Proposed change
-const deduplicateRepos = (repos: RepoData[]): RepoData[] => - repos.filter( - (repo, index, allRepos) => - index === - allRepos.findIndex( - (candidate) => - candidate.external_id === repo.external_id && - candidate.external_codeHostUrl === - repo.external_codeHostUrl, - ), - ); +const deduplicateRepos = (repos: RepoData[]): RepoData[] => { + const uniqueRepos = new Map<string, RepoData>(); + for (const repo of repos) { + const key = `${repo.external_id}\u0000${repo.external_codeHostUrl}`; + if (!uniqueRepos.has(key)) { + uniqueRepos.set(key, repo); + } + } + return [...uniqueRepos.values()]; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/connectionWorkload.ts` around lines 238 - 248, Update deduplicateRepos to use a Map keyed by the composite external_id and external_codeHostUrl values, preserving the current first-wins behavior while reducing deduplication to linear time. Return the Map’s values as the resulting RepoData array.
274-304: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftSequential upserts make discovery latency scale with repository count.
Each discovered repository triggers one awaited
db.repo.upsertround trip. A connection with several thousand repositories produces the same number of sequential round trips. Consider batching the upserts with a bounded concurrency helper, or splitting the work into acreateMany/updateManypair where the schema allows it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/connectionWorkload.ts` around lines 274 - 304, Update the repository persistence loop in the connection workload to avoid awaiting each db.repo.upsert sequentially. Use the project’s bounded-concurrency helper to process deduplicateRepos(discoveredRepos) with a controlled number of concurrent upserts, while preserving the existing upsert payload, selected fields, and currentRepos collection behavior.
365-375: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBound the scheduler fan-out.
Promise.allissues one Redis scheduler upsert per current repository at the same time. A large connection creates thousands of concurrent Redis commands in one burst. Consider chunking the calls, for example in batches of 50 to 100, to keep Redis pressure predictable. The same applies to the permission-sync scheduler loop at lines 452-462.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/connectionWorkload.ts` around lines 365 - 375, Bound scheduler fan-out in the current repository indexing loop by replacing the unbounded Promise.all over currentRepos with sequential batches of roughly 50–100 upsertJobScheduler calls, while preserving each repo’s existing arguments. Apply the same batching approach to the permission-sync scheduler loop, ensuring the next batch starts only after the prior batch completes.packages/backend/src/connectionWorkload.test.ts (2)
26-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
importOriginalfor the@sourcebot/sharedmock.This mock replaces the whole module and hardcodes
CONNECTION_QUEUEandJOB_PRIORITIES. The assertions at lines 314 and 322 then compare against literals that are copies of the real constants. If the realJOB_PRIORITIESvalues change, these tests keep passing against stale values. The permission-sync tests in this cohort spreadimportOriginaland override onlyloadConfigand the queue spec. Use the same pattern here so only the intentionally stubbed exports are replaced.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/connectionWorkload.test.ts` around lines 26 - 62, The `@sourcebot/shared` mock in the connection workload tests should preserve real exports instead of hardcoding CONNECTION_QUEUE and JOB_PRIORITIES. Update the mock factory to use importOriginal, retain the actual shared module values, and override only loadConfig and the intentionally customized queue configuration while keeping the existing test behavior.
341-377: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the swallowed search-context failure.
@sentry/nodeis mocked at line 22 but no test asserts against it. Theprocessimplementation catchessyncSearchContextserrors, logs them, and callsSentry.captureException(lines 143-156 ofpackages/backend/src/connectionWorkload.ts). That is the one path where a failure does not fail the job. Add a test that rejectsmocks.syncSearchContextsand asserts the job still resolves and the error reaches Sentry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/connectionWorkload.test.ts` around lines 341 - 377, Add a test for connectionWorkload.process that makes mocks.syncSearchContexts reject, verifies the job still resolves, and asserts the rejected error is passed to the mocked Sentry.captureException. Reuse the existing successful workload setup and lifecycle context, targeting the syncSearchContexts error-handling path rather than the repo reconciliation failure path.packages/backend/src/ee/accountPermissionSyncWorkload.test.ts (2)
311-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the permission write in the success path.
permissionCreateManyandpermissionDeleteManyare mocked but this test only checks the client construction and the transaction count. The success path result that users depend on is the set of persistedaccountToRepoPermissionrows. Add a case that returns repositories frommocks.getReposForAuthenticatedBitbucketServerUserand asserts the resultingcreateManypayload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/ee/accountPermissionSyncWorkload.test.ts` around lines 311 - 327, The success-path test “syncs the requested account” does not verify persisted permissions. Configure mocks.getReposForAuthenticatedBitbucketServerUser to return repositories, then assert permissionCreateMany was called with the expected accountToRepoPermission rows and preserve the existing client and transaction assertions.
431-459: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a stale-job case for
onCompleted.This test covers the case where the job is still the latest. The guard that matters is the opposite case.
accountUpdateManyis conditioned onlatestPermissionSyncJobId: "job_1", so a superseded job must not clearpermissionSyncIssue. Add a test that setsaccountUpdateMany.mockResolvedValue({ count: 0 })and asserts the hook resolves and still updates its own job row.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/ee/accountPermissionSyncWorkload.test.ts` around lines 431 - 459, Add a stale-job test alongside the existing latest-job onCompleted test: configure accountUpdateMany to resolve with { count: 0 }, invoke createWorkload().onCompleted with the existing lifecycle context, assert the hook resolves without throwing, and verify permissionSyncJobUpdate still updates job_1 to COMPLETED while the account issue is not cleared.packages/backend/src/ee/repoPermissionSyncWorkload.test.ts (1)
39-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeveral mocked branches are never exercised.
mocks.getProjectMembers,mocks.createGitLabFromPersonalAccessToken, andmocks.getUserPermissionsForServerRepoare registered here but no test drives the GitLab or Bitbucket Server paths. The missing-credentials branch is also uncovered: the implementation throwsNo credentials found for repo ${id}whengetAuthCredentialsForReporesolves to a falsy value (seepackages/backend/src/ee/repoPermissionSyncWorkload.tslines 48-207 in the provided context). Add cases for at least the missing-credentials branch and one non-GitHub provider.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/ee/repoPermissionSyncWorkload.test.ts` around lines 39 - 65, Add tests in the repo permission sync workload suite that cover getAuthCredentialsForRepo returning a falsy value and assert the expected “No credentials found for repo ${id}” error, plus a successful non-GitHub provider path such as GitLab or Bitbucket Server that exercises its registered client and permission mocks. Ensure the selected provider test drives the corresponding symbols, including getProjectMembers/createGitLabFromPersonalAccessToken or getUserPermissionsForServerRepo.packages/backend/src/jobManager.ts (1)
115-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTimed-out worker close leaves work in flight.
stop()racesworker.close()against a timer. When the timer wins,stop()continues and closes the BullMQ client while the worker still processes a job. That job then fails with connection errors instead of stalling cleanly. Callworker.close(true)(force) after the grace period so the worker stops before the shared client closes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/jobManager.ts` around lines 115 - 132, Update JobManager.stop so each worker is force-closed with worker.close(true) when the graceful shutdown race times out, ensuring all workers have stopped before bullmqClient.close() runs. Preserve the existing graceful close and timeout behavior.packages/shared/src/jobLogger.test.ts (1)
118-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the parsed structured entry with a literal object.
Line 120 builds the expected value with
parseJobLogEntry, the same production code path under test. A defect inparseJobLogEntryproduces a matching expectation, so this assertion cannot fail. Use a literal object for the structured entry, as the test already does for the legacy entry.♻️ Proposed change
expect(result).toEqual({ logs: [ - parseJobLogEntry(structuredEntry), + { + version: 1, + timestamp: "2026-07-28T03:00:00.000Z", + level: "info", + message: "Started", + attempt: 1, + }, {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/jobLogger.test.ts` around lines 118 - 130, Replace parseJobLogEntry(structuredEntry) in the expected logs array of the relevant test with a literal object containing the structured entry’s expected parsed fields, matching the existing legacy-entry assertion style. Keep the production call under test unchanged and preserve the count and legacy log expectations.packages/backend/src/executionLock.ts (2)
109-122: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
isLockContentionagainst rejected attempt promises.
error.attemptsholds promises. If any promise rejects,Promise.allrejects inside thecatchblock at Line 192. The rejection then replaces the original Redlock error and hides the acquisition failure cause. Returnfalsewhen the attempts cannot be inspected.🛡️ Proposed change
const isLockContention = async (error: unknown): Promise<boolean> => { if (!(error instanceof ExecutionError) || error.attempts.length === 0) { return false; } - const attempts = await Promise.all(error.attempts); + let attempts; + try { + attempts = await Promise.all(error.attempts); + } catch { + return false; + } return attempts.every(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/executionLock.ts` around lines 109 - 122, Update isLockContention to handle rejected promises from error.attempts without propagating the rejection: wrap the Promise.all inspection in error handling and return false when any attempt cannot be resolved, while preserving the existing contention checks for successfully resolved attempts.
156-205: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider a bound on contention retries.
The loop retries lock acquisition without a limit. Only shutdown ends the wait. A long-held lock keeps a worker slot occupied for the whole contention period, which reduces effective concurrency for that queue. Add a maximum wait or a maximum retry count, then fail the attempt so BullMQ retry and backoff handle rescheduling. Emit a metric or warning when the wait exceeds a threshold so contention is observable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/executionLock.ts` around lines 156 - 205, Bound the contention retry loop in the lock-acquisition method surrounding redlock. Track elapsed wait time or retry attempts, stop retrying after a configured maximum, and throw the contention error so BullMQ can reschedule with its retry/backoff policy; also emit the existing warning or metric when the threshold is exceeded. Preserve immediate propagation for acquired-lock failures and shutdown aborts.packages/backend/src/jobManager.test.ts (1)
365-397: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the non-terminal failure branch and hook errors.
The suite only exercises the terminal path where
attemptsMadeequalsopts.attempts. Add two cases: a failure withattemptsMadebelowopts.attemptsmust not callonTerminalFailure, and a hook that rejects must callSentry.captureExceptionand still flush the logger. These cases protect the classification logic flagged inpackages/backend/src/jobManager.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/jobManager.test.ts` around lines 365 - 397, Add tests alongside the existing terminal-failure test for the non-terminal failed-job path and rejected lifecycle hooks. Verify a failure with attemptsMade below opts.attempts does not invoke onTerminalFailure, and verify a rejecting hook calls Sentry.captureException while the job logger still flushes; use the existing BullMQJobManager, worker failed handler, and logger mocks.packages/backend/src/configManager.ts (1)
153-164: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueScheduler removal and connection deletion are not atomic.
removeJobSchedulerruns beforeprisma.connection.delete. If the delete fails, the scheduler is already gone and the connection stops syncing while still present in the database. The reverse order leaves an orphan scheduler that enqueues jobs for a missing connection.The current order is the safer one. Consider logging a warning when
removeJobSchedulerreturnsfalse, so orphaned schedulers are visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/configManager.ts` around lines 153 - 164, Update the deleted-connection loop to capture the boolean result of removeJobScheduler for each connection and log a warning when it returns false, including the connection ID or name so orphaned schedulers are identifiable; preserve the existing removal-before-prisma.connection.delete order.packages/backend/src/reconcileJobSchedulersAtStartup.ts (2)
48-66: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider bounding scheduler upsert concurrency at startup.
targetscontains one entry per repository. Large installations produce thousands of concurrentupsertJobSchedulercalls against Redis in onePromise.all. This can saturate the connection pool during startup.Batch the upserts, for example in chunks of 50 to 100.
The
jobOptionsbranch is also redundant.upsertJobSchedulerdeclaresoptionsas optional, so passingundefinedis equivalent.♻️ Suggested simplification of the branch
- targets.map(({ schedulerId, data }) => { - if (jobOptions) { - return jobManager.upsertJobScheduler( - workloadName, - schedulerId, - schedule, - data, - jobOptions, - ); - } - return jobManager.upsertJobScheduler( - workloadName, - schedulerId, - schedule, - data, - ); - }), + targets.map(({ schedulerId, data }) => + jobManager.upsertJobScheduler( + workloadName, + schedulerId, + schedule, + data, + jobOptions, + ), + ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/reconcileJobSchedulersAtStartup.ts` around lines 48 - 66, Limit startup scheduler upserts in the targets mapping within reconcileJobSchedulersAtStartup by processing entries in bounded batches (for example, 50–100) rather than issuing every upsert through one unbounded Promise.all, while preserving completion of all targets. Simplify the jobManager.upsertJobScheduler invocation by removing the redundant jobOptions branch and passing the optional value consistently.
133-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScheduler ID prefixes are duplicated across files. Three of the four workloads build scheduler IDs from inline string literals instead of shared helpers, so the prefix used for cleanup and the template used for creation can drift apart.
account-permission-syncalready usesACCOUNT_PERMISSION_SYNC_SCHEDULER_ID_PREFIXandgetAccountPermissionSyncSchedulerId. Follow that pattern for the rest.
packages/backend/src/reconcileJobSchedulersAtStartup.ts#L133-L172: replace the inlineconnection-sync-v1-,repo-index-v1-, andrepo-permission-sync-v1-prefixes and ID templates with exported prefix constants and ID helper functions.packages/backend/src/configManager.ts#L18-L19: delete the localgetConnectionSyncSchedulerIdand import the shared connection-sync ID helper instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/reconcileJobSchedulersAtStartup.ts` around lines 133 - 172, Replace the inline scheduler prefixes and ID templates in reconcileJobSchedulersAtStartup with shared exported prefix constants and ID helper functions for connection-sync, repo-index, and repo-permission-sync, matching the existing account-permission-sync pattern; update packages/backend/src/reconcileJobSchedulersAtStartup.ts lines 133-172 accordingly. In packages/backend/src/configManager.ts lines 18-19, remove the local getConnectionSyncSchedulerId and import the shared connection-sync helper instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backend/src/api.ts`:
- Around line 35-41: Protect the Bull Board route in the API setup around
createBullBoard and app.use('/admin/queues', ...): add shared-secret or
basic-auth middleware before mounting the router, and apply helmet() for the
dashboard response. Ensure both protections remain in effect whenever the
backend is externally reachable, while preserving the existing read-only
BullMQAdapter configuration.
In `@packages/backend/src/attachmentPruneWorkload.ts`:
- Around line 60-114: Update process in
packages/backend/src/attachmentPruneWorkload.ts (lines 60-114) to destructure
signal, call signal.throwIfAborted() before each attachment claim and batch
delete, and pass signal to reclaimTombstonedAttachments; update the audit-log
prune loop in packages/backend/src/ee/auditLogPruneWorkload.ts (lines 37-70) to
destructure signal and call signal.throwIfAborted() at the start of every while
iteration.
In `@packages/backend/src/configManager.test.ts`:
- Around line 167-168: Update the test around mocks.upsertJobScheduler to expect
it is called with the current interval when an existing connection is processed,
reflecting the unconditional scheduler upsert in configManager; retain the
assertion that mocks.trigger is not called.
In `@packages/backend/src/configManager.ts`:
- Around line 116-124: Update ConfigManager.syncConnections in
packages/backend/src/configManager.ts:116-124 to call upsertJobScheduler for
every declarative connection, removing the existingConnection guard. Update
packages/backend/src/configManager.test.ts:167-168 to assert the unchanged
connection’s scheduler is upserted with the current interval while trigger
remains uncalled.
In `@packages/backend/src/connectionWorkload.ts`:
- Around line 492-524: Add an exhaustive default branch to
discoverConnectionRepositories that throws a clear error containing the
unexpected config.type, ensuring unhandled runtime connection types cannot fall
through and return undefined.
- Around line 501-523: Thread the workload AbortSignal from the switch through
every non-GitHub compiler and its provider helpers, including
compileGitlabConfig, compileGiteaConfig, compileGerritConfig,
compileBitbucketConfig, compileAzureDevOpsConfig, and
compileGenericGitHostConfig. Update their API, retry, filesystem, and Git
command operations to accept and honor the signal, preserving cancellation when
the connection lock is lost.
In `@packages/backend/src/ee/accountPermissionSyncWorkload.ts`:
- Around line 168-170: Remove email addresses from persisted job-log messages in
accountPermissionSyncWorkload.ts at lines 168-170, 242-244, 313-315, and
346-348, while retaining account.id and account.providerId. In
repoPermissionSyncWorkload.ts at lines 274-276, update the log to include only
collaborator count and logins, dropping the email field.
- Around line 281-311: Update the onCompleted and onTerminalFailure lifecycle
hooks in accountPermissionSyncWorkload to use
accountPermissionSyncJob.updateMany for status writes keyed only by jobId, so
missing cascaded rows do not throw and the hooks do not require the job to
remain current. If completion still needs the related account, fetch it
separately and preserve the existing account metadata update.
In `@packages/backend/src/ee/repoPermissionSyncWorkload.ts`:
- Around line 278-286: Update the account queries around the visible findMany
calls at lines 278, 317, 376, and 435 to always filter issuerUrl with the
canonical GitHub Cloud URL when credentials.hostUrl is absent, otherwise using
repo.external_codeHostUrl as appropriate. Do not allow an undefined issuerUrl or
use ?? null; preserve the stored canonical issuer for normal cloud accounts and
prevent matching accounts from other GitHub issuers.
In `@packages/backend/src/jobManager.ts`:
- Around line 266-284: Update onWorkloadJobFailed to classify BullMQ’s “job
stalled more than allowable limit” error as terminal even when attemptsMade is
below job.opts.attempts. Use this classification for the existing
terminal-failure path so onTerminalFailure updates the lifecycle row, while
preserving normal retry handling for other errors.
In `@packages/backend/src/reconcileJobSchedulersAtStartup.test.ts`:
- Around line 159-164: Fix the negative assertion in the
reconcileJobSchedulersAtStartup test so it inspects recorded upsertJobScheduler
calls with the actual five-argument signature, including jobOptions. Ensure the
assertion specifically verifies that no call contains the permission-sync
scheduler identifier, rather than relying on a mismatched four-argument
toHaveBeenCalledWith pattern.
In `@packages/schemas/src/v3/index.schema.ts`:
- Around line 34-35: Mark the reindexRepoPollingIntervalMs property as
deprecated in both schema definitions, matching the existing deprecated metadata
on the nearby interval setting. Update both occurrences while preserving their
current validation and descriptions.
In `@packages/schemas/src/v3/index.type.ts`:
- Line 104: Update the deprecation JSDoc for reindexRepoPollingIntervalMs to
document both replacements: reindexIntervalMs and resyncConnectionIntervalMs.
Preserve the existing deprecation marker, then regenerate the schema artifacts
so the generated outputs reflect the updated documentation.
In `@schemas/v3/index.json`:
- Around line 54-55: Synchronize the schema description for
maxRepoGarbageCollectionJobConcurrency with the runtime default defined by
maxRepoGarbageCollectionJobConcurrency in constants.ts: update the documented
default from 8 to 2, unless intentionally reverting the runtime constant to 8.
- Around line 33-34: Mark the reindexRepoPollingIntervalMs schema property as
deprecated by adding the same deprecated metadata already used for
resyncConnectionPollingIntervalMs, while preserving its existing minimum
constraint and other definition fields.
---
Nitpick comments:
In `@packages/backend/src/attachmentPruneWorkload.ts`:
- Around line 133-143: Bound the failedIds handling in the attachment-pruning
loop so the notIn filter cannot grow without limit during storage failures.
Prefer stopping the run after a defined failure threshold, or replace failedIds
exclusion with cursor-based id pagination while preserving retries for eligible
DELETING attachments; update the workload’s main pruning function and its
findMany query accordingly.
In `@packages/backend/src/configManager.ts`:
- Around line 153-164: Update the deleted-connection loop to capture the boolean
result of removeJobScheduler for each connection and log a warning when it
returns false, including the connection ID or name so orphaned schedulers are
identifiable; preserve the existing removal-before-prisma.connection.delete
order.
In `@packages/backend/src/connectionWorkload.test.ts`:
- Around line 26-62: The `@sourcebot/shared` mock in the connection workload tests
should preserve real exports instead of hardcoding CONNECTION_QUEUE and
JOB_PRIORITIES. Update the mock factory to use importOriginal, retain the actual
shared module values, and override only loadConfig and the intentionally
customized queue configuration while keeping the existing test behavior.
- Around line 341-377: Add a test for connectionWorkload.process that makes
mocks.syncSearchContexts reject, verifies the job still resolves, and asserts
the rejected error is passed to the mocked Sentry.captureException. Reuse the
existing successful workload setup and lifecycle context, targeting the
syncSearchContexts error-handling path rather than the repo reconciliation
failure path.
In `@packages/backend/src/connectionWorkload.ts`:
- Around line 209-220: Update the onTerminalFailure hook to report the received
error to Sentry with captureException, including jobId and the connection-sync
queue name in the tags, while preserving the existing database failure update.
- Around line 238-248: Update deduplicateRepos to use a Map keyed by the
composite external_id and external_codeHostUrl values, preserving the current
first-wins behavior while reducing deduplication to linear time. Return the
Map’s values as the resulting RepoData array.
- Around line 274-304: Update the repository persistence loop in the connection
workload to avoid awaiting each db.repo.upsert sequentially. Use the project’s
bounded-concurrency helper to process deduplicateRepos(discoveredRepos) with a
controlled number of concurrent upserts, while preserving the existing upsert
payload, selected fields, and currentRepos collection behavior.
- Around line 365-375: Bound scheduler fan-out in the current repository
indexing loop by replacing the unbounded Promise.all over currentRepos with
sequential batches of roughly 50–100 upsertJobScheduler calls, while preserving
each repo’s existing arguments. Apply the same batching approach to the
permission-sync scheduler loop, ensuring the next batch starts only after the
prior batch completes.
In `@packages/backend/src/ee/accountPermissionSyncWorkload.test.ts`:
- Around line 311-327: The success-path test “syncs the requested account” does
not verify persisted permissions. Configure
mocks.getReposForAuthenticatedBitbucketServerUser to return repositories, then
assert permissionCreateMany was called with the expected accountToRepoPermission
rows and preserve the existing client and transaction assertions.
- Around line 431-459: Add a stale-job test alongside the existing latest-job
onCompleted test: configure accountUpdateMany to resolve with { count: 0 },
invoke createWorkload().onCompleted with the existing lifecycle context, assert
the hook resolves without throwing, and verify permissionSyncJobUpdate still
updates job_1 to COMPLETED while the account issue is not cleared.
In `@packages/backend/src/ee/repoPermissionSyncWorkload.test.ts`:
- Around line 39-65: Add tests in the repo permission sync workload suite that
cover getAuthCredentialsForRepo returning a falsy value and assert the expected
“No credentials found for repo ${id}” error, plus a successful non-GitHub
provider path such as GitLab or Bitbucket Server that exercises its registered
client and permission mocks. Ensure the selected provider test drives the
corresponding symbols, including
getProjectMembers/createGitLabFromPersonalAccessToken or
getUserPermissionsForServerRepo.
In `@packages/backend/src/executionLock.ts`:
- Around line 109-122: Update isLockContention to handle rejected promises from
error.attempts without propagating the rejection: wrap the Promise.all
inspection in error handling and return false when any attempt cannot be
resolved, while preserving the existing contention checks for successfully
resolved attempts.
- Around line 156-205: Bound the contention retry loop in the lock-acquisition
method surrounding redlock. Track elapsed wait time or retry attempts, stop
retrying after a configured maximum, and throw the contention error so BullMQ
can reschedule with its retry/backoff policy; also emit the existing warning or
metric when the threshold is exceeded. Preserve immediate propagation for
acquired-lock failures and shutdown aborts.
In `@packages/backend/src/jobManager.test.ts`:
- Around line 365-397: Add tests alongside the existing terminal-failure test
for the non-terminal failed-job path and rejected lifecycle hooks. Verify a
failure with attemptsMade below opts.attempts does not invoke onTerminalFailure,
and verify a rejecting hook calls Sentry.captureException while the job logger
still flushes; use the existing BullMQJobManager, worker failed handler, and
logger mocks.
In `@packages/backend/src/jobManager.ts`:
- Around line 115-132: Update JobManager.stop so each worker is force-closed
with worker.close(true) when the graceful shutdown race times out, ensuring all
workers have stopped before bullmqClient.close() runs. Preserve the existing
graceful close and timeout behavior.
In `@packages/backend/src/reconcileJobSchedulersAtStartup.ts`:
- Around line 48-66: Limit startup scheduler upserts in the targets mapping
within reconcileJobSchedulersAtStartup by processing entries in bounded batches
(for example, 50–100) rather than issuing every upsert through one unbounded
Promise.all, while preserving completion of all targets. Simplify the
jobManager.upsertJobScheduler invocation by removing the redundant jobOptions
branch and passing the optional value consistently.
- Around line 133-172: Replace the inline scheduler prefixes and ID templates in
reconcileJobSchedulersAtStartup with shared exported prefix constants and ID
helper functions for connection-sync, repo-index, and repo-permission-sync,
matching the existing account-permission-sync pattern; update
packages/backend/src/reconcileJobSchedulersAtStartup.ts lines 133-172
accordingly. In packages/backend/src/configManager.ts lines 18-19, remove the
local getConnectionSyncSchedulerId and import the shared connection-sync helper
instead.
In `@packages/backend/src/repoIndexWorkload.ts`:
- Around line 34-56: Add a brief comment near the `prepareRepoIndexJob` call in
`process` documenting that job-state and parent `latest...JobId` updates
intentionally occur there, rather than in `onStarted`, so eligibility checks can
skip jobs without creating spurious `IN_PROGRESS` rows.
- Around line 279-289: Update the initial isPathAValidGitRepoRoot call in the
repository validation block to pass signal, then reuse its result for the
invalid-repository branch. Remove the second isPathAValidGitRepoRoot invocation
and preserve the existing isReadOnly guard, warning, and deletion behavior.
In `@packages/shared/src/jobLogger.test.ts`:
- Around line 118-130: Replace parseJobLogEntry(structuredEntry) in the expected
logs array of the relevant test with a literal object containing the structured
entry’s expected parsed fields, matching the existing legacy-entry assertion
style. Keep the production call under test unchanged and preserve the count and
legacy log expectations.
In `@packages/shared/src/utils.test.ts`:
- Around line 98-112: Extend the resolveConfigSettings tests to cover fallback
from experiment_repoDrivenPermissionSyncIntervalMs and
experiment_userDrivenPermissionSyncIntervalMs to their current keys when only
deprecated values are provided. Add cases confirming each current key takes
precedence when both current and deprecated values are configured, using the
existing DEFAULT_CONFIG_SETTINGS expectations where applicable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dc45242b-3e8d-4b4e-936e-2556180d26fb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (86)
CLAUDE.mddocs/snippets/schemas/v3/index.schema.mdxpackages/backend/package.jsonpackages/backend/src/api.tspackages/backend/src/attachmentPruneWorkload.test.tspackages/backend/src/attachmentPruneWorkload.tspackages/backend/src/attachmentPruner.tspackages/backend/src/bitbucket.tspackages/backend/src/configManager.test.tspackages/backend/src/configManager.tspackages/backend/src/connectionManager.tspackages/backend/src/connectionWorkload.test.tspackages/backend/src/connectionWorkload.tspackages/backend/src/ee/accountPermissionSyncWorkload.test.tspackages/backend/src/ee/accountPermissionSyncWorkload.tspackages/backend/src/ee/accountPermissionSyncer.test.tspackages/backend/src/ee/accountPermissionSyncer.tspackages/backend/src/ee/auditLogPruneWorkload.test.tspackages/backend/src/ee/auditLogPruneWorkload.tspackages/backend/src/ee/auditLogPruner.tspackages/backend/src/ee/permissionSyncEligibility.tspackages/backend/src/ee/repoPermissionSyncWorkload.test.tspackages/backend/src/ee/repoPermissionSyncWorkload.tspackages/backend/src/ee/repoPermissionSyncer.tspackages/backend/src/ee/syncSearchContexts.test.tspackages/backend/src/ee/syncSearchContexts.tspackages/backend/src/executionLock.test.tspackages/backend/src/executionLock.tspackages/backend/src/index.tspackages/backend/src/jobManager.test.tspackages/backend/src/jobManager.tspackages/backend/src/reconcileJobSchedulersAtStartup.test.tspackages/backend/src/reconcileJobSchedulersAtStartup.tspackages/backend/src/repoIndexManager.test.tspackages/backend/src/repoIndexManager.tspackages/backend/src/repoIndexWorkload.test.tspackages/backend/src/repoIndexWorkload.tspackages/backend/src/types.tspackages/backend/src/types/redlock.d.tspackages/backend/src/utils.tspackages/db/prisma/migrations/20260810000000_add_latest_repo_indexing_job_id/migration.sqlpackages/db/prisma/migrations/20260811000000_add_latest_account_permission_sync_job_id/migration.sqlpackages/db/prisma/migrations/20260811001000_add_latest_repo_permission_sync_job_id/migration.sqlpackages/db/prisma/migrations/20260811002000_add_latest_connection_sync_job_id/migration.sqlpackages/db/prisma/schema.prismapackages/schemas/src/v3/index.schema.tspackages/schemas/src/v3/index.type.tspackages/shared/package.jsonpackages/shared/src/bullmqClient.test.tspackages/shared/src/bullmqClient.tspackages/shared/src/constants.tspackages/shared/src/env.server.tspackages/shared/src/index.server.tspackages/shared/src/jobLogger.test.tspackages/shared/src/jobLogger.tspackages/shared/src/queue.tspackages/shared/src/redis.tspackages/shared/src/schedule.test.tspackages/shared/src/schedule.tspackages/shared/src/types.tspackages/shared/src/utils.test.tspackages/shared/src/utils.tspackages/shared/vitest.config.tspackages/web/src/app/(app)/repos/components/repoActionsDropdown.tsxpackages/web/src/app/(app)/repos/components/repoJobsTable.tsxpackages/web/src/app/(app)/repos/components/reposTable.tsxpackages/web/src/app/(app)/settings/connections/components/connectionJobsTable.tsxpackages/web/src/auth.tspackages/web/src/ee/features/permissionSync/accountPermissionSyncQueue.server.test.tspackages/web/src/ee/features/permissionSync/accountPermissionSyncQueue.server.tspackages/web/src/ee/features/sso/actions.test.tspackages/web/src/ee/features/sso/actions.tspackages/web/src/ee/features/sso/components/linkedAccountProviderCard.test.tsxpackages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsxpackages/web/src/features/connections/actions.test.tspackages/web/src/features/connections/actions.tspackages/web/src/features/repos/actions.test.tspackages/web/src/features/repos/actions.tspackages/web/src/features/workerApi/actions.tspackages/web/src/features/workerApi/client.server.test.tspackages/web/src/features/workerApi/client.server.tspackages/web/src/lib/bullmqClient.tspackages/web/src/lib/encryptedPrismaAdapter.test.tspackages/web/src/lib/encryptedPrismaAdapter.tspackages/web/src/lib/redis.tsschemas/v3/index.json
💤 Files with no reviewable changes (12)
- packages/web/src/features/workerApi/client.server.test.ts
- packages/backend/src/repoIndexManager.test.ts
- packages/backend/src/ee/auditLogPruner.ts
- packages/backend/src/attachmentPruner.ts
- packages/shared/src/types.ts
- packages/web/src/features/workerApi/client.server.ts
- packages/backend/src/ee/repoPermissionSyncer.ts
- packages/shared/src/env.server.ts
- packages/backend/src/ee/accountPermissionSyncer.ts
- packages/backend/src/connectionManager.ts
- packages/backend/src/ee/accountPermissionSyncer.test.ts
- packages/backend/src/repoIndexManager.ts
| const bullBoardAdapter = new ExpressAdapter(); | ||
| bullBoardAdapter.setBasePath('/admin/queues'); | ||
| createBullBoard({ | ||
| queues: jobManager.getQueues().map(queue => new BullMQAdapter(queue, { readOnlyMode: true })), | ||
| serverAdapter: bullBoardAdapter, | ||
| }); | ||
| app.use('/admin/queues', bullBoardAdapter.getRouter()); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the worker API port is exposed in deployment configs.
set -euo pipefail
# Find the worker API URL/port default and any published port mappings.
rg -n 'WORKER_API_URL' --glob '!**/node_modules/**' -C 2
fd -H -i -e yml -e yaml -e conf 'docker-compose|Caddyfile|nginx' --exec rg -n -C 3 'ports|proxy|upstream' {} \;
fd -H 'Dockerfile*' --exec rg -n 'EXPOSE' {} \;Repository: sourcebot-dev/sourcebot
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(api\.ts|package\.json|Dockerfile[^/]*|docker-compose[^/]*\.(yml|yaml)|.*\.(yml|yaml)|Caddyfile|nginx[^/]*)$' | head -200
printf '%s\n' '--- api.ts outline and relevant source ---'
wc -l packages/backend/src/api.ts 2>/dev/null || true
ast-grep outline packages/backend/src/api.ts 2>/dev/null || true
sed -n '1,140p' packages/backend/src/api.ts 2>/dev/null || true
printf '%s\n' '--- backend references ---'
rg -n -C 3 'WORKER_API_URL|worker.*(api|port)|API_PORT|PORT|admin/queues|metrics|helmet|auth|listen\(' packages README.md .github 2>/dev/null | head -400
printf '%s\n' '--- deployment exposure declarations ---'
rg -n -C 3 'ports:|EXPOSE|host:|listen:|targetPort:|containerPort:|proxy_pass|upstream|WORKER_API_URL|admin/queues' --glob '*.yml' --glob '*.yaml' --glob 'Dockerfile*' --glob 'Caddyfile' --glob 'nginx*' --glob '*.json' . 2>/dev/null | head -500Repository: sourcebot-dev/sourcebot
Length of output: 39719
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- WORKER_API_URL declarations and consumers ---'
rg -n -C 5 'WORKER_API_URL' . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 2>/dev/null | head -300
printf '%s\n' '--- startup and container routing ---'
for f in entrypoint.sh Dockerfile docker-compose.yml docker-compose-dev.yml; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
sed -n '1,280p' "$f"
fi
done
rg -n -C 4 'backend|worker|api|3000|3001|next start|node .*backend|tsx|WORKER' Dockerfile docker-compose.yml docker-compose-dev.yml packages/*/package.json package.json 2>/dev/null | head -500
printf '%s\n' '--- queue payload definitions and sensitive identifiers ---'
rg -n -C 4 'Queue|queue|Job|payload|connectionId|repoId|accountId|connection_id|repository_id' packages/backend/src packages/shared/src packages/db/src 2>/dev/null | head -600
printf '%s\n' '--- helmet dependency and middleware ---'
rg -n -C 3 '"helmet"|from ['"'"'"]helmet|app\.use\(' package.json packages/*/package.json yarn.lock packages 2>/dev/null | head -300
printf '%s\n' '--- read-only deployment verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
compose = Path("docker-compose.yml").read_text()
dockerfile = Path("Dockerfile").read_text()
published = re.findall(r'^\s*-\s*["\']?([^"\']*3000:3000[^"\']*)["\']?\s*$', compose, re.M)
print("compose_3000_mappings:", published)
print("dockerfile_exposes_3000:", bool(re.search(r'^\s*EXPOSE\s+3000\s*$', dockerfile, re.M)))
print("dockerfile_binds_all_interfaces_hint:", 'HOSTNAME="0.0.0.0"' in dockerfile)
print("loopback_only_3000_mapping:", any(x.strip().startswith(("127.0.0.1:", "localhost:")) for x in published))
PYRepository: sourcebot-dev/sourcebot
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- supervisor process configuration ---'
if [ -f supervisord.conf ]; then
cat -n supervisord.conf
fi
fd -H -t f 'supervisord.conf|supervisor.*\.conf' . --exec sh -c 'echo "--- $1 ---"; cat -n "$1"' sh {} \;
printf '%s\n' '--- backend startup commands ---'
rg -n -C 5 'new Api|Api\(|WORKER_API_URL|3060|supervisord|next start|packages/backend/dist|backend.*start' packages Dockerfile supervisord.conf entrypoint.sh package.json 2>/dev/null | head -250
printf '%s\n' '--- queue registry and payload declarations ---'
rg -n -C 5 'QUEUE|QueueName|DataOf|repo-index|connection-sync|account.*permission|permission-sync|interface .*Job|type .*Job' packages/shared/src packages/backend/src/types.ts packages/backend/src/jobManager.ts packages/backend/src 2>/dev/null | head -350
printf '%s\n' '--- exact identifier-bearing job triggers ---'
rg -n -C 3 'jobManager\.(trigger|upsertJobScheduler)|trigger\(' packages/backend/src --glob '*.ts' 2>/dev/null | rg -n -C 2 'connectionId|repoId|accountId|permission|index|sync' | head -250
printf '%s\n' '--- helmet availability ---'
for f in packages/backend/package.json package.json packages/web/package.json; do
[ -f "$f" ] && { echo "--- $f ---"; rg -n '"helmet"|express' "$f"; }
done
rg -n '(^|/)(helmet|`@types/helmet`)@|helmet@' yarn.lock 2>/dev/null | head -30
printf '%s\n' '--- read-only port verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
env = Path("packages/shared/src/env.server.ts").read_text()
compose = Path("docker-compose.yml").read_text()
api = Path("packages/backend/src/api.ts").read_text()
default = re.search(r'WORKER_API_URL:\s*z\.string\(\)\.url\(\)\.default\("([^"]+)"\)', env)
api_port = re.search(r'Number\(workerApiUrl\.port\).*?protocol === "https:" \? 443 : 80', api, re.S)
published = re.findall(r'^\s*-\s*["\']?([^"\']+)["\']?\s*$', compose, re.M)
print("worker_api_default:", default.group(1) if default else None)
print("compose_published_ports:", [x for x in published if "3000:3000" in x or "3060:3060" in x])
print("worker_api_port_is_3060_by_default:", default is not None and ":3060" in default.group(1))
print("worker_api_route_mounted_without_auth_in_source:",
"app.use('/admin/queues', bullBoardAdapter.getRouter());" in api and
"app.use('/metrics'," in api)
PYRepository: sourcebot-dev/sourcebot
Length of output: 42800
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- shared queue registry ---'
if [ -f packages/shared/src/queue.ts ]; then
cat -n packages/shared/src/queue.ts
else
fd -H -t f 'queue.ts' packages/shared --exec sh -c 'echo "--- $1 ---"; cat -n "$1"' sh {} \;
fi
printf '%s\n' '--- account permission workload payload use ---'
fd -H -t f '*account*permission*' packages/backend/src --exec sh -c '
echo "--- $1 ---"
rg -n -C 4 "accountId|queueSpec|data:|trigger|upsertJobScheduler" "$1"
' sh {} \;
printf '%s\n' '--- Bull Board adapter configuration ---'
rg -n -C 5 'BullMQAdapter|readOnlyMode|createBullBoard|job\.data|stacktrace|failedReason' packages/backend/src packages/backend/package.json yarn.lock 2>/dev/null | head -250Repository: sourcebot-dev/sourcebot
Length of output: 8528
Protect Bull Board when the worker API is externally reachable.
The default deployment publishes only port 3000; WORKER_API_URL defaults the backend to localhost:3060. If a deployment publishes or proxies port 3060, add basic authentication or shared-secret middleware before mounting /admin/queues. The read-only dashboard still exposes job payload IDs and failure details. Add helmet() as defense-in-depth for the HTML dashboard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backend/src/api.ts` around lines 35 - 41, Protect the Bull Board
route in the API setup around createBullBoard and app.use('/admin/queues', ...):
add shared-secret or basic-auth middleware before mounting the router, and apply
helmet() for the dashboard response. Ensure both protections remain in effect
whenever the backend is externally reachable, while preserving the existing
read-only BullMQAdapter configuration.
Source: Linters/SAST tools
| process: async ({ logger }) => { | ||
| if (ttlHours <= 0) { | ||
| logger.debug("Attachment orphan pruning is disabled."); | ||
| return { | ||
| pendingClaimed: 0, | ||
| committedClaimed: 0, | ||
| reclaimed: 0, | ||
| }; | ||
| } | ||
|
|
||
| const cutoff = new Date(Date.now() - ttlHours * ONE_HOUR_MS); | ||
|
|
||
| // Each claim is atomic, so a PENDING blob committed by a concurrent | ||
| // send or a zero-link blob re-linked by a concurrent duplicate-chat | ||
| // loses the claim and is left intact. | ||
| const pendingClaimed = await db.attachment.updateMany({ | ||
| where: { | ||
| status: AttachmentStatus.PENDING, | ||
| createdAt: { lt: cutoff }, | ||
| }, | ||
| data: { status: AttachmentStatus.DELETING }, | ||
| }); | ||
|
|
||
| const committedClaimed = await db.attachment.updateMany({ | ||
| where: { | ||
| status: AttachmentStatus.COMMITTED, | ||
| createdAt: { lt: cutoff }, | ||
| chats: { none: {} }, | ||
| }, | ||
| data: { status: AttachmentStatus.DELETING }, | ||
| }); | ||
|
|
||
| const reclaimed = await reclaimTombstonedAttachments({ | ||
| db, | ||
| storage, | ||
| warn: (message) => logger.warn(message), | ||
| }); | ||
|
|
||
| if ( | ||
| pendingClaimed.count > 0 || | ||
| committedClaimed.count > 0 || | ||
| reclaimed > 0 | ||
| ) { | ||
| logger.debug( | ||
| `Attachment prune: condemned ${pendingClaimed.count} PENDING + ` + | ||
| `${committedClaimed.count} COMMITTED orphan(s), reclaimed ${reclaimed} tombstone(s).`, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| pendingClaimed: pendingClaimed.count, | ||
| committedClaimed: committedClaimed.count, | ||
| reclaimed, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Scheduled prune workloads ignore the abort signal. Both pruning workloads destructure only logger from the process context and then run unbounded batch loops that delete storage blobs and database rows. Neither loop stops when the worker shuts down or the execution lock is lost.
packages/backend/src/attachmentPruneWorkload.ts#L60-L114: destructuresignal, pass it intoreclaimTombstonedAttachments, and callsignal.throwIfAborted()before each claim and each batch delete.packages/backend/src/ee/auditLogPruneWorkload.ts#L37-L70: destructuresignaland callsignal.throwIfAborted()at the start of eachwhileiteration.
As per path instructions for packages/backend/**/*.{ts,tsx}: "Call signal.throwIfAborted() before side effects and after long-running or external operations so workloads stop promptly after losing their lock."
📍 Affects 2 files
packages/backend/src/attachmentPruneWorkload.ts#L60-L114(this comment)packages/backend/src/ee/auditLogPruneWorkload.ts#L37-L70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backend/src/attachmentPruneWorkload.ts` around lines 60 - 114,
Update process in packages/backend/src/attachmentPruneWorkload.ts (lines 60-114)
to destructure signal, call signal.throwIfAborted() before each attachment claim
and batch delete, and pass signal to reclaimTombstonedAttachments; update the
audit-log prune loop in packages/backend/src/ee/auditLogPruneWorkload.ts (lines
37-70) to destructure signal and call signal.throwIfAborted() at the start of
every while iteration.
Source: Path instructions
| expect(mocks.upsertJobScheduler).not.toHaveBeenCalled(); | ||
| expect(mocks.trigger).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This assertion locks in the scheduler gap.
expect(mocks.upsertJobScheduler).not.toHaveBeenCalled() asserts that an existing connection never gets its scheduler refreshed. See the comment on packages/backend/src/configManager.ts lines 116-124 for the root cause. If you make the upsert unconditional, change this assertion to verify the scheduler is upserted with the current interval while trigger stays uncalled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backend/src/configManager.test.ts` around lines 167 - 168, Update
the test around mocks.upsertJobScheduler to expect it is called with the current
interval when an existing connection is processed, reflecting the unconditional
scheduler upsert in configManager; retain the assertion that mocks.trigger is
not called.
| if (!existingConnection) { | ||
| await this.jobManager.upsertJobScheduler( | ||
| "connection-sync", | ||
| getConnectionSyncSchedulerId(connection.id), | ||
| intervalMs, | ||
| { connectionId: connection.id }, | ||
| { priority: JOB_PRIORITIES.SCHEDULED }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Existing connections never get their sync scheduler refreshed. ConfigManager.syncConnections calls upsertJobScheduler only inside if (!existingConnection). A change to resyncConnectionIntervalMs in the config file therefore does not reach existing connections until the process restarts, and a scheduler that is missing in Redis is never restored by a config-change sync.
packages/backend/src/configManager.ts#L116-L124: remove theif (!existingConnection)guard and callupsertJobSchedulerfor every declarative connection. The operation is idempotent.packages/backend/src/configManager.test.ts#L167-L168: change the assertion so it verifies the scheduler is upserted with the current interval for an unchanged connection, whiletriggerremains uncalled.
📍 Affects 2 files
packages/backend/src/configManager.ts#L116-L124(this comment)packages/backend/src/configManager.test.ts#L167-L168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backend/src/configManager.ts` around lines 116 - 124, Update
ConfigManager.syncConnections in packages/backend/src/configManager.ts:116-124
to call upsertJobScheduler for every declarative connection, removing the
existingConnection guard. Update
packages/backend/src/configManager.test.ts:167-168 to assert the unchanged
connection’s scheduler is upserted with the current interval while trigger
remains uncalled.
| const discoverConnectionRepositories = async ({ | ||
| config, | ||
| connectionId, | ||
| signal, | ||
| }: { | ||
| config: ConnectionConfig; | ||
| connectionId: number; | ||
| signal: AbortSignal; | ||
| }) => { | ||
| switch (config.type) { | ||
| case "github": { | ||
| return compileGithubConfig(config, connectionId, signal); | ||
| } | ||
| case "gitlab": { | ||
| return compileGitlabConfig(config, connectionId); | ||
| } | ||
| case "gitea": { | ||
| return compileGiteaConfig(config, connectionId); | ||
| } | ||
| case "gerrit": { | ||
| return compileGerritConfig(config, connectionId); | ||
| } | ||
| case "bitbucket": { | ||
| return compileBitbucketConfig(config, connectionId); | ||
| } | ||
| case "azuredevops": { | ||
| return compileAzureDevOpsConfig(config, connectionId); | ||
| } | ||
| case "git": { | ||
| return compileGenericGitHostConfig(config, connectionId); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add an exhaustive default branch.
connection.config is cast with as unknown as ConnectionConfig at line 73, so the runtime value is not verified against the union. If a stored connection carries an unhandled type, the switch falls through and returns undefined. Line 72 then destructures { repoData, warnings } from undefined and the job fails with a TypeError instead of a clear message.
🛡️ Proposed fix
case "git": {
return compileGenericGitHostConfig(config, connectionId);
}
+ default: {
+ const exhaustiveCheck: never = config;
+ throw new Error(
+ `Unsupported connection type for connection ${connectionId}: ${
+ (exhaustiveCheck as { type?: string }).type
+ }`,
+ );
+ }
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const discoverConnectionRepositories = async ({ | |
| config, | |
| connectionId, | |
| signal, | |
| }: { | |
| config: ConnectionConfig; | |
| connectionId: number; | |
| signal: AbortSignal; | |
| }) => { | |
| switch (config.type) { | |
| case "github": { | |
| return compileGithubConfig(config, connectionId, signal); | |
| } | |
| case "gitlab": { | |
| return compileGitlabConfig(config, connectionId); | |
| } | |
| case "gitea": { | |
| return compileGiteaConfig(config, connectionId); | |
| } | |
| case "gerrit": { | |
| return compileGerritConfig(config, connectionId); | |
| } | |
| case "bitbucket": { | |
| return compileBitbucketConfig(config, connectionId); | |
| } | |
| case "azuredevops": { | |
| return compileAzureDevOpsConfig(config, connectionId); | |
| } | |
| case "git": { | |
| return compileGenericGitHostConfig(config, connectionId); | |
| } | |
| } | |
| }; | |
| const discoverConnectionRepositories = async ({ | |
| config, | |
| connectionId, | |
| signal, | |
| }: { | |
| config: ConnectionConfig; | |
| connectionId: number; | |
| signal: AbortSignal; | |
| }) => { | |
| switch (config.type) { | |
| case "github": { | |
| return compileGithubConfig(config, connectionId, signal); | |
| } | |
| case "gitlab": { | |
| return compileGitlabConfig(config, connectionId); | |
| } | |
| case "gitea": { | |
| return compileGiteaConfig(config, connectionId); | |
| } | |
| case "gerrit": { | |
| return compileGerritConfig(config, connectionId); | |
| } | |
| case "bitbucket": { | |
| return compileBitbucketConfig(config, connectionId); | |
| } | |
| case "azuredevops": { | |
| return compileAzureDevOpsConfig(config, connectionId); | |
| } | |
| case "git": { | |
| return compileGenericGitHostConfig(config, connectionId); | |
| } | |
| default: { | |
| const exhaustiveCheck: never = config; | |
| throw new Error( | |
| `Unsupported connection type for connection ${connectionId}: ${ | |
| (exhaustiveCheck as { type?: string }).type | |
| }`, | |
| ); | |
| } | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backend/src/connectionWorkload.ts` around lines 492 - 524, Add an
exhaustive default branch to discoverConnectionRepositories that throws a clear
error containing the unexpected config.type, ensuring unhandled runtime
connection types cannot fall through and return undefined.
| expect(mocks.upsertJobScheduler).not.toHaveBeenCalledWith( | ||
| expect.stringContaining("permission-sync"), | ||
| expect.anything(), | ||
| expect.anything(), | ||
| expect.anything(), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This negative assertion always passes.
reconcileJobSchedulersAtStartup always passes five arguments to upsertJobScheduler because jobOptions is set for every workload. This assertion lists four arguments, so it can never match a real call. The test would still pass if permission schedulers were upserted.
Assert on the recorded call arguments instead.
💚 Proposed fix
- expect(mocks.upsertJobScheduler).not.toHaveBeenCalledWith(
- expect.stringContaining("permission-sync"),
- expect.anything(),
- expect.anything(),
- expect.anything(),
- );
+ const upsertedWorkloads = mocks.upsertJobScheduler.mock.calls.map(
+ ([workloadName]) => workloadName,
+ );
+ expect(upsertedWorkloads).not.toContain("account-permission-sync");
+ expect(upsertedWorkloads).not.toContain("repo-permission-sync");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(mocks.upsertJobScheduler).not.toHaveBeenCalledWith( | |
| expect.stringContaining("permission-sync"), | |
| expect.anything(), | |
| expect.anything(), | |
| expect.anything(), | |
| ); | |
| const upsertedWorkloads = mocks.upsertJobScheduler.mock.calls.map( | |
| ([workloadName]) => workloadName, | |
| ); | |
| expect(upsertedWorkloads).not.toContain("account-permission-sync"); | |
| expect(upsertedWorkloads).not.toContain("repo-permission-sync"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backend/src/reconcileJobSchedulersAtStartup.test.ts` around lines
159 - 164, Fix the negative assertion in the reconcileJobSchedulersAtStartup
test so it inspects recorded upsertJobScheduler calls with the actual
five-argument signature, including jobOptions. Ensure the assertion specifically
verifies that no call contains the permission-sync scheduler identifier, rather
than relying on a mismatched four-argument toHaveBeenCalledWith pattern.
| "minimum": 1, | ||
| "deprecated": true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mark reindexRepoPollingIntervalMs as deprecated in both schemas.
This migration removes interval-based repository indexing polling. The reindexRepoPollingIntervalMs entries at Lines 37-40 and 223-226 still describe that removed poller without deprecation metadata. Configuration tooling can therefore accept the obsolete setting without warning.
Proposed fix
"reindexRepoPollingIntervalMs": {
"type": "number",
"description": "The polling rate (in milliseconds) at which the db should be checked for repos that should be re-indexed. Defaults to 1 second.",
- "minimum": 1
+ "minimum": 1,
+ "deprecated": true
},Apply the same change to both schema definitions.
Also applies to: 220-221
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/schemas/src/v3/index.schema.ts` around lines 34 - 35, Mark the
reindexRepoPollingIntervalMs property as deprecated in both schema definitions,
matching the existing deprecated metadata on the nearby interval setting. Update
both occurrences while preserving their current validation and descriptions.
| */ | ||
| resyncConnectionIntervalMs?: number; | ||
| /** | ||
| * @deprecated |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 \
'resyncConnectionPollingIntervalMs|reindexRepoPollingIntervalMs|resyncConnectionIntervalMs|reindexIntervalMs' \
--glob '*.{ts,tsx,js,jsx,json,md,mdx}' .Repository: sourcebot-dev/sourcebot
Length of output: 27403
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- focused diff ---'
git diff -- packages/schemas/src/v3/index.type.ts packages/schemas/src/v3/index.schema.ts schemas/v3/index.json
printf '%s\n' '--- type and schema context ---'
sed -n '85,125p' packages/schemas/src/v3/index.type.ts
sed -n '1,55p' packages/schemas/src/v3/index.schema.ts
printf '%s\n' '--- polling-setting references, including non-code files ---'
rg -n -C 2 \
'resyncConnectionPollingIntervalMs|reindexRepoPollingIntervalMs' \
.
printf '%s\n' '--- replacement-setting references ---'
rg -n -C 2 \
'reindexIntervalMs|resyncConnectionIntervalMs|maxConnectionSyncJobConcurrency|maxRepoIndexingJobConcurrency|maxRepoGarbageCollectionJobConcurrency' \
packages docs schemas CHANGELOG.mdRepository: sourcebot-dev/sourcebot
Length of output: 38450
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import subprocess
tracked = subprocess.check_output(["git", "ls-files", "-z"], text=False).decode().split("\0")
polling = {
"resyncConnectionPollingIntervalMs",
"reindexRepoPollingIntervalMs",
}
runtime_roots = ("packages/",)
excluded_parts = ("/schemas/", "/docs/", "/test", ".test.", "constants.ts")
for identifier in sorted(polling):
print(f"--- {identifier} ---")
for name in tracked:
if not name.startswith(runtime_roots):
continue
path = Path(name)
if not path.is_file() or any(part in name for part in excluded_parts):
continue
for number, line in enumerate(path.read_text(errors="replace").splitlines(), 1):
if identifier in line:
print(f"{name}:{number}:{line.strip()}")
type_path = Path("packages/schemas/src/v3/index.type.ts")
lines = type_path.read_text().splitlines()
for identifier in sorted(polling):
for number, line in enumerate(lines, 1):
if identifier in line:
preceding = lines[max(0, number - 4):number - 1]
print(f"JSDOC {identifier} at line {number}:")
print("\n".join(f" {item}" for item in preceding))
print("--- replacement consumers ---")
for identifier in ("resyncConnectionIntervalMs", "reindexIntervalMs"):
count = 0
for name in tracked:
if not name.startswith(runtime_roots):
continue
path = Path(name)
if not path.is_file() or any(part in name for part in excluded_parts):
continue
for number, line in enumerate(path.read_text(errors="replace").splitlines(), 1):
if identifier in line:
print(f"{name}:{number}:{line.strip()}")
count += 1
print(f"{identifier}: {count} runtime references")
PYRepository: sourcebot-dev/sourcebot
Length of output: 1614
Deprecate reindexRepoPollingIntervalMs and document both replacements.
Add @deprecated with Use reindexIntervalMs instead. Add Use resyncConnectionIntervalMs instead to the existing deprecation notice. Regenerate the schema artifacts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/schemas/src/v3/index.type.ts` at line 104, Update the deprecation
JSDoc for reindexRepoPollingIntervalMs to document both replacements:
reindexIntervalMs and resyncConnectionIntervalMs. Preserve the existing
deprecation marker, then regenerate the schema artifacts so the generated
outputs reflect the updated documentation.
| "minimum": 1, | ||
| "deprecated": true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mark reindexRepoPollingIntervalMs as deprecated.
The schema still treats this repository-indexing polling setting as active. The PR replaces that database poller with workloads, so this property needs the same "deprecated": true metadata as resyncConnectionPollingIntervalMs.
This follows the PR objective to replace interval-based repository-indexing pollers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/v3/index.json` around lines 33 - 34, Mark the
reindexRepoPollingIntervalMs schema property as deprecated by adding the same
deprecated metadata already used for resyncConnectionPollingIntervalMs, while
preserving its existing minimum constraint and other definition fields.
| "minimum": 1, | ||
| "deprecated": true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Synchronize the documented default with the runtime default.
packages/shared/src/constants.ts:28 now sets maxRepoGarbageCollectionJobConcurrency to 2, but this schema description still says Defaults to 8. Update the description to 2, or keep the runtime default at 8.
The runtime default is defined in packages/shared/src/constants.ts:28.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/v3/index.json` around lines 54 - 55, Synchronize the schema
description for maxRepoGarbageCollectionJobConcurrency with the runtime default
defined by maxRepoGarbageCollectionJobConcurrency in constants.ts: update the
documented default from 8 to 2, unless intentionally reverting the runtime
constant to 8.
1 participant
Note
High Risk
Large refactor of core worker orchestration (indexing, connection sync, permission sync, cleanup) with new locking and latest-job lifecycle semantics; mis-wiring could cause stuck jobs, duplicate work, or incorrect permission state.
Overview
Introduces a central
JobManager+Workloadmodel (shared queue registry in@sourcebot/shared, BullMQ workers, Redlock-style execution locks, and DB lifecycle hooks) and moves most background work off ad-hoc managers and DB polling onto BullMQ queues and job schedulers.Connection sync is now a
connection-syncworkload: discover repos, upsert links, reconcile per-repo index and permission-sync schedulers, trigger immediate index/cleanup jobs, and updatelatestSyncJobId/ job rows with lock-keyed connection resources. Config sync registers connection schedulers on create, triggers interactive sync on config changes, and removes schedulers when declarative connections are deleted.Permission syncing is split into
account-permission-syncandrepo-permission-syncworkloads (replacingAccountPermissionSyncerand repo-driven syncers), with per-account/per-repo locks, fail-closed permission cleanup on classified OAuth/upstream errors, and conditional parent updates vialatest…JobId. Housekeeping (attachment-prune,audit-log-prune) becomes scheduled workloads instead ofsetIntervalpruners.The worker API drops the old manual trigger routes for connection/index/account permission sync; it mounts read-only Bull Board at
/admin/queuesand usesjobManager.triggerfor the experimental GitHub repo path. Docs/schema mark legacy connection polling and repo GC concurrency settings as deprecated. CLAUDE.md documents workload execution locks and lifecycle rules.Reviewed by Cursor Bugbot for commit 65ec093. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
/admin/queues.Improvements