refactor: remove server mode, router mode is now the only runtime path#644
Conversation
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean, well-organized removal of ~400 lines of dead server-mode code. One file was missed in the cleanup: src/webhook/webhookTypes.ts still contains the now-unused checkCapacity and fireAndForget properties and their JSDoc references. Everything else looks good — no dangling imports, CI passes, tests properly updated.
Code Issues
Should Fix
-
src/webhook/webhookTypes.ts:64-82 — The
WebhookHandlerConfiginterface still declarescheckCapacity?: booleanandfireAndForget?: booleanwith their associated JSDoc, even thoughwebhookHandlers.tsno longer destructures or uses these properties. TheprocessWebhookJSDoc (lines 44-56) also still describesfireAndForgetbranching logic that was removed in this PR.Fix: Delete lines 64-82 (the two stale optional properties and their JSDoc) and simplify the
processWebhookJSDoc to reflect router-only (always-await) semantics. Something like:/** * Processing callback. The handler awaits this callback before responding, * so 200 means "job queued." Errors propagate to Hono's error handler (500). * * May optionally return `WebhookLogOverrides` to enrich the webhook log entry. */
Removes the unused `fireAndForget` and `checkCapacity` properties from `WebhookHandlerConfig` interface along with their associated JSDoc. Also simplifies the `processWebhook` JSDoc to reflect router-only (always-await) semantics. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
nhopeatall
left a comment
There was a problem hiding this comment.
LGTM — Clean removal of the server (single-process) execution mode. The refactoring is thorough and consistent:
- All in-process queuing logic (
webhookQueue,activeCards,isCurrentlyProcessing/setProcessing) is fully removed from source, tests, and barrel exports — no stale references remain. - The
WebhookHandlerConfiginterface is properly simplified:fireAndForgetandcheckCapacityoptions removed, leaving only the router-mode await path. - Error handling in
createWebhookHandlercorrectly logs to Sentry viahandleProcessingErrorthen re-throws to Hono's error handler — preserving the existing router behavior. - The
src/server/→src/webhook/rename cleanly separates shared webhook parsing/logging from the old monolithic server module. - All 3921 unit tests pass, TypeScript compiles with zero errors, and CI is green.
- Documentation (CLAUDE.md, README.md, .env.example) and infrastructure scripts (.cascade/setup.sh, ensure-services.sh) are updated for the Redis/BullMQ dependency.
Summary
Removes the
server(single-process/fire-and-forget) execution mode, leaving only router mode as the sole runtime path. This eliminates ~400 lines of dead code and in-process queuing logic that was superseded by the BullMQ worker architecture.Card: https://trello.com/c/69abfd306dcb3c2a6207feb8
What was changed
src/index.ts+src/server.ts(monolithic entry point, replaced bysrc/router/index.ts)src/server/webhookReactionSender.ts(server-mode-only reaction sender)src/utils/webhookQueue.ts(in-process webhook queue, replaced by BullMQ/Redis)src/utils/activeCards.ts(in-process active card tracking, no longer needed)src/triggers/shared/webhook-queue.ts(in-process queue processor)src/webhook/module with relocated webhook handler factory, parsers, logging helpers, and types fromsrc/server/src/webhook/webhookHandlers.ts— router-only factory: always awaitsprocessWebhook, errors propagate to Hono's error handlersrc/pm/webhook-handler.ts— removedisCurrentlyProcessing,enqueueWebhook,setCardActive/clearCardActive,processNextQueuedandsetProcessingcallssrc/triggers/github/webhook-handler.ts— same simplifications for GitHub webhook handlersrc/utils/lifecycle.ts— removedsetProcessing/isCurrentlyProcessingexportssrc/utils/index.ts— removed re-exports for deleted utilspackage.json—main,dev,startscripts now point tosrc/router/index.ts.cascade/setup.sh,.cascade/ensure-services.sh,.cascade/env— added Redis setup and health check (required for router mode / BullMQ).env.example— addedREDIS_URLdocumentationCLAUDE.md,README.md— new three-service architecture, Redis prerequisite, updated directory structureTest changes
tests/unit/server.test.ts(server mode integration tests)tests/unit/utils/webhookQueue.test.ts(in-process queue tests)tests/unit/utils/activeCards.test.ts(active cards tests)tests/unit/triggers/webhook-queue.test.ts(webhook-queue processor tests)tests/unit/server/(server-mode unit tests)tests/unit/webhook/webhookHandlers.test.ts— updated for router-only behaviortests/unit/webhook/webhookParsers.test.ts— relocated fromtests/unit/server/tests/unit/pm/webhook-handler.test.ts— removed server-mode teststests/unit/triggers/github-webhook-handler.test.ts— removed server-mode teststests/unit/utils/lifecycle.test.ts— removedsetProcessing/isCurrentlyProcessingtestsTest plan
src/router/index.tsis the sole entry point for the application🤖 Generated with Claude Code