refactor(router): split worker-manager god module into focused modules

[aaight]

Summary

Refactors the god module src/router/worker-manager.ts (418 lines, 5 mixed responsibilities) into three focused modules:

• src/router/container-manager.ts — Docker container lifecycle: spawnWorker, killWorker, cleanupWorker, buildWorkerEnv, extractProjectIdFromJob, detachAll, plus the activeWorkers Map and ActiveWorker interface
• src/router/bullmq-workers.ts — BullMQ consumer factory: createQueueWorker de-duplicates the identical event handler boilerplate that was copy-pasted for both queues, plus parseRedisConnection
• src/router/worker-manager.ts — Thin orchestrator (~90 lines): wires the two modules together, re-exports the public API for backward compatibility

Key duplication eliminated: The two near-identical BullMQ Worker setups (lines 300–344 and 348–385) that shared the same capacity check, concurrency/lockDuration config, and completed/failed/error event handlers are now a single createQueueWorker() factory call.

Zero API changes: getActiveWorkerCount, getActiveWorkers, startWorkerProcessor, and stopWorkerProcessor are still importable from worker-manager.ts (via re-exports).

Test plan

• tests/unit/router/bullmq-workers.test.ts — 10 tests covering parseRedisConnection and createQueueWorker factory (event handler dispatch, Sentry capture, label usage)
• tests/unit/router/container-manager.test.ts — 21 tests covering extractProjectIdFromJob, buildWorkerEnv, spawnWorker, killWorker, cleanupWorker, detachAll
• tests/unit/router/worker-manager.test.ts — 12 tests covering re-exports, startWorkerProcessor, and stopWorkerProcessor orchestration
• Full test suite: 2885 tests passing, zero regressions
• TypeScript: npm run typecheck — zero errors
• Lint: npm run lint — zero errors

Trello card: https://trello.com/c/JeJc7qN6/96-find-top-candidate-for-refactoring-and-plan-clean-refactoring-of-it-look-for-god-classes-modules-functions-files-and-code-duplic

🤖 Generated with Claude Code

[aaight]

🤔 Just a sec, looking into that router refactor now

[aaight]

Thanks for taking a look! The refactor is complete and all tests are passing. Let me know if you have any questions or feedback. 👍

[aaight]

🔍 Reviewing code — Examining the PR changes for quality and correctness...

[nhopeatall]

Summary

Clean, well-executed refactoring that splits a 418-line god module into three focused modules with clear responsibilities. The decomposition is logical, backward compatibility is preserved via re-exports, and test coverage is thorough (43 new tests). Approving.

Code Issues

Should Fix

• src/router/bullmq-workers.ts:25 — parseRedisConnection is a third copy of the same Redis URL parsing logic that already exists as parseRedisUrl in both src/router/queue.ts:8 and src/queue/client.ts:44 (identical implementations). Consider extracting a shared utility or re-exporting from one of the existing locations. This could be a follow-up.

Notes

• The new parseRedisConnection includes password: parsed.password || undefined, which is actually a fix — the original worker-manager.ts inline construction dropped the password field entirely. This means Redis URLs with authentication now work correctly for BullMQ workers.
• Backward compatibility verified: the sole external consumer (src/router/index.ts) imports getActiveWorkerCount, getActiveWorkers, startWorkerProcessor, stopWorkerProcessor — all still exported from worker-manager.ts.
• CI is green (lint, typecheck, 2885 tests passing).

[nhopeatall]

This is the third copy of the same Redis URL → BullMQ ConnectionOptions parser. Identical implementations already exist at:

• src/router/queue.ts:8 (parseRedisUrl)
• src/queue/client.ts:44 (parseRedisUrl)

Consider extracting a shared utility (e.g. src/router/redis-utils.ts or similar) and having all three call sites import from it. Fine as a follow-up.

[aaight]

Addressed — extracted parseRedisUrl into a shared src/utils/redis.ts utility and updated all three call sites to import from it:

• src/router/queue.ts — removed inline parseRedisUrl, now imports from utils/redis.ts
• src/queue/client.ts — removed inline parseRedisUrl, now imports from utils/redis.ts
• src/router/bullmq-workers.ts — removed parseRedisConnection, re-exports parseRedisUrl from the shared utility for backward compatibility

All 2885 tests pass, typecheck and lint clean.

[zbigniewsobiecki]

@aaight address code review concerns

[aaight]

Request Addressed

What was asked

Address code review concerns — specifically the duplicate parseRedisUrl implementations flagged in the review.

Changes Made

• Created src/utils/redis.ts — shared parseRedisUrl utility that all three call sites now import from
• Updated src/router/bullmq-workers.ts — removed inline parseRedisConnection, re-exports from shared utility
• Updated src/router/queue.ts — removed inline parseRedisUrl, imports from shared utility
• Updated src/queue/client.ts — removed inline parseRedisUrl, imports from shared utility
• Updated tests — renamed parseRedisConnection references to parseRedisUrl in test files

Verification

• All 2885 tests passing, zero regressions
• TypeScript: tsc --noEmit — zero errors
• Lint: biome check — zero errors
• Pre-commit and pre-push hooks passing

[aaight]

🔍 Reviewing code — Examining the PR changes for quality and correctness...

[nhopeatall]

LGTM — Clean decomposition of a god module into three focused modules with proper separation of concerns.

What's good:

• Module split aligns well with single-responsibility: container lifecycle (container-manager.ts), BullMQ boilerplate (bullmq-workers.ts), and orchestration (worker-manager.ts)
• Backward compatibility maintained — worker-manager.ts re-exports the public API, so src/router/index.ts and all other consumers are unaffected
• parseRedisUrl deduplication eliminates 3 copies across queue.ts, client.ts, and worker-manager.ts into a single utils/redis.ts
• Bonus bug fix: the old worker-manager.ts inline Redis connection (new URL(...).hostname/port) silently dropped the password field, while queue.ts and client.ts both used the full parseRedisUrl with password. Now all three consumers use the same function
• createQueueWorker factory cleanly eliminates the copy-pasted event handler boilerplate between the two queue workers
• Test coverage is thorough — each module tested in isolation with appropriate mocking boundaries, and the orchestrator tests verify delegation to the sub-modules

No issues found.