diff --git a/.cascade/ensure-services.sh b/.cascade/ensure-services.sh index 21afbd0a..282f81bd 100755 --- a/.cascade/ensure-services.sh +++ b/.cascade/ensure-services.sh @@ -91,4 +91,42 @@ if pg_isready -q 2>/dev/null; then fi fi +# Redis check and restart (required for router mode / BullMQ job queue) +if redis-cli ping 2>/dev/null | grep -q PONG; then + echo "Redis: running" +else + echo "Redis: down - attempting restart..." + + if command -v brew &>/dev/null; then + brew services start redis 2>/dev/null || true + elif command -v service &>/dev/null; then + sudo service redis-server start 2>/dev/null || \ + redis-server --daemonize yes 2>/dev/null || true + elif command -v redis-server &>/dev/null; then + redis-server --daemonize yes 2>/dev/null || true + fi + + # Wait for Redis to be ready + for i in {1..10}; do + if redis-cli ping 2>/dev/null | grep -q PONG; then + break + fi + echo "Waiting for Redis... ($i/10)" + sleep 1 + done + + # Final check + if redis-cli ping 2>/dev/null | grep -q PONG; then + echo "Redis: restarted successfully" + else + echo "Redis: FAILED TO START" + echo "" + echo "Troubleshooting:" + echo " - Install Redis: brew install redis (macOS) or apt-get install redis-server (Linux)" + echo " - Start manually: redis-server" + echo " - Note: Redis is required for router mode (BullMQ job queue)" + exit 1 + fi +fi + echo "=== All services running ===" diff --git a/.cascade/env b/.cascade/env index e4815a30..44ca356f 100644 --- a/.cascade/env +++ b/.cascade/env @@ -2,3 +2,4 @@ CI=true DATABASE_URL=postgresql://postgres:postgres@localhost:5432/cascade DATABASE_SSL=false TEST_DATABASE_URL=postgresql://postgres:postgres@localhost:5432/cascade_test +REDIS_URL=redis://localhost:6379 diff --git a/.cascade/setup.sh b/.cascade/setup.sh index ea9c84e2..0f9b1feb 100755 --- a/.cascade/setup.sh +++ b/.cascade/setup.sh @@ -297,6 +297,75 @@ else log_warn "PostgreSQL not ready, skipping migrations" fi +# ============================================================================= +# 6. Redis Setup (required for router mode / BullMQ job queue) +# ============================================================================= +echo "" +echo "--- Redis Setup ---" + +start_redis_macos() { + if command -v brew &> /dev/null; then + if ! brew list redis &> /dev/null; then + log_info "Redis not installed, installing..." + brew install redis + fi + log_info "Starting Redis..." + brew services start redis 2>/dev/null || true + # Wait for Redis to be ready + for i in {1..10}; do + if redis-cli ping 2>/dev/null | grep -q PONG; then + break + fi + log_info "Waiting for Redis... ($i/10)" + sleep 1 + done + else + log_error "Homebrew not found on macOS. Please install Redis manually." + return 1 + fi +} + +start_redis_linux() { + if ! command -v redis-server &> /dev/null; then + log_info "Redis not installed, installing..." + if command -v apt-get &> /dev/null; then + sudo apt-get update && sudo apt-get install -y redis-server + else + log_error "Cannot install Redis - apt-get not available" + return 1 + fi + fi + + # Start Redis if not running + if ! redis-cli ping 2>/dev/null | grep -q PONG; then + log_info "Starting Redis..." + sudo service redis-server start 2>/dev/null || \ + redis-server --daemonize yes 2>/dev/null || true + # Wait for Redis to be ready + for i in {1..10}; do + if redis-cli ping 2>/dev/null | grep -q PONG; then + break + fi + log_info "Waiting for Redis... ($i/10)" + sleep 1 + done + fi +} + +case "$OS" in + macos) start_redis_macos ;; + linux) start_redis_linux ;; + *) log_warn "Unknown OS, skipping Redis auto-start" ;; +esac + +# Verify Redis is running +if redis-cli ping 2>/dev/null | grep -q PONG; then + log_info "Redis is running" +else + log_warn "Redis failed to start — router mode requires Redis for BullMQ job queue" + log_warn "Run 'redis-server' manually or install Redis before using 'npm run dev'" +fi + # ============================================================================= # Summary # ============================================================================= @@ -304,5 +373,6 @@ echo "" echo "=== CASCADE Setup Complete ===" echo "OS: $OS" echo "PostgreSQL: $(pg_isready 2>&1 || echo 'not running')" +echo "Redis: $(redis-cli ping 2>/dev/null || echo 'not running')" echo "Node: $(node --version)" echo "npm: $(npm --version)" diff --git a/.env.example b/.env.example index ea9f8d78..32c5fbed 100644 --- a/.env.example +++ b/.env.example @@ -13,6 +13,12 @@ OPENROUTER_API_KEY= PORT=3000 LOG_LEVEL=info +# Redis — required for router mode (BullMQ job queue for webhook processing) +# Run: brew install redis && brew services start redis (macOS) +# apt-get install redis-server && service redis-server start (Linux) +# The .cascade/setup.sh script installs and starts Redis automatically. +REDIS_URL=redis://localhost:6379 + # Optional: Path to projects config file # CONFIG_PATH=./config/projects.json diff --git a/CLAUDE.md b/CLAUDE.md index c160585a..9f937bf7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,22 +5,32 @@ ```bash npm install cd web && npm install && cd .. -npm run dev # Backend +# Start Redis (required for router/BullMQ): +# macOS: brew install redis && brew services start redis +# Linux: apt-get install redis-server && service redis-server start +# The .cascade/setup.sh script handles this automatically. +npm run dev # Router (webhook receiver, requires Redis) npm run dev:web # Dashboard frontend (separate terminal) ``` ## Architecture -CASCADE reacts to Trello webhooks and runs AI agents to analyze, plan, and implement features. +CASCADE runs as three services (no monolithic server mode): + +1. **Router** (`src/router/index.ts`) — receives webhooks, enqueues jobs to Redis via BullMQ +2. **Worker** (`src/worker-entry.ts`) — processes one job per container, exits when done +3. **Dashboard** (`src/dashboard.ts`) — API + tRPC for web UI and CLI ### Trigger System The extensible trigger system routes events to agents: ``` -Trello/GitHub Webhook → TriggerRegistry → Agent → Code Changes → PR +Trello/GitHub Webhook → Router → Redis/BullMQ → Worker → TriggerRegistry → Agent → Code Changes → PR ``` +- `src/router/` - Webhook receiver (enqueues jobs to Redis) +- `src/webhook/` - Shared webhook handler factory, parsers, and logging - `src/triggers/` - Event handlers (Trello card moves, labels, GitHub PRs, attachments) - `src/agents/` - AI agents (splitting, planning, implementation, review, debug) - `src/gadgets/` - Tools agents can use (Trello API, Git operations, file system) @@ -53,6 +63,8 @@ Lefthook runs pre-commit (lint, typecheck) and pre-push (test) hooks automatical ## Key Directories +- `src/router/` - Router entry point (webhook receiver, enqueues to Redis) +- `src/webhook/` - Shared webhook handler factory, parsers, and logging helpers - `src/config/` - Configuration provider, caching, Zod schemas - `src/db/` - Database client, Drizzle schema, repositories - `src/triggers/` - Extensible trigger system (Trello, GitHub) @@ -70,6 +82,7 @@ Lefthook runs pre-commit (lint, typecheck) and pre-push (test) hooks automatical Required: - `DATABASE_URL` - PostgreSQL connection string (Supabase transaction pooler, port 6543) +- `REDIS_URL` - Redis connection string for BullMQ job queue (router + worker). Defaults to `redis://localhost:6379`. Run `.cascade/setup.sh` to install and start Redis locally. Optional (infrastructure): - `PORT` - Server port (default: 3000) diff --git a/README.md b/README.md index 125208ed..aa631334 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Multi-project Trello-to-code automation platform. CASCADE reacts to Trello card - npm - Git - GitHub CLI (`gh`) - for PR creation +- Redis (required for router mode / BullMQ job queue) ### Installation @@ -169,13 +170,23 @@ npm run build npm start ``` +### Architecture + +CASCADE runs as three services: + +1. **Router** (`src/router/index.ts`) — receives webhooks, enqueues jobs to Redis via BullMQ +2. **Worker** (`src/worker-entry.ts`) — processes one job per container, exits when done +3. **Dashboard** (`src/dashboard.ts`) — API + tRPC for web UI and CLI + ### Project Structure ``` cascade/ ├── src/ -│ ├── index.ts # Entry point -│ ├── server.ts # Hono HTTP server +│ ├── router/ # Router entry point (webhook receiver) +│ ├── worker-entry.ts # Worker entry point (job processor) +│ ├── dashboard.ts # Dashboard entry point (API + tRPC) +│ ├── webhook/ # Shared webhook handler factory + parsers │ ├── config/ # Configuration loading & validation │ ├── triggers/ # Extensible trigger system │ │ ├── registry.ts # TriggerRegistry diff --git a/package.json b/package.json index 89c7cb7a..4f42ee4f 100644 --- a/package.json +++ b/package.json @@ -3,15 +3,15 @@ "version": "1.0.0", "description": "Trello-to-Code Automation Platform - Reacts to Trello card movements and collaborates like a dev team", "type": "module", - "main": "dist/index.js", + "main": "dist/router/index.js", "scripts": { - "dev": "node --env-file=.env --import tsx/esm --watch src/index.ts", + "dev": "node --env-file=.env --import tsx/esm --watch src/router/index.ts", "dev:web": "cd web && npx vite", "build": "tsc && npm run build:copy-yaml && npm run build:copy-system-prompts", "build:copy-yaml": "mkdir -p dist/agents/definitions && cp src/agents/definitions/*.yaml dist/agents/definitions/", "build:copy-system-prompts": "mkdir -p dist/agents/prompts && cp -r src/agents/prompts/templates dist/agents/prompts/", "build:web": "cd web && npm run build", - "start": "node dist/index.js", + "start": "node dist/router/index.js", "test": "vitest run --project unit", "test:unit": "vitest run --project unit", "test:integration": "vitest run --project integration", diff --git a/src/index.ts b/src/index.ts deleted file mode 100644 index 24d9e3d8..00000000 --- a/src/index.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { serve } from '@hono/node-server'; -import { loadEnvConfigSafe } from './config/env.js'; -import { loadConfig } from './config/provider.js'; -import { createServer } from './server.js'; -import { - createTriggerRegistry, - processGitHubWebhook, - processJiraWebhook, - registerBuiltInTriggers, -} from './triggers/index.js'; -import { processTrelloWebhook } from './triggers/trello/webhook-handler.js'; -import { logger, setLogLevel } from './utils/index.js'; - -async function main(): Promise { - // Load environment config - const envConfig = loadEnvConfigSafe(); - setLogLevel(envConfig.logLevel); - - logger.info('Starting Cascade server', { port: envConfig.port }); - - // Load projects config from database - const config = await loadConfig(); - logger.info('Loaded projects config', { projects: config.projects.map((p) => p.id) }); - - // Create trigger registry - const triggerRegistry = createTriggerRegistry(); - registerBuiltInTriggers(triggerRegistry); - - // Create server - const app = createServer({ - config, - onTrelloWebhook: async (payload) => { - await processTrelloWebhook(payload, triggerRegistry); - }, - onGitHubWebhook: async (payload, eventType) => { - await processGitHubWebhook(payload, eventType, triggerRegistry); - }, - onJiraWebhook: async (payload) => { - await processJiraWebhook(payload, triggerRegistry); - }, - }); - - // Start server - const server = serve({ - fetch: app.fetch, - port: envConfig.port || 3000, - }); - - logger.info(`Cascade server listening on port ${envConfig.port || 3000}`); - - // Graceful shutdown - const shutdown = () => { - logger.info('Shutting down...'); - server.close(); - process.exit(0); - }; - - process.on('SIGINT', shutdown); - process.on('SIGTERM', shutdown); -} - -main().catch((err) => { - console.error('Failed to start server:', err); - process.exit(1); -}); diff --git a/src/pm/webhook-handler.ts b/src/pm/webhook-handler.ts index 374cd73c..6fe4b09b 100644 --- a/src/pm/webhook-handler.ts +++ b/src/pm/webhook-handler.ts @@ -15,7 +15,6 @@ import { } from '../router/agent-type-lock.js'; import type { TriggerRegistry } from '../triggers/registry.js'; import { runAgentWithCredentials } from '../triggers/shared/webhook-execution.js'; -import { processNextQueuedWebhook } from '../triggers/shared/webhook-queue.js'; import type { TriggerResult } from '../triggers/types.js'; import type { CascadeConfig, @@ -23,17 +22,7 @@ import type { TriggerContext, TriggerSource, } from '../types/index.js'; -import { - clearCardActive, - enqueueWebhook, - getQueueLength, - isCardActive, - isCurrentlyProcessing, - logger, - setCardActive, - setProcessing, - startWatchdog, -} from '../utils/index.js'; +import { logger, startWatchdog } from '../utils/index.js'; import { getPMProvider, withPMProvider } from './context.js'; import type { PMIntegration } from './integration.js'; import { PMLifecycleManager, resolveProjectPMConfig } from './lifecycle.js'; @@ -59,14 +48,6 @@ async function executeAgent( // Webhook Processing // ============================================================================ -function processNextQueued(integration: PMIntegration, registry: TriggerRegistry): void { - processNextQueuedWebhook( - (payload, _eventType, ackCommentId) => - processPMWebhook(integration, payload, registry, ackCommentId as string | undefined), - integration.type.charAt(0).toUpperCase() + integration.type.slice(1), - ); -} - async function cleanupOrphanAck( integration: PMIntegration, projectId: string, @@ -105,7 +86,6 @@ async function resolveTriggerResult( return result; } -// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: webhook orchestration with multiple guard checks async function handleMatchedTrigger( integration: PMIntegration, registry: TriggerRegistry, @@ -130,12 +110,6 @@ async function handleMatchedTrigger( result.agentInput.ackCommentId = ackCommentId; } - const workItemId = result.workItemId; - if (workItemId && isCardActive(workItemId)) { - logger.info('Work item already being processed, skipping', { workItemId }); - return; - } - // Agent-type concurrency limit let agentTypeMaxConcurrency: number | null = null; if (result.agentType) { @@ -150,34 +124,25 @@ async function handleMatchedTrigger( logger.info(`${integration.type} trigger matched`, { agentType: result.agentType, - workItemId, + workItemId: result.workItemId, }); - setProcessing(true); startWatchdog(config.defaults.watchdogTimeoutMs); const pmConfig = resolveProjectPMConfig(project); const lifecycle = new PMLifecycleManager(getPMProvider(), pmConfig); try { - if (workItemId) { - setCardActive(workItemId); - } await executeAgent(integration, result, project, config); } catch (err) { logger.error(`Failed to process ${integration.type} webhook`, { error: String(err) }); - if (workItemId) { - await lifecycle.handleError(workItemId, String(err)); + if (result.workItemId) { + await lifecycle.handleError(result.workItemId, String(err)); } } finally { - if (workItemId) { - clearCardActive(workItemId); - } if (result.agentType && agentTypeMaxConcurrency !== null) { clearAgentTypeEnqueued(project.id, result.agentType); } - setProcessing(false); - processNextQueued(integration, registry); } } @@ -210,20 +175,6 @@ export async function processPMWebhook( return; } - if (isCurrentlyProcessing()) { - const queued = enqueueWebhook(payload, undefined, ackCommentId); - if (queued) { - logger.info(`Currently processing, ${integration.type} webhook queued`, { - queueLength: getQueueLength(), - }); - } else { - logger.warn(`Queue full, ${integration.type} webhook rejected`, { - queueLength: getQueueLength(), - }); - } - return; - } - logger.info(`${integration.type} webhook details`, { projectIdentifier: event.projectIdentifier, workItemId: event.workItemId, diff --git a/src/router/index.ts b/src/router/index.ts index 96e915e9..a738b65f 100644 --- a/src/router/index.ts +++ b/src/router/index.ts @@ -6,15 +6,15 @@ import '../pm/bootstrap.js'; import { initPrompts } from '../agents/prompts/index.js'; import { initAgentMessages } from '../config/agentMessages.js'; import { seedAgentDefinitions } from '../db/seeds/seedAgentDefinitions.js'; +import { registerBuiltInTriggers } from '../triggers/builtins.js'; +import { createTriggerRegistry } from '../triggers/registry.js'; +import { logger } from '../utils/logging.js'; import { createWebhookHandler, parseGitHubPayload, parseJiraPayload, parseTrelloPayload, -} from '../server/webhookHandlers.js'; -import { registerBuiltInTriggers } from '../triggers/builtins.js'; -import { createTriggerRegistry } from '../triggers/registry.js'; -import { logger } from '../utils/logging.js'; +} from '../webhook/webhookHandlers.js'; import { GitHubRouterAdapter, injectEventType } from './adapters/github.js'; import { JiraRouterAdapter } from './adapters/jira.js'; import { TrelloRouterAdapter } from './adapters/trello.js'; @@ -67,8 +67,6 @@ app.post( '/trello/webhook', createWebhookHandler({ source: 'trello', - checkCapacity: false, - fireAndForget: false, parsePayload: parseTrelloPayload, processWebhook: async (payload) => { const adapter = new TrelloRouterAdapter(); @@ -91,8 +89,6 @@ app.post( '/github/webhook', createWebhookHandler({ source: 'github', - checkCapacity: false, - fireAndForget: false, parsePayload: parseGitHubPayload, processWebhook: async (payload, eventType) => { const adapter = new GitHubRouterAdapter(); @@ -113,8 +109,6 @@ app.post( '/jira/webhook', createWebhookHandler({ source: 'jira', - checkCapacity: false, - fireAndForget: false, parsePayload: parseJiraPayload, processWebhook: async (payload) => { const adapter = new JiraRouterAdapter(); diff --git a/src/server.ts b/src/server.ts deleted file mode 100644 index 1aa94173..00000000 --- a/src/server.ts +++ /dev/null @@ -1,164 +0,0 @@ -import { existsSync, readFileSync } from 'node:fs'; -import { join } from 'node:path'; -import { serveStatic } from '@hono/node-server/serve-static'; -import { trpcServer } from '@hono/trpc-server'; -import { Hono } from 'hono'; -import { getCookie } from 'hono/cookie'; -import { cors } from 'hono/cors'; -import { logger as honoLogger } from 'hono/logger'; -import { SESSION_COOKIE_NAME } from './api/auth/cookie.js'; -import { loginHandler } from './api/auth/login.js'; -import { logoutHandler } from './api/auth/logout.js'; -import { resolveUserFromSession } from './api/auth/session.js'; -import { computeEffectiveOrgId } from './api/context.js'; -import { appRouter } from './api/router.js'; -import { captureException } from './sentry.js'; -import { - buildReactionSender, - createWebhookHandler, - parseGitHubPayload, - parseJiraPayload, - parseTrelloPayload, -} from './server/webhookHandlers.js'; -import type { CascadeConfig } from './types/index.js'; -import { logger } from './utils/index.js'; - -export interface ServerDependencies { - config: CascadeConfig; - onTrelloWebhook: (payload: unknown) => Promise; - onGitHubWebhook: (payload: unknown, eventType: string) => Promise; - onJiraWebhook: (payload: unknown) => Promise; -} - -export function createServer(deps: ServerDependencies): Hono { - const app = new Hono(); - - // Middleware - const corsOrigin = process.env.CORS_ORIGIN; - app.use('*', corsOrigin ? cors({ origin: corsOrigin, credentials: true }) : cors()); - app.use('*', honoLogger()); - - // Health check - app.get('/health', (c) => { - return c.json({ - status: 'ok', - timestamp: new Date().toISOString(), - projects: deps.config.projects.map((p) => p.id), - }); - }); - - // ========================================================================= - // Dashboard auth routes (plain Hono — they set cookies) - // ========================================================================= - app.post('/api/auth/login', loginHandler); - app.post('/api/auth/logout', logoutHandler); - - // ========================================================================= - // tRPC (all dashboard data queries) - // ========================================================================= - app.use( - '/trpc/*', - trpcServer({ - endpoint: '/trpc', - router: appRouter, - createContext: async (_opts, c) => { - const token = getCookie(c, SESSION_COOKIE_NAME); - const user = token ? await resolveUserFromSession(token) : null; - const effectiveOrgId = await computeEffectiveOrgId(user, c.req.header('x-org-context')); - - return { user, effectiveOrgId }; - }, - }), - ); - - // ========================================================================= - // Webhooks - // ========================================================================= - - // Trello webhook - GET/HEAD for verification (Trello sends HEAD to verify) - app.get('/trello/webhook', (c) => { - return c.text('OK', 200); - }); - - // Trello webhook - POST for events - app.post( - '/trello/webhook', - createWebhookHandler({ - source: 'trello', - parsePayload: parseTrelloPayload, - sendReaction: buildReactionSender('trello', deps.config), - processWebhook: (payload) => deps.onTrelloWebhook(payload), - }), - ); - - // GitHub webhook - GET/HEAD for verification - app.get('/github/webhook', (c) => { - return c.text('OK', 200); - }); - - // GitHub webhook - POST for events - app.post( - '/github/webhook', - createWebhookHandler({ - source: 'github', - parsePayload: parseGitHubPayload, - sendReaction: buildReactionSender('github'), - processWebhook: (payload, eventType) => deps.onGitHubWebhook(payload, eventType ?? 'unknown'), - }), - ); - - // JIRA webhook - GET/HEAD for verification - app.get('/jira/webhook', (c) => { - return c.text('OK', 200); - }); - - // JIRA webhook - POST for events - app.post( - '/jira/webhook', - createWebhookHandler({ - source: 'jira', - parsePayload: parseJiraPayload, - sendReaction: buildReactionSender('jira', deps.config), - processWebhook: (payload) => deps.onJiraWebhook(payload), - }), - ); - - // ========================================================================= - // Static file serving (production — built frontend) - // ========================================================================= - const webDistPath = join(import.meta.dirname, '..', 'dist', 'web'); - const webDistExists = existsSync(webDistPath); - - if (webDistExists) { - app.use('/*', serveStatic({ root: './dist/web' })); - } - - // SPA fallback — serve index.html for unmatched routes - app.notFound((c) => { - if (webDistExists) { - const accept = c.req.header('Accept') ?? ''; - if (accept.includes('text/html')) { - const indexPath = join(webDistPath, 'index.html'); - try { - const html = readFileSync(indexPath, 'utf-8'); - return c.html(html); - } catch { - // fall through to JSON 404 - } - } - } - return c.json({ error: 'Not Found' }, 404); - }); - - // Error handler - app.onError((err, c) => { - logger.error('Unhandled error', { error: String(err), path: c.req.path }); - captureException(err, { - tags: { source: 'hono_error' }, - extra: { path: c.req.path, method: c.req.method }, - }); - return c.json({ error: 'Internal Server Error' }, 500); - }); - - return app; -} diff --git a/src/server/webhookReactionSender.ts b/src/server/webhookReactionSender.ts deleted file mode 100644 index f093d71c..00000000 --- a/src/server/webhookReactionSender.ts +++ /dev/null @@ -1,133 +0,0 @@ -/** - * Unified fire-and-forget reaction sender for webhook endpoints. - * - * Replaces the three platform-specific builders - * (`buildTrelloReactionSender`, `buildGitHubReactionSender`, - * `buildJiraReactionSender`) with a single `buildReactionSender` factory - * that dispatches to `sendAcknowledgeReaction` based on the webhook source. - * - * Platform-specific event filtering and project resolution live here so - * that the caller (server.ts) only needs one import and one call per - * webhook route. - */ - -import { findProjectByRepo } from '../config/provider.js'; -import { resolvePersonaIdentities } from '../github/personas.js'; -import { sendAcknowledgeReaction } from '../router/reactions.js'; -import type { CascadeConfig } from '../types/index.js'; -import { logger } from '../utils/index.js'; - -// --------------------------------------------------------------------------- -// Types -// --------------------------------------------------------------------------- - -type ReactionSender = (payload: unknown, eventType: string | undefined) => void; - -// --------------------------------------------------------------------------- -// Internal platform helpers -// --------------------------------------------------------------------------- - -/** - * Fire-and-forget Trello reaction. - * Only reacts on `commentCard` events. - */ -function trelloReactionSender(config: CascadeConfig): ReactionSender { - return (payload, eventType) => { - if (eventType !== 'commentCard') return; - const boardId = (payload as Record>).model?.id as - | string - | undefined; - const project = config.projects.find((p) => p.trello?.boardId === boardId); - if (!project) return; - void sendAcknowledgeReaction('trello', project.id, payload).catch((err) => - logger.error('[Server] Trello reaction error:', { error: String(err) }), - ); - }; -} - -/** - * Fire-and-forget GitHub reaction. - * Only reacts on `issue_comment` or `pull_request_review_comment` events. - */ -function gitHubReactionSender(): ReactionSender { - return (payload, eventType) => { - if (eventType !== 'issue_comment' && eventType !== 'pull_request_review_comment') return; - const repoFullName = ( - (payload as Record)?.repository as Record - )?.full_name as string | undefined; - if (!repoFullName) return; - void (async () => { - try { - const project = await findProjectByRepo(repoFullName); - if (!project) { - logger.warn('[Server] No project found for repo, skipping GitHub reaction', { - repoFullName, - }); - return; - } - const personaIdentities = await resolvePersonaIdentities(project.id); - await sendAcknowledgeReaction('github', repoFullName, payload, personaIdentities, project); - } catch (err) { - logger.error('[Server] GitHub reaction error:', { error: String(err) }); - } - })(); - }; -} - -/** - * Fire-and-forget JIRA reaction. - * Only reacts on events whose name starts with `comment_`. - */ -function jiraReactionSender(config: CascadeConfig): ReactionSender { - return (payload, eventType) => { - if (!eventType?.startsWith('comment_')) return; - const jiraProjectKey = ( - ((payload as Record)?.issue as Record)?.fields as Record< - string, - unknown - > - )?.project as Record | undefined; - const projectKey = jiraProjectKey?.key as string | undefined; - const project = projectKey - ? config.projects.find((p) => p.jira?.projectKey === projectKey) - : undefined; - if (!project) return; - void sendAcknowledgeReaction('jira', project.id, payload).catch((err) => - logger.error('[Server] JIRA reaction error:', { error: String(err) }), - ); - }; -} - -// --------------------------------------------------------------------------- -// Unified factory -// --------------------------------------------------------------------------- - -/** - * Build a fire-and-forget reaction sender for the given webhook source. - * - * - `'trello'` — reacts on `commentCard` events; needs `config` for project lookup. - * - `'github'` — reacts on `issue_comment` / `pull_request_review_comment` events; - * resolves the project dynamically via `findProjectByRepo`. - * - `'jira'` — reacts on `comment_*` events; needs `config` for project lookup. - * - * The returned function is safe to call fire-and-forget: all errors are caught - * and logged internally. - */ -export function buildReactionSender(source: string, config?: CascadeConfig): ReactionSender { - switch (source) { - case 'trello': { - if (!config) throw new Error('buildReactionSender: config required for trello'); - return trelloReactionSender(config); - } - case 'github': { - return gitHubReactionSender(); - } - case 'jira': { - if (!config) throw new Error('buildReactionSender: config required for jira'); - return jiraReactionSender(config); - } - default: - // Unknown source — return a no-op reaction sender for forward compatibility - return () => {}; - } -} diff --git a/src/triggers/github/webhook-handler.ts b/src/triggers/github/webhook-handler.ts index f7dba689..423b486d 100644 --- a/src/triggers/github/webhook-handler.ts +++ b/src/triggers/github/webhook-handler.ts @@ -19,22 +19,11 @@ import { markRecentlyDispatched, } from '../../router/agent-type-lock.js'; import type { CascadeConfig, ProjectConfig, TriggerContext } from '../../types/index.js'; -import { - clearCardActive, - enqueueWebhook, - getQueueLength, - isCardActive, - isCurrentlyProcessing, - logger, - setCardActive, - setProcessing, - startWatchdog, -} from '../../utils/index.js'; +import { logger, startWatchdog } from '../../utils/index.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { safeOperation } from '../../utils/safeOperation.js'; import type { TriggerRegistry } from '../registry.js'; import { runAgentWithCredentials } from '../shared/webhook-execution.js'; -import { processNextQueuedWebhook } from '../shared/webhook-queue.js'; import type { TriggerResult } from '../types.js'; import { postAcknowledgmentComment, updateInitialCommentWithError } from './ack-comments.js'; import { pollWaitForChecks } from './check-polling.js'; @@ -42,41 +31,6 @@ import { GitHubWebhookIntegration } from './integration.js'; const integration = new GitHubWebhookIntegration(); -function processNextQueuedGitHubWebhook(registry: TriggerRegistry): void { - processNextQueuedWebhook( - (payload, eventType, ackCommentId, ackMsg) => - processGitHubWebhook( - payload, - eventType ?? 'pull_request_review_comment', - registry, - ackCommentId as number | undefined, - ackMsg, - ), - 'GitHub', - (entry) => entry.eventType ?? 'pull_request_review_comment', - ); -} - -/** Enqueue the webhook if another job is currently processing. Returns true if enqueued. */ -function tryEnqueueIfBusy( - payload: unknown, - eventType: string, - ackCommentId?: number, - ackMessage?: string, -): boolean { - if (!isCurrentlyProcessing()) return false; - const queued = enqueueWebhook(payload, eventType, ackCommentId, ackMessage); - if (queued) { - logger.info('Currently processing, GitHub webhook queued', { - queueLength: getQueueLength(), - eventType, - }); - } else { - logger.warn('Queue full, GitHub webhook rejected', { queueLength: getQueueLength() }); - } - return true; -} - /** Dispatch to trigger registry within PM credential + provider scope. */ async function dispatchTrigger( registry: TriggerRegistry, @@ -114,20 +68,12 @@ async function maybePostAckComment( ); } -/** Run the agent with GitHub-specific execution config, managing processing flags. */ -// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: webhook orchestration with multiple guard checks +/** Run the agent with GitHub-specific execution config. */ async function runGitHubAgent( result: TriggerResult, project: ProjectConfig, config: CascadeConfig, - registry: TriggerRegistry, ): Promise { - const workItemId = result.workItemId; - if (workItemId && isCardActive(workItemId)) { - logger.info('Work item already being processed, skipping', { workItemId }); - return; - } - // Agent-type concurrency limit let agentTypeMaxConcurrency: number | null = null; if (result.agentType) { @@ -140,11 +86,9 @@ async function runGitHubAgent( } } - setProcessing(true); startWatchdog(config.defaults.watchdogTimeoutMs); try { - if (workItemId) setCardActive(workItemId); // Establish PM credential + provider scope for agents with workItemId // (needed for PM lifecycle operations: labels, status moves, PR links) const pmProvider = createPMProvider(project); @@ -176,12 +120,9 @@ async function runGitHubAgent( updateInitialCommentWithError(result, { success: false, error: String(err) }), ); } finally { - if (workItemId) clearCardActive(workItemId); if (result.agentType && agentTypeMaxConcurrency !== null) { clearAgentTypeEnqueued(project.id, result.agentType); } - setProcessing(false); - processNextQueuedGitHubWebhook(registry); } } @@ -202,8 +143,6 @@ export async function processGitHubWebhook( return; } - if (tryEnqueueIfBusy(payload, eventType, ackCommentId, ackMessage)) return; - const projectConfig = await integration.lookupProject(event.projectIdentifier); if (!projectConfig) { logger.warn('No project configured for repository', { @@ -272,5 +211,5 @@ export async function processGitHubWebhook( await maybePostAckComment(result, payload, eventType, project); } - await runGitHubAgent(result, project, config, registry); + await runGitHubAgent(result, project, config); } diff --git a/src/triggers/shared/webhook-queue.ts b/src/triggers/shared/webhook-queue.ts deleted file mode 100644 index 7cc8d74c..00000000 --- a/src/triggers/shared/webhook-queue.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { dequeueWebhook, getQueueLength } from '../../utils/index.js'; -import { logger } from '../../utils/logging.js'; - -/** - * Dequeue and process the next queued webhook if one is waiting. - * - * @param processWebhook - Function to call with the next queued payload. - * @param label - Log label for the source (e.g. 'Trello', 'GitHub', 'JIRA'). - * @param getEventType - Optional function to extract event type from the queued entry. - */ -export function processNextQueuedWebhook( - processWebhook: ( - payload: unknown, - eventType?: string, - ackCommentId?: string | number, - ackMessage?: string, - ) => Promise, - label: string, - getEventType?: (entry: { payload: unknown; eventType?: string }) => string | undefined, -): void { - const next = dequeueWebhook(); - if (next) { - const eventType = getEventType ? getEventType(next) : undefined; - const logContext: Record = { queueLength: getQueueLength() }; - if (eventType) logContext.eventType = eventType; - logger.info(`Processing queued ${label} webhook`, logContext); - setImmediate(() => { - processWebhook(next.payload, eventType, next.ackCommentId, next.ackMessage).catch((err) => { - logger.error(`Failed to process queued ${label} webhook`, { - error: String(err), - }); - }); - }); - } -} diff --git a/src/utils/activeCards.ts b/src/utils/activeCards.ts deleted file mode 100644 index 4794179a..00000000 --- a/src/utils/activeCards.ts +++ /dev/null @@ -1,21 +0,0 @@ -const activeCards = new Set(); - -export function isCardActive(cardId: string): boolean { - return activeCards.has(cardId); -} - -export function setCardActive(cardId: string): void { - activeCards.add(cardId); -} - -export function clearCardActive(cardId: string): void { - activeCards.delete(cardId); -} - -export function getActiveCardCount(): number { - return activeCards.size; -} - -export function clearAllActiveCards(): void { - activeCards.clear(); -} diff --git a/src/utils/index.ts b/src/utils/index.ts index bbb10022..de5a5b91 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,7 +1,5 @@ export { logger, setLogLevel, getLogLevel } from './logging.js'; export { - setProcessing, - isCurrentlyProcessing, startWatchdog, clearWatchdog, setWatchdogCleanup, @@ -15,19 +13,4 @@ export { getWorkspaceDir, parseRepoFullName, } from './repo.js'; -export { - enqueueWebhook, - dequeueWebhook, - getQueueLength, - clearQueue, - getMaxQueueSize, - canAcceptWebhook, -} from './webhookQueue.js'; export { createFileLogger, cleanupLogFile, type FileLogger } from './fileLogger.js'; -export { - isCardActive, - setCardActive, - clearCardActive, - getActiveCardCount, - clearAllActiveCards, -} from './activeCards.js'; diff --git a/src/utils/lifecycle.ts b/src/utils/lifecycle.ts index cfbc6a25..fe5d1a8f 100644 --- a/src/utils/lifecycle.ts +++ b/src/utils/lifecycle.ts @@ -2,17 +2,8 @@ import { flush } from '../sentry.js'; import { logger } from './logging.js'; let watchdogTimer: ReturnType | null = null; -let isProcessing = false; let watchdogCleanup: (() => Promise) | null = null; -export function setProcessing(processing: boolean): void { - isProcessing = processing; -} - -export function isCurrentlyProcessing(): boolean { - return isProcessing; -} - // Watchdog cleanup callback - called before force exit export function setWatchdogCleanup(cleanup: () => Promise): void { watchdogCleanup = cleanup; diff --git a/src/utils/webhookQueue.ts b/src/utils/webhookQueue.ts deleted file mode 100644 index 723f6733..00000000 --- a/src/utils/webhookQueue.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { logger } from './logging.js'; - -const MAX_QUEUE_SIZE = 10; - -interface QueuedWebhook { - payload: unknown; - eventType?: string; // Optional for backward compatibility (Trello doesn't need it) - ackCommentId?: string | number; - ackMessage?: string; - receivedAt: Date; -} - -const queue: QueuedWebhook[] = []; - -export function enqueueWebhook( - payload: unknown, - eventType?: string, - ackCommentId?: string | number, - ackMessage?: string, -): boolean { - if (queue.length >= MAX_QUEUE_SIZE) { - logger.warn('Webhook queue full, rejecting', { - queueLength: queue.length, - maxSize: MAX_QUEUE_SIZE, - }); - return false; - } - - queue.push({ - payload, - eventType, - ackCommentId, - ackMessage, - receivedAt: new Date(), - }); - - logger.debug('Webhook enqueued', { queueLength: queue.length }); - return true; -} - -export function dequeueWebhook(): QueuedWebhook | undefined { - const item = queue.shift(); - if (item) { - logger.debug('Webhook dequeued', { - queueLength: queue.length, - ageMs: Date.now() - item.receivedAt.getTime(), - }); - } - return item; -} - -export function getQueueLength(): number { - return queue.length; -} - -export function clearQueue(): void { - const length = queue.length; - queue.length = 0; - if (length > 0) { - logger.debug('Queue cleared', { itemsCleared: length }); - } -} - -export function getMaxQueueSize(): number { - return MAX_QUEUE_SIZE; -} - -export function canAcceptWebhook(): boolean { - return queue.length < MAX_QUEUE_SIZE; -} diff --git a/src/server/webhookHandlers.ts b/src/webhook/webhookHandlers.ts similarity index 52% rename from src/server/webhookHandlers.ts rename to src/webhook/webhookHandlers.ts index 900269e4..6e270541 100644 --- a/src/server/webhookHandlers.ts +++ b/src/webhook/webhookHandlers.ts @@ -1,16 +1,8 @@ /** * Generic webhook handler factory for Trello, GitHub, and JIRA endpoints. * - * Eliminates the three near-identical 50-60 line POST handler blocks that - * previously existed in both `src/server.ts` and `src/router/index.ts` by - * extracting the shared flow (capacity check, header extraction, parse, - * log, react, process) into a single parameterized factory. - * - * Supports two processing modes via `fireAndForget`: - * - `true` (default, server mode): respond 200 immediately, process later. - * - `false` (router mode): await processing so 200 means "job queued." - * Errors propagate to Hono's error handler (500), preserving the old - * router behavior. + * Router mode only: always awaits processing before returning 200 + * (so 200 means "job queued"). Errors propagate to Hono's error handler (500). * * Supports log enrichment via the return value of `processWebhook`. When * the callback returns `WebhookLogOverrides`, those fields override the @@ -20,17 +12,16 @@ import type { Context, Handler } from 'hono'; import { extractRawHeaders } from '../router/webhookParsing.js'; -import { canAcceptWebhook, isCurrentlyProcessing, logger } from '../utils/index.js'; +import { logger } from '../utils/index.js'; import { logWebhookCall } from '../utils/webhookLogger.js'; import { handleProcessingError, logSuccessfulWebhook } from './webhookLogging.js'; // --------------------------------------------------------------------------- -// Re-exports for backward compatibility +// Re-exports // --------------------------------------------------------------------------- export type { ParseResult, WebhookHandlerConfig, WebhookLogOverrides } from './webhookTypes.js'; export { parseGitHubPayload, parseJiraPayload, parseTrelloPayload } from './webhookParsers.js'; -export { buildReactionSender } from './webhookReactionSender.js'; // --------------------------------------------------------------------------- // Types (local import for factory use) @@ -46,30 +37,16 @@ import type { WebhookHandlerConfig } from './webhookTypes.js'; * Build a Hono POST handler for a webhook endpoint. * * The handler: - * 1. Optionally checks machine capacity (503 if over limit). - * 2. Parses the request payload via `config.parsePayload`. - * 3. Logs the webhook call to the database (both success and failure paths). - * 4. Fires a fire-and-forget acknowledgment reaction on success. - * 5. Processes the webhook (fire-and-forget or awaited, per `fireAndForget`). - * 6. Returns 200 immediately (or 400/503 on failure). + * 1. Parses the request payload via `config.parsePayload`. + * 2. Logs the webhook call to the database (both success and failure paths). + * 3. Fires a fire-and-forget acknowledgment reaction on success. + * 4. Awaits processing so 200 means "job queued." + * 5. Returns 200 (or 400 on parse failure). */ export function createWebhookHandler(config: WebhookHandlerConfig): Handler { - const { - source, - parsePayload, - sendReaction, - processWebhook, - checkCapacity = true, - fireAndForget = true, - } = config; + const { source, parsePayload, sendReaction, processWebhook } = config; return async (c: Context) => { - // --- Capacity gate (server mode only) --- - if (checkCapacity && isCurrentlyProcessing() && !canAcceptWebhook()) { - logger.warn('Machine at capacity, returning 503'); - return c.text('Service Unavailable', 503); - } - const rawHeaders = extractRawHeaders(c); // --- Parse --- @@ -97,21 +74,15 @@ export function createWebhookHandler(config: WebhookHandlerConfig): Handler { sendReaction(payload, eventType); } - if (fireAndForget) { - // --- Log then process asynchronously (server mode) --- - // Log overrides from processWebhook are not available in this mode - // because processing hasn't started yet. - logSuccessfulWebhook(source, c, rawHeaders, payload, eventType); - setImmediate(() => { - processWebhook(payload, eventType).catch((err) => handleProcessingError(source, err)); - }); - } else { - // --- Await processing then log (router mode) --- - // Process synchronously so 200 means "job queued." - // Errors propagate to Hono's error handler (500), matching old router - // behavior where a processing failure was not acknowledged with 200. + // --- Await processing (router mode always awaits) --- + // Process synchronously so 200 means "job queued." + // Errors propagate to Hono's error handler (500). + try { const logOverrides = await processWebhook(payload, eventType); logSuccessfulWebhook(source, c, rawHeaders, payload, eventType, logOverrides); + } catch (err) { + handleProcessingError(source, err); + throw err; } return c.text('OK', 200); diff --git a/src/server/webhookLogging.ts b/src/webhook/webhookLogging.ts similarity index 100% rename from src/server/webhookLogging.ts rename to src/webhook/webhookLogging.ts diff --git a/src/server/webhookParsers.ts b/src/webhook/webhookParsers.ts similarity index 100% rename from src/server/webhookParsers.ts rename to src/webhook/webhookParsers.ts diff --git a/src/server/webhookTypes.ts b/src/webhook/webhookTypes.ts similarity index 50% rename from src/server/webhookTypes.ts rename to src/webhook/webhookTypes.ts index 8ae244f5..b7112ff4 100644 --- a/src/server/webhookTypes.ts +++ b/src/webhook/webhookTypes.ts @@ -41,43 +41,14 @@ export interface WebhookHandlerConfig { sendReaction?: (payload: unknown, eventType: string | undefined) => void; /** - * Processing callback. By default invoked via `setImmediate` (fire-and-forget) - * after a 200 is returned to the caller. When `fireAndForget` is `false`, the - * handler awaits this callback before responding — useful when processing must - * complete (e.g. job queuing) before acknowledging the webhook. + * 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 - * (e.g. `processed`, `projectId`). This is the recommended way to communicate - * processing outcome to the log — it avoids shared mutable state and is - * inherently safe under concurrent requests. - * - * When `fireAndForget` is `true`, returned overrides are ignored (logging - * happens before processing starts). When `fireAndForget` is `false`, they - * are used to enrich the log after processing completes. + * May optionally return `WebhookLogOverrides` to enrich the webhook log entry. */ processWebhook: ( payload: unknown, eventType: string | undefined, // biome-ignore lint/suspicious/noConfusingVoidType: void needed for Promise compat ) => Promise; - - /** - * Whether to apply the global capacity gate (isCurrentlyProcessing && - * !canAcceptWebhook → 503). Set to `false` for the router deployment - * mode which handles back-pressure differently. - * Defaults to `true`. - */ - checkCapacity?: boolean; - - /** - * Whether to schedule `processWebhook` asynchronously via `setImmediate` - * (fire-and-forget) or await it before responding. - * - * - `true` (default) — server mode: respond 200 immediately, process later. - * - `false` — router mode: await processing so 200 means "job queued." - * Errors from `processWebhook` propagate to Hono's error handler (500), - * matching the old router behavior where a failure was not acknowledged - * with 200. - */ - fireAndForget?: boolean; } diff --git a/tests/unit/pm/webhook-handler.test.ts b/tests/unit/pm/webhook-handler.test.ts index 5a9bd322..a8f197cc 100644 --- a/tests/unit/pm/webhook-handler.test.ts +++ b/tests/unit/pm/webhook-handler.test.ts @@ -12,24 +12,13 @@ vi.mock('../../../src/triggers/shared/agent-execution.js', () => ({ runAgentExecutionPipeline: vi.fn().mockResolvedValue(undefined), })); -vi.mock('../../../src/triggers/shared/webhook-queue.js', () => ({ - processNextQueuedWebhook: vi.fn(), -})); - vi.mock('../../../src/utils/index.js', () => ({ - clearCardActive: vi.fn(), - enqueueWebhook: vi.fn().mockReturnValue(true), - getQueueLength: vi.fn().mockReturnValue(0), - isCardActive: vi.fn().mockReturnValue(false), - isCurrentlyProcessing: vi.fn().mockReturnValue(false), logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), }, - setCardActive: vi.fn(), - setProcessing: vi.fn(), startWatchdog: vi.fn(), })); @@ -65,23 +54,9 @@ vi.mock('../../../src/router/agent-type-lock.js', () => ({ import { processPMWebhook } from '../../../src/pm/webhook-handler.js'; import { checkAgentTypeConcurrency } from '../../../src/router/agent-type-lock.js'; import { runAgentExecutionPipeline } from '../../../src/triggers/shared/agent-execution.js'; -import { - clearCardActive, - enqueueWebhook, - isCardActive, - isCurrentlyProcessing, - setCardActive, - setProcessing, - startWatchdog, -} from '../../../src/utils/index.js'; - -const mockIsCurrentlyProcessing = vi.mocked(isCurrentlyProcessing); -const mockIsCardActive = vi.mocked(isCardActive); -const mockEnqueueWebhook = vi.mocked(enqueueWebhook); -const mockSetProcessing = vi.mocked(setProcessing); +import { startWatchdog } from '../../../src/utils/index.js'; + const mockStartWatchdog = vi.mocked(startWatchdog); -const mockSetCardActive = vi.mocked(setCardActive); -const mockClearCardActive = vi.mocked(clearCardActive); const mockRunAgentExecutionPipeline = vi.mocked(runAgentExecutionPipeline); // ============================================================================ @@ -139,9 +114,6 @@ function createMockRegistry(result?: object | null) { } beforeEach(() => { - mockIsCurrentlyProcessing.mockReturnValue(false); - mockIsCardActive.mockReturnValue(false); - mockEnqueueWebhook.mockReturnValue(true); mockRunAgentExecutionPipeline.mockResolvedValue(undefined); }); @@ -161,17 +133,6 @@ describe('processPMWebhook', () => { expect(registry.dispatch).not.toHaveBeenCalled(); }); - it('enqueues webhook when currently processing', async () => { - mockIsCurrentlyProcessing.mockReturnValue(true); - const integration = createMockIntegration(); - const registry = createMockRegistry(); - - await processPMWebhook(integration as never, { type: 'card_moved' }, registry as never); - - expect(mockEnqueueWebhook).toHaveBeenCalled(); - expect(registry.dispatch).not.toHaveBeenCalled(); - }); - it('returns early when no project found for identifier', async () => { const integration = createMockIntegration({ lookupProject: vi.fn().mockResolvedValue(null), @@ -201,16 +162,6 @@ describe('processPMWebhook', () => { expect(mockRunAgentExecutionPipeline).toHaveBeenCalled(); }); - it('sets card active and clears it after execution', async () => { - const integration = createMockIntegration(); - const registry = createMockRegistry(); - - await processPMWebhook(integration as never, { type: 'card_moved' }, registry as never); - - expect(mockSetCardActive).toHaveBeenCalledWith('card-abc'); - expect(mockClearCardActive).toHaveBeenCalledWith('card-abc'); - }); - it('starts watchdog on trigger match', async () => { const integration = createMockIntegration(); const registry = createMockRegistry(); @@ -220,36 +171,6 @@ describe('processPMWebhook', () => { expect(mockStartWatchdog).toHaveBeenCalledWith(120000); }); - it('sets processing to true on start and false when done', async () => { - const integration = createMockIntegration(); - const registry = createMockRegistry(); - - await processPMWebhook(integration as never, { type: 'card_moved' }, registry as never); - - expect(mockSetProcessing).toHaveBeenCalledWith(true); - expect(mockSetProcessing).toHaveBeenCalledWith(false); - }); - - it('skips agent execution when work item is already active', async () => { - mockIsCardActive.mockReturnValue(true); - const integration = createMockIntegration(); - const registry = createMockRegistry(); - - await processPMWebhook(integration as never, { type: 'card_moved' }, registry as never); - - expect(mockRunAgentExecutionPipeline).not.toHaveBeenCalled(); - }); - - it('still clears processing flag when agent throws', async () => { - mockRunAgentExecutionPipeline.mockRejectedValue(new Error('Agent failed')); - const integration = createMockIntegration(); - const registry = createMockRegistry(); - - await processPMWebhook(integration as never, { type: 'card_moved' }, registry as never); - - expect(mockSetProcessing).toHaveBeenCalledWith(false); - }); - it('uses pre-resolved trigger result when provided', async () => { const integration = createMockIntegration(); const registry = createMockRegistry(null); // registry would return null @@ -287,21 +208,6 @@ describe('processPMWebhook', () => { expect(mockRunAgentExecutionPipeline).toHaveBeenCalled(); }); - it('does not set card active when workItemId is undefined', async () => { - const integration = createMockIntegration(); - const registry = { - dispatch: vi.fn().mockResolvedValue({ - agentType: 'implementation', - workItemId: undefined, // no workItemId - agentInput: {}, - }), - }; - - await processPMWebhook(integration as never, { type: 'card_moved' }, registry as never); - - expect(mockSetCardActive).not.toHaveBeenCalled(); - }); - it('skips agent execution when agent-type concurrency is blocked', async () => { vi.mocked(checkAgentTypeConcurrency).mockResolvedValueOnce({ maxConcurrency: 1, diff --git a/tests/unit/server.test.ts b/tests/unit/server.test.ts deleted file mode 100644 index a52a12be..00000000 --- a/tests/unit/server.test.ts +++ /dev/null @@ -1,377 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -// Must mock heavy imports BEFORE importing server module -vi.mock('../../src/router/reactions.js', () => ({ - sendAcknowledgeReaction: vi.fn().mockResolvedValue(undefined), -})); - -vi.mock('../../src/config/provider.js', () => ({ - findProjectByRepo: vi.fn(), -})); - -vi.mock('../../src/github/personas.js', () => ({ - resolvePersonaIdentities: vi.fn(), -})); - -vi.mock('../../src/utils/index.js', () => ({ - canAcceptWebhook: vi.fn().mockReturnValue(true), - isCurrentlyProcessing: vi.fn().mockReturnValue(false), - logger: { - warn: vi.fn(), - info: vi.fn(), - debug: vi.fn(), - error: vi.fn(), - }, -})); - -vi.mock('../../src/utils/webhookLogger.js', () => ({ - logWebhookCall: vi.fn(), -})); - -vi.mock('../../src/api/router.js', () => ({ - appRouter: {}, -})); - -vi.mock('@hono/trpc-server', () => ({ - trpcServer: vi.fn(() => async (_c: unknown, next: () => Promise) => next()), -})); - -vi.mock('../../src/api/auth/login.js', () => ({ - loginHandler: vi.fn(), -})); -vi.mock('../../src/api/auth/logout.js', () => ({ - logoutHandler: vi.fn(), -})); -vi.mock('../../src/api/auth/session.js', () => ({ - resolveUserFromSession: vi.fn().mockResolvedValue(null), -})); -vi.mock('../../src/api/context.js', () => ({ - computeEffectiveOrgId: vi.fn().mockResolvedValue('org-1'), -})); - -import { findProjectByRepo } from '../../src/config/provider.js'; -import { resolvePersonaIdentities } from '../../src/github/personas.js'; -import { sendAcknowledgeReaction } from '../../src/router/reactions.js'; -import { createServer } from '../../src/server.js'; -import type { ServerDependencies } from '../../src/server.js'; - -const mockSendAcknowledgeReaction = vi.mocked(sendAcknowledgeReaction); -const mockFindProjectByRepo = vi.mocked(findProjectByRepo); -const mockResolvePersonaIdentities = vi.mocked(resolvePersonaIdentities); - -function buildDeps(overrides: Partial = {}): ServerDependencies { - return { - config: { - defaults: { - model: 'test-model', - agentModels: {}, - maxIterations: 50, - agentIterations: {}, - watchdogTimeoutMs: 30000, - cardBudgetUsd: 5, - agentBackend: 'llmist', - progressModel: 'test-model', - progressIntervalMinutes: 5, - }, - projects: [ - { - id: 'project-1', - orgId: 'org-1', - name: 'Test Project', - repo: 'owner/repo', - baseBranch: 'main', - branchPrefix: 'feature/', - pm: { type: 'trello' }, - trello: { - boardId: 'board-123', - lists: { splitting: 'l1', planning: 'l2', todo: 'l3' }, - labels: {}, - }, - }, - ], - }, - onTrelloWebhook: vi.fn().mockResolvedValue(undefined), - onGitHubWebhook: vi.fn().mockResolvedValue(undefined), - onJiraWebhook: vi.fn().mockResolvedValue(undefined), - ...overrides, - }; -} - -async function postJson( - app: ReturnType, - path: string, - body: unknown, - headers: Record = {}, -): Promise { - const request = new Request(`http://localhost${path}`, { - method: 'POST', - headers: { 'Content-Type': 'application/json', ...headers }, - body: JSON.stringify(body), - }); - return app.fetch(request); -} - -describe('createServer', () => { - describe('Trello webhook', () => { - it('calls sendAcknowledgeReaction for commentCard events', async () => { - vi.useFakeTimers(); - const deps = buildDeps(); - const app = createServer(deps); - - const payload = { - model: { id: 'board-123', name: 'Board' }, - action: { - id: 'action-1', - type: 'commentCard', - data: { text: 'hello', card: { id: 'c1' } }, - }, - }; - - const response = await postJson(app, '/trello/webhook', payload); - expect(response.status).toBe(200); - - // Allow promises to resolve - await vi.runAllTimersAsync(); - - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith('trello', 'project-1', payload); - vi.useRealTimers(); - }); - - it('does not call sendAcknowledgeReaction for non-comment Trello events', async () => { - vi.useFakeTimers(); - const deps = buildDeps(); - const app = createServer(deps); - - const payload = { - model: { id: 'board-123', name: 'Board' }, - action: { - id: 'action-1', - type: 'updateCard', - data: {}, - }, - }; - - const response = await postJson(app, '/trello/webhook', payload); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); - - it('does not call sendAcknowledgeReaction when board is not configured', async () => { - vi.useFakeTimers(); - const deps = buildDeps(); - const app = createServer(deps); - - const payload = { - model: { id: 'unknown-board', name: 'Board' }, - action: { - id: 'action-1', - type: 'commentCard', - data: { text: 'hello' }, - }, - }; - - const response = await postJson(app, '/trello/webhook', payload); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); - }); - - describe('GitHub webhook', () => { - it('calls sendAcknowledgeReaction for issue_comment events', async () => { - vi.useFakeTimers(); - const deps = buildDeps(); - const app = createServer(deps); - - // Mock project resolution - const mockProject = deps.config.projects[0]; - mockFindProjectByRepo.mockResolvedValue(mockProject); - mockResolvePersonaIdentities.mockResolvedValue({ - implementer: 'bot-implementer', - reviewer: 'bot-reviewer', - }); - - const payload = { - action: 'created', - issue: { number: 1 }, - comment: { id: 42, body: 'hello' }, - repository: { full_name: 'owner/repo' }, - }; - - const response = await postJson(app, '/github/webhook', payload, { - 'X-GitHub-Event': 'issue_comment', - }); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockFindProjectByRepo).toHaveBeenCalledWith('owner/repo'); - expect(mockResolvePersonaIdentities).toHaveBeenCalledWith('project-1'); - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith( - 'github', - 'owner/repo', - payload, - { implementer: 'bot-implementer', reviewer: 'bot-reviewer' }, - mockProject, - ); - vi.useRealTimers(); - }); - - it('calls sendAcknowledgeReaction for pull_request_review_comment events', async () => { - vi.useFakeTimers(); - const deps = buildDeps(); - const app = createServer(deps); - - // Mock project resolution - const mockProject = deps.config.projects[0]; - mockFindProjectByRepo.mockResolvedValue(mockProject); - mockResolvePersonaIdentities.mockResolvedValue({ - implementer: 'bot-implementer', - reviewer: 'bot-reviewer', - }); - - const payload = { - action: 'created', - comment: { id: 99, body: 'review comment' }, - pull_request: { number: 5 }, - repository: { full_name: 'owner/repo' }, - }; - - const response = await postJson(app, '/github/webhook', payload, { - 'X-GitHub-Event': 'pull_request_review_comment', - }); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockFindProjectByRepo).toHaveBeenCalledWith('owner/repo'); - expect(mockResolvePersonaIdentities).toHaveBeenCalledWith('project-1'); - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith( - 'github', - 'owner/repo', - payload, - { implementer: 'bot-implementer', reviewer: 'bot-reviewer' }, - mockProject, - ); - vi.useRealTimers(); - }); - - it('does not call sendAcknowledgeReaction for non-comment GitHub events', async () => { - vi.useFakeTimers(); - const deps = buildDeps(); - const app = createServer(deps); - - const payload = { - action: 'completed', - check_suite: { id: 1, conclusion: 'success' }, - repository: { full_name: 'owner/repo' }, - }; - - const response = await postJson(app, '/github/webhook', payload, { - 'X-GitHub-Event': 'check_suite', - }); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); - }); - - describe('JIRA webhook', () => { - it('calls sendAcknowledgeReaction for comment_created events', async () => { - vi.useFakeTimers(); - const deps = buildDeps({ - config: { - defaults: buildDeps().config.defaults, - projects: [ - { - id: 'jira-project-1', - orgId: 'org-1', - name: 'JIRA Project', - repo: 'owner/repo', - baseBranch: 'main', - branchPrefix: 'feature/', - pm: { type: 'jira' }, - jira: { - projectKey: 'PROJ', - baseUrl: 'https://company.atlassian.net', - statuses: {}, - }, - }, - ], - }, - }); - const app = createServer(deps); - - const payload = { - webhookEvent: 'comment_created', - issue: { - id: '10001', - key: 'PROJ-1', - fields: { project: { key: 'PROJ' } }, - }, - comment: { id: '20001', body: { type: 'doc' } }, - }; - - const response = await postJson(app, '/jira/webhook', payload); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith('jira', 'jira-project-1', payload); - vi.useRealTimers(); - }); - - it('does not call sendAcknowledgeReaction for non-comment JIRA events', async () => { - vi.useFakeTimers(); - const deps = buildDeps({ - config: { - defaults: buildDeps().config.defaults, - projects: [ - { - id: 'jira-project-1', - orgId: 'org-1', - name: 'JIRA Project', - repo: 'owner/repo', - baseBranch: 'main', - branchPrefix: 'feature/', - pm: { type: 'jira' }, - jira: { - projectKey: 'PROJ', - baseUrl: 'https://company.atlassian.net', - statuses: {}, - }, - }, - ], - }, - }); - const app = createServer(deps); - - const payload = { - webhookEvent: 'jira:issue_updated', - issue: { - id: '10001', - key: 'PROJ-1', - fields: { project: { key: 'PROJ' } }, - }, - }; - - const response = await postJson(app, '/jira/webhook', payload); - expect(response.status).toBe(200); - - await vi.runAllTimersAsync(); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); - }); -}); diff --git a/tests/unit/server/webhookReactionSender.test.ts b/tests/unit/server/webhookReactionSender.test.ts deleted file mode 100644 index d84a7fa1..00000000 --- a/tests/unit/server/webhookReactionSender.test.ts +++ /dev/null @@ -1,278 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -const { - mockFindProjectByRepo, - mockResolvePersonaIdentities, - mockSendAcknowledgeReaction, - mockLogger, -} = vi.hoisted(() => ({ - mockFindProjectByRepo: vi.fn(), - mockResolvePersonaIdentities: vi.fn(), - mockSendAcknowledgeReaction: vi.fn(), - mockLogger: { - info: vi.fn(), - warn: vi.fn(), - debug: vi.fn(), - error: vi.fn(), - }, -})); - -vi.mock('../../../src/config/provider.js', () => ({ - findProjectByRepo: mockFindProjectByRepo, -})); - -vi.mock('../../../src/github/personas.js', () => ({ - resolvePersonaIdentities: mockResolvePersonaIdentities, -})); - -vi.mock('../../../src/router/reactions.js', () => ({ - sendAcknowledgeReaction: mockSendAcknowledgeReaction, -})); - -vi.mock('../../../src/utils/index.js', () => ({ - logger: mockLogger, -})); - -import { buildReactionSender } from '../../../src/server/webhookReactionSender.js'; -import type { CascadeConfig } from '../../../src/types/index.js'; - -function makeConfig(projectOverrides: Record = {}): CascadeConfig { - return { - projects: [ - { - id: 'project-1', - name: 'Test Project', - repo: 'owner/repo', - baseBranch: 'main', - trello: { boardId: 'board-123', lists: {}, labels: {} }, - jira: { projectKey: 'PROJ' }, - ...projectOverrides, - }, - ], - } as unknown as CascadeConfig; -} - -function makeTrelloPayload(boardId = 'board-123') { - return { - model: { id: boardId }, - action: { type: 'commentCard', data: {} }, - }; -} - -function makeGitHubPayload(repoFullName = 'owner/repo') { - return { - repository: { full_name: repoFullName }, - action: 'created', - }; -} - -function makeJiraPayload(projectKey = 'PROJ') { - return { - webhookEvent: 'comment_created', - issue: { - key: `${projectKey}-1`, - fields: { - project: { key: projectKey }, - }, - }, - }; -} - -describe('buildReactionSender - trello', () => { - beforeEach(() => { - vi.resetAllMocks(); - mockSendAcknowledgeReaction.mockResolvedValue(undefined); - }); - - it('throws when config is not provided for trello', () => { - expect(() => buildReactionSender('trello')).toThrow( - 'buildReactionSender: config required for trello', - ); - }); - - it('reacts on commentCard events', async () => { - const sender = buildReactionSender('trello', makeConfig()); - - sender(makeTrelloPayload(), 'commentCard'); - - // Allow microtasks to run - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith( - 'trello', - 'project-1', - expect.objectContaining({ model: { id: 'board-123' } }), - ); - }); - - it('does not react on non-commentCard events', async () => { - const sender = buildReactionSender('trello', makeConfig()); - - sender(makeTrelloPayload(), 'updateCard'); - - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); - - it('does not react when no project found for boardId', async () => { - const config = makeConfig({ trello: { boardId: 'different-board', lists: {}, labels: {} } }); - const sender = buildReactionSender('trello', config); - - sender(makeTrelloPayload('no-match-board'), 'commentCard'); - - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); -}); - -describe('buildReactionSender - github', () => { - beforeEach(() => { - vi.resetAllMocks(); - mockSendAcknowledgeReaction.mockResolvedValue(undefined); - mockFindProjectByRepo.mockResolvedValue({ - id: 'project-1', - name: 'Test Project', - }); - mockResolvePersonaIdentities.mockResolvedValue({ - implementer: 'impl-bot', - reviewer: 'review-bot', - }); - }); - - it('does not require config', () => { - expect(() => buildReactionSender('github')).not.toThrow(); - }); - - it('reacts on issue_comment events', async () => { - const sender = buildReactionSender('github'); - - sender(makeGitHubPayload(), 'issue_comment'); - - await new Promise((r) => setTimeout(r, 50)); - - expect(mockFindProjectByRepo).toHaveBeenCalledWith('owner/repo'); - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith( - 'github', - 'owner/repo', - expect.any(Object), - expect.any(Object), - expect.any(Object), - ); - }); - - it('reacts on pull_request_review_comment events', async () => { - const sender = buildReactionSender('github'); - - sender(makeGitHubPayload(), 'pull_request_review_comment'); - - await new Promise((r) => setTimeout(r, 50)); - - expect(mockSendAcknowledgeReaction).toHaveBeenCalled(); - }); - - it('does not react on other event types', async () => { - const sender = buildReactionSender('github'); - - sender(makeGitHubPayload(), 'push'); - - await new Promise((r) => setTimeout(r, 50)); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); - - it('logs warning when no project found for repo', async () => { - mockFindProjectByRepo.mockResolvedValue(null); - const sender = buildReactionSender('github'); - - sender(makeGitHubPayload(), 'issue_comment'); - - await new Promise((r) => setTimeout(r, 50)); - - expect(mockLogger.warn).toHaveBeenCalledWith( - expect.stringContaining('No project found for repo'), - expect.any(Object), - ); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); - - it('logs error when reaction fails', async () => { - mockFindProjectByRepo.mockResolvedValue({ id: 'p1', name: 'P1' }); - mockSendAcknowledgeReaction.mockRejectedValue(new Error('Network error')); - const sender = buildReactionSender('github'); - - sender(makeGitHubPayload(), 'issue_comment'); - - await new Promise((r) => setTimeout(r, 50)); - - expect(mockLogger.error).toHaveBeenCalledWith( - '[Server] GitHub reaction error:', - expect.any(Object), - ); - }); -}); - -describe('buildReactionSender - jira', () => { - beforeEach(() => { - vi.resetAllMocks(); - mockSendAcknowledgeReaction.mockResolvedValue(undefined); - }); - - it('throws when config is not provided for jira', () => { - expect(() => buildReactionSender('jira')).toThrow( - 'buildReactionSender: config required for jira', - ); - }); - - it('reacts on comment_created events', async () => { - const sender = buildReactionSender('jira', makeConfig()); - - sender(makeJiraPayload(), 'comment_created'); - - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith( - 'jira', - 'project-1', - expect.any(Object), - ); - }); - - it('reacts on comment_updated events', async () => { - const sender = buildReactionSender('jira', makeConfig()); - - sender(makeJiraPayload(), 'comment_updated'); - - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).toHaveBeenCalled(); - }); - - it('does not react on non-comment events', async () => { - const sender = buildReactionSender('jira', makeConfig()); - - sender(makeJiraPayload(), 'jira:issue_updated'); - - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); - - it('does not react when no project found for JIRA key', async () => { - const sender = buildReactionSender('jira', makeConfig({ jira: { projectKey: 'OTHER' } })); - - sender(makeJiraPayload('PROJ'), 'comment_created'); - - await new Promise((r) => setTimeout(r, 10)); - - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); -}); - -describe('buildReactionSender - unknown source', () => { - it('returns a no-op function for unknown sources', () => { - const sender = buildReactionSender('slack'); - expect(() => sender({}, 'some_event')).not.toThrow(); - }); -}); diff --git a/tests/unit/triggers/github-webhook-handler.test.ts b/tests/unit/triggers/github-webhook-handler.test.ts index b68a09c7..28af1c0f 100644 --- a/tests/unit/triggers/github-webhook-handler.test.ts +++ b/tests/unit/triggers/github-webhook-handler.test.ts @@ -61,10 +61,6 @@ vi.mock('../../../src/triggers/shared/webhook-execution.js', () => ({ runAgentWithCredentials: vi.fn().mockResolvedValue(undefined), })); -vi.mock('../../../src/triggers/shared/webhook-queue.js', () => ({ - processNextQueuedWebhook: vi.fn(), -})); - vi.mock('../../../src/triggers/github/ack-comments.js', () => ({ postAcknowledgmentComment: vi.fn().mockResolvedValue(undefined), updateInitialCommentWithError: vi.fn().mockResolvedValue(undefined), @@ -82,19 +78,12 @@ vi.mock('../../../src/router/agent-type-lock.js', () => ({ })); vi.mock('../../../src/utils/index.js', () => ({ - clearCardActive: vi.fn(), - enqueueWebhook: vi.fn().mockReturnValue(true), - getQueueLength: vi.fn().mockReturnValue(0), - isCardActive: vi.fn().mockReturnValue(false), - isCurrentlyProcessing: vi.fn().mockReturnValue(false), logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), }, - setCardActive: vi.fn(), - setProcessing: vi.fn(), startWatchdog: vi.fn(), })); @@ -104,23 +93,9 @@ import { postAcknowledgmentComment } from '../../../src/triggers/github/ack-comm import { pollWaitForChecks } from '../../../src/triggers/github/check-polling.js'; import { processGitHubWebhook } from '../../../src/triggers/github/webhook-handler.js'; import { runAgentWithCredentials } from '../../../src/triggers/shared/webhook-execution.js'; -import { - clearCardActive, - enqueueWebhook, - isCardActive, - isCurrentlyProcessing, - setCardActive, - setProcessing, - startWatchdog, -} from '../../../src/utils/index.js'; - -const mockIsCurrentlyProcessing = vi.mocked(isCurrentlyProcessing); -const mockIsCardActive = vi.mocked(isCardActive); -const mockEnqueueWebhook = vi.mocked(enqueueWebhook); -const mockSetProcessing = vi.mocked(setProcessing); +import { startWatchdog } from '../../../src/utils/index.js'; + const mockStartWatchdog = vi.mocked(startWatchdog); -const mockSetCardActive = vi.mocked(setCardActive); -const mockClearCardActive = vi.mocked(clearCardActive); const mockRunAgentWithCredentials = vi.mocked(runAgentWithCredentials); const mockPostAckComment = vi.mocked(postAcknowledgmentComment); @@ -143,9 +118,6 @@ const validPayload = { beforeEach(() => { vi.clearAllMocks(); - mockIsCurrentlyProcessing.mockReturnValue(false); - mockIsCardActive.mockReturnValue(false); - mockEnqueueWebhook.mockReturnValue(true); mockRunAgentWithCredentials.mockResolvedValue(undefined); }); @@ -168,16 +140,6 @@ describe('processGitHubWebhook', () => { // This test just verifies the handler doesn't crash on minimal payload. }); - it('enqueues webhook when currently processing', async () => { - mockIsCurrentlyProcessing.mockReturnValue(true); - const registry = createMockRegistry(); - - await processGitHubWebhook(validPayload, 'pull_request', registry as never); - - expect(mockEnqueueWebhook).toHaveBeenCalled(); - expect(registry.dispatch).not.toHaveBeenCalled(); - }); - it('dispatches to trigger registry when project found', async () => { const registry = createMockRegistry(); await processGitHubWebhook(validPayload, 'pull_request', registry as never); @@ -190,39 +152,12 @@ describe('processGitHubWebhook', () => { expect(mockRunAgentWithCredentials).toHaveBeenCalled(); }); - it('sets processing to true on start and false when done', async () => { - const registry = createMockRegistry(); - await processGitHubWebhook(validPayload, 'pull_request', registry as never); - expect(mockSetProcessing).toHaveBeenCalledWith(true); - expect(mockSetProcessing).toHaveBeenCalledWith(false); - }); - it('starts watchdog on trigger match', async () => { const registry = createMockRegistry(); await processGitHubWebhook(validPayload, 'pull_request', registry as never); expect(mockStartWatchdog).toHaveBeenCalledWith(120000); }); - it('sets and clears card active when workItemId is present', async () => { - const registry = createMockRegistry('implementation', 'card-abc'); - await processGitHubWebhook(validPayload, 'pull_request', registry as never); - expect(mockSetCardActive).toHaveBeenCalledWith('card-abc'); - expect(mockClearCardActive).toHaveBeenCalledWith('card-abc'); - }); - - it('does not set card active when workItemId is undefined', async () => { - const registry = createMockRegistry('implementation', undefined); - await processGitHubWebhook(validPayload, 'pull_request', registry as never); - expect(mockSetCardActive).not.toHaveBeenCalled(); - }); - - it('skips agent execution when work item is already active', async () => { - mockIsCardActive.mockReturnValue(true); - const registry = createMockRegistry('implementation', 'card-abc'); - await processGitHubWebhook(validPayload, 'pull_request', registry as never); - expect(mockRunAgentWithCredentials).not.toHaveBeenCalled(); - }); - it('posts ack comment when no ackCommentId provided', async () => { const registry = createMockRegistry(); await processGitHubWebhook(validPayload, 'pull_request', registry as never); @@ -274,13 +209,6 @@ describe('processGitHubWebhook', () => { expect(mockRunAgentWithCredentials).toHaveBeenCalled(); }); - it('still clears processing when agent throws', async () => { - mockRunAgentWithCredentials.mockRejectedValue(new Error('Agent failed')); - const registry = createMockRegistry(); - await processGitHubWebhook(validPayload, 'pull_request', registry as never); - expect(mockSetProcessing).toHaveBeenCalledWith(false); - }); - it('skips agent execution when agent-type concurrency is blocked', async () => { vi.mocked(checkAgentTypeConcurrency).mockResolvedValueOnce({ maxConcurrency: 1, @@ -301,7 +229,6 @@ describe('processGitHubWebhook', () => { }; await processGitHubWebhook(validPayload, 'pull_request', registry as never); expect(mockRunAgentWithCredentials).not.toHaveBeenCalled(); - expect(mockSetProcessing).not.toHaveBeenCalled(); }); it('deletes ack comment when pollWaitForChecks returns false', async () => { diff --git a/tests/unit/triggers/webhook-queue.test.ts b/tests/unit/triggers/webhook-queue.test.ts deleted file mode 100644 index 308fb7e3..00000000 --- a/tests/unit/triggers/webhook-queue.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { processNextQueuedWebhook } from '../../../src/triggers/shared/webhook-queue.js'; -import { clearQueue, enqueueWebhook } from '../../../src/utils/webhookQueue.js'; - -describe('processNextQueuedWebhook', () => { - afterEach(() => { - clearQueue(); - }); - - it('does nothing when queue is empty', () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - - processNextQueuedWebhook(processWebhook, 'Test'); - - expect(processWebhook).not.toHaveBeenCalled(); - }); - - it('forwards payload and eventType to processWebhook', async () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - enqueueWebhook({ action: 'test' }, 'issue_comment'); - - processNextQueuedWebhook(processWebhook, 'Test'); - - // processWebhook is called via setImmediate — wait for it - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith( - { action: 'test' }, - undefined, // eventType comes from getEventType, not the queued entry - undefined, // no ackCommentId - undefined, // no ackMessage - ); - }); - - it('uses getEventType to extract event type from queued entry', async () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - enqueueWebhook({ action: 'test' }, 'pull_request'); - - processNextQueuedWebhook(processWebhook, 'Test', (entry) => entry.eventType); - - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith( - { action: 'test' }, - 'pull_request', - undefined, - undefined, - ); - }); - - it('forwards ackCommentId through the queue', async () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-123'); - - processNextQueuedWebhook(processWebhook, 'Test', (entry) => entry.eventType); - - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith( - { action: 'test' }, - 'issue_comment', - 'ack-123', - undefined, - ); - }); - - it('forwards numeric ackCommentId through the queue', async () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - enqueueWebhook({ action: 'test' }, undefined, 10646); - - processNextQueuedWebhook(processWebhook, 'Test'); - - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith({ action: 'test' }, undefined, 10646, undefined); - }); - - it('forwards ackMessage through the queue', async () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-123', 'Looking into it...'); - - processNextQueuedWebhook(processWebhook, 'Test', (entry) => entry.eventType); - - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith( - { action: 'test' }, - 'issue_comment', - 'ack-123', - 'Looking into it...', - ); - }); - - it('processes items in FIFO order preserving ackCommentId', async () => { - const processWebhook = vi.fn().mockResolvedValue(undefined); - enqueueWebhook({ order: 1 }, undefined, 'first-ack'); - enqueueWebhook({ order: 2 }, undefined, 'second-ack'); - - // Process first item - processNextQueuedWebhook(processWebhook, 'Test'); - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith({ order: 1 }, undefined, 'first-ack', undefined); - - // Process second item - processNextQueuedWebhook(processWebhook, 'Test'); - await new Promise((resolve) => setImmediate(resolve)); - - expect(processWebhook).toHaveBeenCalledWith({ order: 2 }, undefined, 'second-ack', undefined); - }); -}); diff --git a/tests/unit/utils/activeCards.test.ts b/tests/unit/utils/activeCards.test.ts deleted file mode 100644 index ab12e0e5..00000000 --- a/tests/unit/utils/activeCards.test.ts +++ /dev/null @@ -1,120 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest'; -import { - clearAllActiveCards, - clearCardActive, - getActiveCardCount, - isCardActive, - setCardActive, -} from '../../../src/utils/activeCards.js'; - -describe('activeCards', () => { - afterEach(() => { - clearAllActiveCards(); - }); - - describe('setCardActive', () => { - it('marks a card as active', () => { - setCardActive('card-123'); - - expect(isCardActive('card-123')).toBe(true); - }); - - it('can mark multiple cards as active', () => { - setCardActive('card-1'); - setCardActive('card-2'); - setCardActive('card-3'); - - expect(isCardActive('card-1')).toBe(true); - expect(isCardActive('card-2')).toBe(true); - expect(isCardActive('card-3')).toBe(true); - }); - - it('is idempotent for the same card', () => { - setCardActive('card-123'); - setCardActive('card-123'); - - expect(getActiveCardCount()).toBe(1); - }); - }); - - describe('isCardActive', () => { - it('returns false for inactive cards', () => { - expect(isCardActive('card-123')).toBe(false); - }); - - it('returns true for active cards', () => { - setCardActive('card-123'); - - expect(isCardActive('card-123')).toBe(true); - }); - }); - - describe('clearCardActive', () => { - it('removes a card from the active set', () => { - setCardActive('card-123'); - - clearCardActive('card-123'); - - expect(isCardActive('card-123')).toBe(false); - }); - - it('does not throw for non-existent cards', () => { - expect(() => clearCardActive('non-existent')).not.toThrow(); - }); - - it('only removes the specified card', () => { - setCardActive('card-1'); - setCardActive('card-2'); - - clearCardActive('card-1'); - - expect(isCardActive('card-1')).toBe(false); - expect(isCardActive('card-2')).toBe(true); - }); - }); - - describe('getActiveCardCount', () => { - it('returns 0 when no cards are active', () => { - expect(getActiveCardCount()).toBe(0); - }); - - it('tracks the number of active cards', () => { - setCardActive('card-1'); - expect(getActiveCardCount()).toBe(1); - - setCardActive('card-2'); - expect(getActiveCardCount()).toBe(2); - - clearCardActive('card-1'); - expect(getActiveCardCount()).toBe(1); - - clearCardActive('card-2'); - expect(getActiveCardCount()).toBe(0); - }); - }); - - describe('clearAllActiveCards', () => { - it('removes all active cards', () => { - setCardActive('card-1'); - setCardActive('card-2'); - setCardActive('card-3'); - - clearAllActiveCards(); - - expect(getActiveCardCount()).toBe(0); - expect(isCardActive('card-1')).toBe(false); - expect(isCardActive('card-2')).toBe(false); - expect(isCardActive('card-3')).toBe(false); - }); - - it('allows new cards to be added after clearing', () => { - setCardActive('old-card'); - clearAllActiveCards(); - - setCardActive('new-card'); - - expect(isCardActive('new-card')).toBe(true); - expect(getActiveCardCount()).toBe(1); - }); - }); -}); diff --git a/tests/unit/utils/lifecycle.test.ts b/tests/unit/utils/lifecycle.test.ts index 20b1bedd..19c4de00 100644 --- a/tests/unit/utils/lifecycle.test.ts +++ b/tests/unit/utils/lifecycle.test.ts @@ -16,8 +16,6 @@ import { flush } from '../../../src/sentry.js'; import { clearWatchdog, clearWatchdogCleanup, - isCurrentlyProcessing, - setProcessing, setWatchdogCleanup, startWatchdog, } from '../../../src/utils/lifecycle.js'; @@ -34,28 +32,10 @@ describe('lifecycle', () => { // Clean up all timers clearWatchdog(); clearWatchdogCleanup(); - setProcessing(false); vi.useRealTimers(); vi.restoreAllMocks(); }); - describe('processing state', () => { - it('defaults to not processing', () => { - expect(isCurrentlyProcessing()).toBe(false); - }); - - it('can set processing to true', () => { - setProcessing(true); - expect(isCurrentlyProcessing()).toBe(true); - }); - - it('can set processing back to false', () => { - setProcessing(true); - setProcessing(false); - expect(isCurrentlyProcessing()).toBe(false); - }); - }); - describe('watchdog', () => { it('force exits after timeout', async () => { startWatchdog(30000); diff --git a/tests/unit/utils/webhookQueue.test.ts b/tests/unit/utils/webhookQueue.test.ts deleted file mode 100644 index 4c36c098..00000000 --- a/tests/unit/utils/webhookQueue.test.ts +++ /dev/null @@ -1,201 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest'; -import { - clearQueue, - dequeueWebhook, - enqueueWebhook, - getMaxQueueSize, - getQueueLength, -} from '../../../src/utils/webhookQueue.js'; - -describe('webhookQueue', () => { - afterEach(() => { - clearQueue(); - }); - - describe('enqueueWebhook', () => { - it('adds a webhook to the queue', () => { - const payload = { action: { type: 'test' } }; - - const result = enqueueWebhook(payload); - - expect(result).toBe(true); - expect(getQueueLength()).toBe(1); - }); - - it('accepts multiple webhooks up to the limit', () => { - const maxSize = getMaxQueueSize(); - - for (let i = 0; i < maxSize; i++) { - const result = enqueueWebhook({ index: i }); - expect(result).toBe(true); - } - - expect(getQueueLength()).toBe(maxSize); - }); - - it('rejects webhooks when queue is full', () => { - const maxSize = getMaxQueueSize(); - - // Fill the queue - for (let i = 0; i < maxSize; i++) { - enqueueWebhook({ index: i }); - } - - // Try to add one more - const result = enqueueWebhook({ index: maxSize }); - - expect(result).toBe(false); - expect(getQueueLength()).toBe(maxSize); - }); - }); - - describe('dequeueWebhook', () => { - it('returns undefined for empty queue', () => { - const result = dequeueWebhook(); - - expect(result).toBeUndefined(); - }); - - it('returns and removes the first item (FIFO)', () => { - enqueueWebhook({ value: 'first' }); - enqueueWebhook({ value: 'second' }); - enqueueWebhook({ value: 'third' }); - - const first = dequeueWebhook(); - const second = dequeueWebhook(); - const third = dequeueWebhook(); - - expect(first?.payload).toEqual({ value: 'first' }); - expect(second?.payload).toEqual({ value: 'second' }); - expect(third?.payload).toEqual({ value: 'third' }); - expect(getQueueLength()).toBe(0); - }); - - it('includes receivedAt timestamp', () => { - const before = new Date(); - enqueueWebhook({ test: true }); - const after = new Date(); - - const item = dequeueWebhook(); - - expect(item).toBeDefined(); - expect(item?.receivedAt.getTime()).toBeGreaterThanOrEqual(before.getTime()); - expect(item?.receivedAt.getTime()).toBeLessThanOrEqual(after.getTime()); - }); - }); - - describe('getQueueLength', () => { - it('returns 0 for empty queue', () => { - expect(getQueueLength()).toBe(0); - }); - - it('tracks queue size correctly', () => { - enqueueWebhook({ a: 1 }); - expect(getQueueLength()).toBe(1); - - enqueueWebhook({ b: 2 }); - expect(getQueueLength()).toBe(2); - - dequeueWebhook(); - expect(getQueueLength()).toBe(1); - - dequeueWebhook(); - expect(getQueueLength()).toBe(0); - }); - }); - - describe('clearQueue', () => { - it('removes all items from the queue', () => { - enqueueWebhook({ a: 1 }); - enqueueWebhook({ b: 2 }); - enqueueWebhook({ c: 3 }); - - clearQueue(); - - expect(getQueueLength()).toBe(0); - expect(dequeueWebhook()).toBeUndefined(); - }); - - it('allows new items after clearing', () => { - enqueueWebhook({ old: true }); - clearQueue(); - - const result = enqueueWebhook({ new: true }); - - expect(result).toBe(true); - expect(getQueueLength()).toBe(1); - expect(dequeueWebhook()?.payload).toEqual({ new: true }); - }); - }); - - describe('ackCommentId', () => { - it('preserves string ackCommentId through enqueue/dequeue', () => { - enqueueWebhook({ action: 'test' }, undefined, 'comment-42'); - - const item = dequeueWebhook(); - - expect(item?.ackCommentId).toBe('comment-42'); - }); - - it('preserves numeric ackCommentId through enqueue/dequeue', () => { - enqueueWebhook({ action: 'test' }, undefined, 10646); - - const item = dequeueWebhook(); - - expect(item?.ackCommentId).toBe(10646); - }); - - it('defaults ackCommentId to undefined when not provided', () => { - enqueueWebhook({ action: 'test' }); - - const item = dequeueWebhook(); - - expect(item?.ackCommentId).toBeUndefined(); - }); - - it('preserves ackCommentId alongside eventType', () => { - enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-99'); - - const item = dequeueWebhook(); - - expect(item?.eventType).toBe('issue_comment'); - expect(item?.ackCommentId).toBe('ack-99'); - }); - }); - - describe('ackMessage', () => { - it('preserves ackMessage through enqueue/dequeue', () => { - enqueueWebhook({ action: 'test' }, 'issue_comment', 'ack-1', 'Looking into it...'); - - const item = dequeueWebhook(); - - expect(item?.ackMessage).toBe('Looking into it...'); - }); - - it('defaults ackMessage to undefined when not provided', () => { - enqueueWebhook({ action: 'test' }, undefined, 'ack-1'); - - const item = dequeueWebhook(); - - expect(item?.ackMessage).toBeUndefined(); - }); - - it('preserves ackMessage alongside ackCommentId and eventType', () => { - enqueueWebhook({ action: 'test' }, 'pull_request', 42, 'On it — checking the PR...'); - - const item = dequeueWebhook(); - - expect(item?.eventType).toBe('pull_request'); - expect(item?.ackCommentId).toBe(42); - expect(item?.ackMessage).toBe('On it — checking the PR...'); - }); - }); - - describe('getMaxQueueSize', () => { - it('returns the maximum queue size', () => { - const maxSize = getMaxQueueSize(); - - expect(maxSize).toBe(10); - }); - }); -}); diff --git a/tests/unit/server/webhookHandlers.test.ts b/tests/unit/webhook/webhookHandlers.test.ts similarity index 50% rename from tests/unit/server/webhookHandlers.test.ts rename to tests/unit/webhook/webhookHandlers.test.ts index c80bd88c..9b49cb5c 100644 --- a/tests/unit/server/webhookHandlers.test.ts +++ b/tests/unit/webhook/webhookHandlers.test.ts @@ -2,21 +2,7 @@ import { Hono } from 'hono'; import { beforeEach, describe, expect, it, vi } from 'vitest'; // Must mock heavy imports BEFORE importing the module under test -vi.mock('../../../src/router/reactions.js', () => ({ - sendAcknowledgeReaction: vi.fn().mockResolvedValue(undefined), -})); - -vi.mock('../../../src/config/provider.js', () => ({ - findProjectByRepo: vi.fn(), -})); - -vi.mock('../../../src/github/personas.js', () => ({ - resolvePersonaIdentities: vi.fn(), -})); - vi.mock('../../../src/utils/index.js', () => ({ - canAcceptWebhook: vi.fn().mockReturnValue(true), - isCurrentlyProcessing: vi.fn().mockReturnValue(false), logger: { warn: vi.fn(), info: vi.fn(), @@ -33,27 +19,17 @@ vi.mock('../../../src/sentry.js', () => ({ captureException: vi.fn(), })); -import { findProjectByRepo } from '../../../src/config/provider.js'; -import { resolvePersonaIdentities } from '../../../src/github/personas.js'; -import { sendAcknowledgeReaction } from '../../../src/router/reactions.js'; import { captureException } from '../../../src/sentry.js'; +import { logWebhookCall } from '../../../src/utils/webhookLogger.js'; import { createWebhookHandler, parseGitHubPayload, parseJiraPayload, parseTrelloPayload, -} from '../../../src/server/webhookHandlers.js'; -import { buildReactionSender } from '../../../src/server/webhookReactionSender.js'; -import { canAcceptWebhook, isCurrentlyProcessing } from '../../../src/utils/index.js'; -import { logWebhookCall } from '../../../src/utils/webhookLogger.js'; +} from '../../../src/webhook/webhookHandlers.js'; const mockCaptureException = vi.mocked(captureException); const mockLogWebhookCall = vi.mocked(logWebhookCall); -const mockIsCurrentlyProcessing = vi.mocked(isCurrentlyProcessing); -const mockCanAcceptWebhook = vi.mocked(canAcceptWebhook); -const mockSendAcknowledgeReaction = vi.mocked(sendAcknowledgeReaction); -const mockFindProjectByRepo = vi.mocked(findProjectByRepo); -const mockResolvePersonaIdentities = vi.mocked(resolvePersonaIdentities); /** Build a minimal Hono app with the handler mounted at POST /webhook */ function buildApp(handler: ReturnType): Hono { @@ -76,45 +52,12 @@ async function postJson( } // --------------------------------------------------------------------------- -// createWebhookHandler — core factory behaviour +// createWebhookHandler — router mode (always awaits processing) // --------------------------------------------------------------------------- describe('createWebhookHandler', () => { beforeEach(() => { - mockIsCurrentlyProcessing.mockReturnValue(false); - mockCanAcceptWebhook.mockReturnValue(true); - }); - - it('returns 503 when at capacity (checkCapacity=true)', async () => { - mockIsCurrentlyProcessing.mockReturnValue(true); - mockCanAcceptWebhook.mockReturnValue(false); - - const handler = createWebhookHandler({ - source: 'trello', - parsePayload: async () => ({ ok: true, payload: {}, eventType: 'test' }), - processWebhook: vi.fn().mockResolvedValue(undefined), - checkCapacity: true, - }); - - const app = buildApp(handler); - const res = await postJson(app, {}); - expect(res.status).toBe(503); - }); - - it('does NOT return 503 when checkCapacity=false even at capacity', async () => { - mockIsCurrentlyProcessing.mockReturnValue(true); - mockCanAcceptWebhook.mockReturnValue(false); - - const handler = createWebhookHandler({ - source: 'trello', - checkCapacity: false, - parsePayload: async () => ({ ok: true, payload: {}, eventType: 'test' }), - processWebhook: vi.fn().mockResolvedValue(undefined), - }); - - const app = buildApp(handler); - const res = await postJson(app, {}); - expect(res.status).toBe(200); + vi.clearAllMocks(); }); it('returns 400 when parsePayload fails', async () => { @@ -160,9 +103,11 @@ describe('createWebhookHandler', () => { ); }); - it('calls processWebhook asynchronously via setImmediate', async () => { - vi.useFakeTimers(); - const processWebhook = vi.fn().mockResolvedValue(undefined); + it('awaits processWebhook before responding', async () => { + const callOrder: string[] = []; + const processWebhook = vi.fn().mockImplementation(async () => { + callOrder.push('process'); + }); const handler = createWebhookHandler({ source: 'trello', parsePayload: async () => ({ ok: true, payload: { x: 1 }, eventType: 'commentCard' }), @@ -170,14 +115,12 @@ describe('createWebhookHandler', () => { }); const app = buildApp(handler); - await postJson(app, { x: 1 }); - - // Not yet called — setImmediate hasn't fired - expect(processWebhook).not.toHaveBeenCalled(); + const res = await postJson(app, { x: 1 }); - await vi.runAllTimersAsync(); + // processWebhook was called synchronously before response + expect(res.status).toBe(200); expect(processWebhook).toHaveBeenCalledWith({ x: 1 }, 'commentCard'); - vi.useRealTimers(); + expect(callOrder).toEqual(['process']); }); it('calls sendReaction when provided and parse succeeds', async () => { @@ -210,31 +153,9 @@ describe('createWebhookHandler', () => { expect(sendReaction).not.toHaveBeenCalled(); }); - it('awaits processWebhook when fireAndForget=false', async () => { - const callOrder: string[] = []; - const processWebhook = vi.fn().mockImplementation(async () => { - callOrder.push('process'); - }); - const handler = createWebhookHandler({ - source: 'trello', - fireAndForget: false, - parsePayload: async () => ({ ok: true, payload: { x: 1 }, eventType: 'commentCard' }), - processWebhook, - }); - - const app = buildApp(handler); - const res = await postJson(app, { x: 1 }); - - // processWebhook was called synchronously before response - expect(res.status).toBe(200); - expect(processWebhook).toHaveBeenCalledWith({ x: 1 }, 'commentCard'); - expect(callOrder).toEqual(['process']); - }); - - it('uses processWebhook return value to enrich log when fireAndForget=false', async () => { + it('uses processWebhook return value to enrich log', async () => { const handler = createWebhookHandler({ source: 'trello', - fireAndForget: false, parsePayload: async () => ({ ok: true, payload: { x: 1 }, eventType: 'commentCard' }), processWebhook: vi.fn().mockResolvedValue({ processed: false, projectId: 'proj-123' }), }); @@ -251,32 +172,9 @@ describe('createWebhookHandler', () => { ); }); - it('ignores processWebhook return value when fireAndForget=true (logs before processing)', async () => { - vi.useFakeTimers(); - const handler = createWebhookHandler({ - source: 'github', - fireAndForget: true, - parsePayload: async () => ({ ok: true, payload: { y: 2 }, eventType: 'push' }), - processWebhook: vi.fn().mockResolvedValue({ processed: false, projectId: 'proj-456' }), - }); - - const app = buildApp(handler); - await postJson(app, { y: 2 }); - - // In fire-and-forget mode, log happens before processing, so overrides are not available - expect(mockLogWebhookCall).toHaveBeenCalledWith( - expect.objectContaining({ - statusCode: 200, - processed: true, // default, not the override - }), - ); - vi.useRealTimers(); - }); - it('logs processed:true by default when processWebhook returns void', async () => { const handler = createWebhookHandler({ source: 'jira', - fireAndForget: false, parsePayload: async () => ({ ok: true, payload: {}, eventType: 'issue_updated' }), processWebhook: vi.fn().mockResolvedValue(undefined), }); @@ -292,13 +190,11 @@ describe('createWebhookHandler', () => { ); }); - it('log overrides reflect actual processing outcome when fireAndForget=false', async () => { + it('log overrides reflect actual processing outcome', async () => { const handler = createWebhookHandler({ source: 'trello', - fireAndForget: false, parsePayload: async () => ({ ok: true, payload: {}, eventType: 'commentCard' }), processWebhook: async () => { - // Simulate actual processing that determines outcome return { processed: true, projectId: 'proj-789' }; }, }); @@ -314,35 +210,9 @@ describe('createWebhookHandler', () => { ); }); - it('captures processWebhook errors to Sentry in fire-and-forget mode', async () => { - vi.useFakeTimers(); - const processError = new Error('redis connection failed'); - const handler = createWebhookHandler({ - source: 'trello', - fireAndForget: true, - parsePayload: async () => ({ ok: true, payload: {}, eventType: 'commentCard' }), - processWebhook: vi.fn().mockRejectedValue(processError), - }); - - const app = buildApp(handler); - const res = await postJson(app, {}); - // Fire-and-forget always returns 200 - expect(res.status).toBe(200); - - // Let setImmediate fire and the rejection be caught - await vi.runAllTimersAsync(); - - expect(mockCaptureException).toHaveBeenCalledWith( - expect.objectContaining({ message: 'redis connection failed' }), - expect.objectContaining({ tags: { source: 'trello_webhook' } }), - ); - vi.useRealTimers(); - }); - - it('lets processWebhook errors propagate when fireAndForget=false', async () => { + it('propagates processWebhook errors to Hono error handler', async () => { const handler = createWebhookHandler({ source: 'jira', - fireAndForget: false, parsePayload: async () => ({ ok: true, payload: {}, eventType: 'issue_updated' }), processWebhook: vi.fn().mockRejectedValue(new Error('queue connection failed')), }); @@ -359,13 +229,33 @@ describe('createWebhookHandler', () => { const body = await res.text(); expect(body).toContain('queue connection failed'); }); + + it('captures processWebhook errors to Sentry before re-throwing', async () => { + const processError = new Error('redis connection failed'); + const handler = createWebhookHandler({ + source: 'trello', + parsePayload: async () => ({ ok: true, payload: {}, eventType: 'commentCard' }), + processWebhook: vi.fn().mockRejectedValue(processError), + }); + + const app = new Hono(); + app.post('/webhook', handler); + app.onError((_err, c) => c.text('Error', 500)); + + await postJson(app, {}); + + expect(mockCaptureException).toHaveBeenCalledWith( + expect.objectContaining({ message: 'redis connection failed' }), + expect.objectContaining({ tags: { source: 'trello_webhook' } }), + ); + }); }); // --------------------------------------------------------------------------- -// Platform parsers +// Platform parsers (integration tests via Hono) // --------------------------------------------------------------------------- -describe('parseTrelloPayload', () => { +describe('parseTrelloPayload (via createWebhookHandler)', () => { it('extracts eventType from action.type', async () => { const app = new Hono(); app.post('/test', async (c) => { @@ -401,7 +291,7 @@ describe('parseTrelloPayload', () => { }); }); -describe('parseJiraPayload', () => { +describe('parseJiraPayload (via createWebhookHandler)', () => { it('extracts eventType from webhookEvent', async () => { const app = new Hono(); app.post('/test', async (c) => { @@ -420,7 +310,7 @@ describe('parseJiraPayload', () => { }); }); -describe('parseGitHubPayload', () => { +describe('parseGitHubPayload (via createWebhookHandler)', () => { it('extracts eventType from X-GitHub-Event header', async () => { const app = new Hono(); app.post('/test', async (c) => { @@ -441,138 +331,3 @@ describe('parseGitHubPayload', () => { expect(body).toMatchObject({ ok: true, eventType: 'issue_comment' }); }); }); - -// --------------------------------------------------------------------------- -// Unified reaction sender (buildReactionSender) -// --------------------------------------------------------------------------- - -describe('buildReactionSender — trello', () => { - const config = { - defaults: {} as never, - projects: [ - { - id: 'proj-1', - trello: { boardId: 'board-abc' }, - } as never, - ], - }; - - it('sends reaction for commentCard events', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('trello', config); - const payload = { model: { id: 'board-abc' }, action: { type: 'commentCard' } }; - sender(payload, 'commentCard'); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith('trello', 'proj-1', payload); - vi.useRealTimers(); - }); - - it('does not send reaction for non-commentCard events', () => { - const sender = buildReactionSender('trello', config); - sender({ model: { id: 'board-abc' } }, 'updateCard'); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - }); - - it('does not send reaction when board not found', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('trello', config); - sender({ model: { id: 'unknown-board' } }, 'commentCard'); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); -}); - -describe('buildReactionSender — github', () => { - it('sends reaction for issue_comment events', async () => { - vi.useFakeTimers(); - const mockProject = { id: 'proj-1' } as never; - mockFindProjectByRepo.mockResolvedValue(mockProject); - mockResolvePersonaIdentities.mockResolvedValue({ - implementer: 'bot-impl', - reviewer: 'bot-rev', - }); - - const sender = buildReactionSender('github'); - const payload = { repository: { full_name: 'owner/repo' }, comment: { id: 1 } }; - sender(payload, 'issue_comment'); - await vi.runAllTimersAsync(); - - expect(mockFindProjectByRepo).toHaveBeenCalledWith('owner/repo'); - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith( - 'github', - 'owner/repo', - payload, - { implementer: 'bot-impl', reviewer: 'bot-rev' }, - mockProject, - ); - vi.useRealTimers(); - }); - - it('does not send reaction for push events', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('github'); - sender({ repository: { full_name: 'owner/repo' } }, 'push'); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); - - it('does not send reaction when repo is missing', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('github'); - sender({}, 'issue_comment'); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); -}); - -describe('buildReactionSender — jira', () => { - const config = { - defaults: {} as never, - projects: [ - { - id: 'jira-proj-1', - jira: { projectKey: 'PROJ' }, - } as never, - ], - }; - - it('sends reaction for comment_created events', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('jira', config); - const payload = { - webhookEvent: 'comment_created', - issue: { fields: { project: { key: 'PROJ' } } }, - }; - sender(payload, 'comment_created'); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).toHaveBeenCalledWith('jira', 'jira-proj-1', payload); - vi.useRealTimers(); - }); - - it('does not send reaction for non-comment_ events', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('jira', config); - sender( - { webhookEvent: 'jira:issue_updated', issue: { fields: { project: { key: 'PROJ' } } } }, - 'jira:issue_updated', - ); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); - - it('does not send reaction when project key not found', async () => { - vi.useFakeTimers(); - const sender = buildReactionSender('jira', config); - sender( - { webhookEvent: 'comment_created', issue: { fields: { project: { key: 'UNKNOWN' } } } }, - 'comment_created', - ); - await vi.runAllTimersAsync(); - expect(mockSendAcknowledgeReaction).not.toHaveBeenCalled(); - vi.useRealTimers(); - }); -}); diff --git a/tests/unit/server/webhookParsers.test.ts b/tests/unit/webhook/webhookParsers.test.ts similarity index 99% rename from tests/unit/server/webhookParsers.test.ts rename to tests/unit/webhook/webhookParsers.test.ts index 0f1329e6..80ed8b51 100644 --- a/tests/unit/server/webhookParsers.test.ts +++ b/tests/unit/webhook/webhookParsers.test.ts @@ -22,7 +22,7 @@ import { parseGitHubPayload, parseJiraPayload, parseTrelloPayload, -} from '../../../src/server/webhookParsers.js'; +} from '../../../src/webhook/webhookParsers.js'; function makeHonoContext(body: unknown, headers: Record = {}) { return {