refactor(triggers): extract shared webhook utilities and add Sentry concurrency

src/triggers/README.md

@@ -0,0 +1,131 @@
+# Trigger System
+
+This directory contains the trigger handlers and registry that route webhook events to agents.
+
+## Architecture Overview
+
+```
+Webhook → Router → Redis/BullMQ → Worker → TriggerRegistry → Agent
+```
+
+### Two-tier webhook handling
+
+Webhook processing is split into two distinct tiers:
+
+| Tier | Where | Purpose |
+|------|-------|---------|
+| **Router** | `src/router/` | Receive, validate, acknowledge, enqueue |
+| **Worker** | `src/triggers/` | Resolve trigger, establish credentials, run agent |
+
+**Router side is fully unified** — all four providers (Trello, JIRA, GitHub, Sentry) share `processRouterWebhook()` + `RouterPlatformAdapter`. No provider-specific branching in the router.
+
+**Worker side has intentional divergence** — see below.
+
+---
+
+## Worker-Side Handler Comparison
+
+| Feature | PM (`processPMWebhook`) | GitHub (`processGitHubWebhook`) | Sentry (`processSentryWebhook`) |
+|---------|------------------------|--------------------------------|--------------------------------|
+| Trigger dispatch | ✅ Registry | ✅ Registry or pre-resolved | ✅ Registry or pre-resolved |
+| Ack comment (PR) | ❌ N/A | ✅ Posts to PR | ❌ N/A |
+| Ack comment (PM) | ✅ Via PM lifecycle | ✅ For PM-focused agents | ❌ N/A |
+| CI check polling | ❌ N/A | ✅ `pollWaitForChecks()` | ❌ N/A |
+| PM credential scope | ✅ `integration.withCredentials` | ✅ `withPMCredentials` | ✅ `withPMCredentials` |
+| PM lifecycle ops | ✅ prepareForAgent / handleFailure | ✅ For PM-focused agents | ❌ Skipped |
+| Persona token mgmt | ❌ N/A | ✅ Implementer / reviewer | ❌ N/A |
+| Agent concurrency | ✅ `checkAgentTypeConcurrency` | ✅ `withAgentTypeConcurrency` | ✅ `withAgentTypeConcurrency` |
+
+---
+
+## Why GitHub and Sentry Cannot Use `processPMWebhook()`
+
+`processPMWebhook()` assumes **PM semantics**:
+- It calls `integration.parseWebhookPayload()` expecting a PM event (card ID, board identifier)
+- It drives `PMLifecycleManager` (prepareForAgent → handleFailure / handleSuccess)
+- The `PMIntegration` interface provides card parsing, ack cleanup, and credential scoping
+
+Forcing GitHub or Sentry into this pipeline would require:
+- Provider-specific `if` branches inside `processPMWebhook()` — worse than current design
+- Mocking PM lifecycle ops (they don't apply to Sentry alerts or GitHub PRs)
+
+### GitHub-specific features (cannot be generalized)
+
+1. **CI check polling** (`pollWaitForChecks`) — GitHub is the only provider with CI. No other source polls build status before running an agent.
+2. **PR acknowledgment comments** — GitHub PRs get a comment like "👀 Reviewing…" immediately. No other source has this flow.
+3. **Dual-persona token management** — The implementer vs. reviewer persona selection is GitHub-specific. No Trello/JIRA/Sentry equivalent.
+4. **PM-focused agent routing** — When a PM-focused agent (e.g. `backlog-manager`) fires from a GitHub PR event, it posts the ack to Trello/JIRA instead of the PR, and uses PM-appropriate lifecycle config.
+
+### Sentry-specific simplicity (intentional)
+
+Sentry is an alerting source. There are no:
+- Work item cards to manage lifecycle on
+- PR comments to post
+- CI checks to poll
+
+Sentry's handler is intentionally minimal: load project, resolve trigger, run agent in PM scope.
+
+---
+
+## Shared Utilities (`src/triggers/shared/`)
+
+To reduce duplication across the three worker-side handlers, shared utilities are extracted to `src/triggers/shared/`:
+
+| File | Purpose | Used By |
+|------|---------|---------|
+| `concurrency.ts` | `withAgentTypeConcurrency()` — wraps check→mark→execute→clear | GitHub, Sentry |
+| `trigger-resolution.ts` | `resolveTriggerResult()` — pre-resolved or dispatch | Sentry (GitHub and PM use inline logic) |
+| `credential-scope.ts` | `withPMScope()` — `withPMCredentials` + `withPMProvider` | GitHub, Sentry |
+| `pm-ack.ts` | `postPMAckComment()` — posts ack to Trello/JIRA | GitHub worker handler |
+| `agent-execution.ts` | `runAgentExecutionPipeline()` — full agent lifecycle | All handlers (via `webhook-execution.ts`) |
+| `webhook-execution.ts` | `runAgentWithCredentials()` — LLM keys + credentials + pipeline | GitHub, PM |
+
+---
+
+## Flow Diagrams
+
+### PM webhook (Trello / JIRA)
+
+```
+processPMWebhook(integration, payload, registry)
+  └─ integration.parseWebhookPayload(payload)       → event
+  └─ integration.lookupProject(event.identifier)    → project
+  └─ integration.withCredentials(projectId)
+       └─ withPMProvider(pmProvider)
+            └─ resolveTriggerResult(registry, ctx, preResolved)
+            └─ handleMatchedTrigger(...)
+                 └─ withAgentTypeConcurrency(projectId, agentType)
+                 └─ startWatchdog()
+                 └─ executeAgent() → runAgentWithCredentials()
+                      └─ injectLlmApiKeys()
+                      └─ withGitHubToken(personaToken)
+                      └─ runAgentExecutionPipeline(...)
+```
+
+### GitHub webhook
+
+```
+processGitHubWebhook(payload, eventType, registry, ackCommentId, triggerResult)
+  └─ integration.parseWebhookPayload(payload)       → event
+  └─ integration.lookupProject(event.repo)          → project
+  └─ [inline] if triggerResult → use it, else dispatchTrigger(registry, payload, project)
+  └─ [optional] pollWaitForChecks(result, repo)     → checksOk
+  └─ maybePostAckComment(result, ...)               → PR or PM ack
+  └─ runGitHubAgent(result, project, config)
+       └─ withAgentTypeConcurrency(projectId, agentType)
+            └─ startWatchdog()
+            └─ withPMScope(project)
+                 └─ runAgentWithCredentials(integration, result, ...)
+```
+
+### Sentry webhook
+
+```
+processSentryWebhook(payload, projectId, registry, triggerResult)
+  └─ loadProjectConfigById(projectId)               → project
+  └─ resolveTriggerResult(registry, ctx, preResolved)
+  └─ withAgentTypeConcurrency(projectId, agentType)
+       └─ startWatchdog()
+       └─ withPMScope(project)
+            └─ runAgentExecutionPipeline(result, ...)
+```

src/triggers/github/webhook-handler.ts

@@ -6,26 +6,23 @@
  * - CI check polling → ./check-polling.ts
  * - Credential scoping + agent execution → ../shared/webhook-execution.ts
  * - GitHub-specific AgentExecutionConfig → ./integration.ts
+ * - Agent-type concurrency → ../shared/concurrency.ts
+ * - PM credential scope → ../shared/credential-scope.ts
+ * - PM ack posting → ../shared/pm-ack.ts
  */
 
 import { isPMFocusedAgent } from '../../agents/definitions/loader.js';
 import { githubClient, withGitHubToken } from '../../github/client.js';
 import { getPersonaToken, resolvePersonaIdentities } from '../../github/personas.js';
-import { withPMCredentials, withPMProvider } from '../../pm/context.js';
-import { createPMProvider, pmRegistry } from '../../pm/index.js';
 import { extractGitHubContext, generateAckMessage } from '../../router/ackMessageGenerator.js';
-import { postJiraAck, postTrelloAck } from '../../router/acknowledgments.js';
-import {
-	checkAgentTypeConcurrency,
-	clearAgentTypeEnqueued,
-	markAgentTypeEnqueued,
-	markRecentlyDispatched,
-} from '../../router/agent-type-lock.js';
 import type { CascadeConfig, ProjectConfig, TriggerContext } from '../../types/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 { withAgentTypeConcurrency } from '../shared/concurrency.js';
+import { withPMScope } from '../shared/credential-scope.js';
+import { postPMAckComment } from '../shared/pm-ack.js';
 import { runAgentWithCredentials } from '../shared/webhook-execution.js';
 import type { TriggerResult } from '../types.js';
 import { postAcknowledgmentComment, updateInitialCommentWithError } from './ack-comments.js';
@@ -53,10 +50,6 @@ function requireProjectId(project: ProjectConfig): string {
 	return project.id;
 }
 
-function isValidPmType(pmType: string | undefined): pmType is 'trello' | 'jira' {
-	return pmType === 'trello' || pmType === 'jira';
-}
-
 async function maybePostPmAckComment(
 	result: TriggerResult,
 	payload: unknown,
@@ -73,18 +66,13 @@ async function maybePostPmAckComment(
 	);
 	const pmType = project.pm?.type;
 
-	if (!isValidPmType(pmType)) {
-		logger.warn('Unknown PM type for PM-focused agent ack (worker-side)', {
-			agentType: result.agentType,
-			pmType,
-		});
-		return;
-	}
-
-	const commentId =
-		pmType === 'trello'
-			? await postTrelloAck(projectId, workItemId, message)
-			: await postJiraAck(projectId, workItemId, message);
+	const commentId = await postPMAckComment(
+		projectId,
+		workItemId,
+		pmType,
+		message,
+		result.agentType ?? undefined,
+	);
 
 	if (commentId) {
 		result.agentInput.ackCommentId = commentId;
@@ -102,14 +90,7 @@ async function dispatchTrigger(
 	const personaIdentities = await resolvePersonaIdentities(projectId);
 	const githubToken = await getPersonaToken(projectId, 'implementation');
 	const ctx: TriggerContext = { project, source: 'github', payload, personaIdentities };
-	const pmProvider = createPMProvider(project);
-	return withPMCredentials(
-		projectId,
-		project.pm?.type,
-		(t) => pmRegistry.getOrNull(t),
-		() =>
-			withPMProvider(pmProvider, () => withGitHubToken(githubToken, () => registry.dispatch(ctx))),
-	);
+	return withPMScope(project, () => withGitHubToken(githubToken, () => registry.dispatch(ctx)));
 }
 
 /** Post ack comment on the PR using the agent-specific persona token. */
@@ -167,43 +148,38 @@ async function runGitHubAgent(
 	project: ProjectConfig,
 	config: CascadeConfig,
 ): Promise<void> {
-	// Agent-type concurrency limit
-	let agentTypeMaxConcurrency: number | null = null;
-	if (result.agentType) {
-		const concurrencyCheck = await checkAgentTypeConcurrency(project.id, result.agentType);
-		agentTypeMaxConcurrency = concurrencyCheck.maxConcurrency;
-		if (concurrencyCheck.blocked) return;
-		if (agentTypeMaxConcurrency !== null) {
-			markRecentlyDispatched(project.id, result.agentType);
-			markAgentTypeEnqueued(project.id, result.agentType);
-		}
-	}
-
-	startWatchdog(project.watchdogTimeoutMs);
-
 	// PM-focused agents (e.g. backlog-manager) triggered from GitHub should use
 	// PM-appropriate lifecycle config: no GitHub PR comment callbacks, allow PM lifecycle ops.
 	const pmFocused = result.agentType ? await isPMFocusedAgent(result.agentType) : false;
 
-	try {
+	const agentType = result.agentType;
+
+	const execute = async () => {
+		// Only start the watchdog when the agent actually runs (after concurrency check passes).
+		// Starting it before the check risks a spurious process.exit(1) if the container
+		// is still alive after a concurrency-blocked job finishes.
+		startWatchdog(project.watchdogTimeoutMs);
+
 		// Establish PM credential + provider scope for agents with workItemId
 		// (needed for PM lifecycle operations: labels, status moves, PR links)
-		const pmProvider = createPMProvider(project);
-		await withPMCredentials(
-			project.id,
-			project.pm?.type,
-			(t) => pmRegistry.getOrNull(t),
-			() =>
-				withPMProvider(pmProvider, () =>
-					runAgentWithCredentials(
-						integration,
-						result,
-						project,
-						config,
-						resolveGitHubExecutionConfig(pmFocused),
-					),
-				),
+		await withPMScope(project, () =>
+			runAgentWithCredentials(
+				integration,
+				result,
+				project,
+				config,
+				resolveGitHubExecutionConfig(pmFocused),
+			),
 		);
+	};
+
+	// Agent-type concurrency limit wraps the entire execution
+	try {
+		if (agentType) {
+			await withAgentTypeConcurrency(project.id, agentType, execute, 'GitHub agent');
+		} else {
+			await execute();
+		}
 	} catch (err) {
 		logger.error('Failed to process GitHub webhook', { error: String(err) });
 		if (!pmFocused) {
@@ -216,10 +192,6 @@ async function runGitHubAgent(
 				updateInitialCommentWithError(result, { success: false, error: String(err) }),
 			);
 		}
-	} finally {
-		if (result.agentType && agentTypeMaxConcurrency !== null) {
-			clearAgentTypeEnqueued(project.id, result.agentType);
-		}
 	}
 }
 

src/triggers/sentry/webhook-handler.ts

@@ -5,15 +5,21 @@
  * falling back to dispatching through the trigger registry if not.
  * After resolving the trigger result, runs the matched agent via the
  * shared execution pipeline.
+ *
+ * Shared utilities used:
+ * - Trigger resolution → ../shared/trigger-resolution.ts
+ * - Agent-type concurrency → ../shared/concurrency.ts
+ * - PM credential scope → ../shared/credential-scope.ts
  */
 
-import { withPMCredentials, withPMProvider } from '../../pm/context.js';
-import { createPMProvider, pmRegistry } from '../../pm/index.js';
 import type { TriggerResult } from '../../types/index.js';
 import { startWatchdog } from '../../utils/lifecycle.js';
 import { logger } from '../../utils/logging.js';
 import type { TriggerRegistry } from '../registry.js';
 import { runAgentExecutionPipeline } from '../shared/agent-execution.js';
+import { withAgentTypeConcurrency } from '../shared/concurrency.js';
+import { withPMScope } from '../shared/credential-scope.js';
+import { resolveTriggerResult } from '../shared/trigger-resolution.js';
 
 export async function processSentryWebhook(
 	payload: unknown,
@@ -29,22 +35,14 @@ export async function processSentryWebhook(
 		return;
 	}
 
+	const ctx = {
+		project: pc.project,
+		source: 'sentry' as const,
+		payload,
+	};
+
 	// Resolve trigger result — use pre-computed from router or dispatch via registry
-	let result: TriggerResult | null;
-	if (triggerResult) {
-		logger.info('processSentryWebhook: using pre-computed trigger result', {
-			projectId,
-			agentType: triggerResult.agentType,
-		});
-		result = triggerResult;
-	} else {
-		const ctx = {
-			project: pc.project,
-			source: 'sentry' as const,
-			payload,
-		};
-		result = await registry.dispatch(ctx);
-	}
+	const result = await resolveTriggerResult(registry, ctx, triggerResult, 'processSentryWebhook');
 
 	if (!result) {
 		logger.info('processSentryWebhook: no trigger matched', { projectId });
@@ -63,20 +61,22 @@ export async function processSentryWebhook(
 		agentType: result.agentType,
 	});
 
-	startWatchdog(pc.project.watchdogTimeoutMs);
-
-	const pmProvider = createPMProvider(pc.project);
-	await withPMCredentials(
+	await withAgentTypeConcurrency(
 		pc.project.id,
-		pc.project.pm?.type,
-		(t) => pmRegistry.getOrNull(t),
-		() =>
-			withPMProvider(pmProvider, () =>
+		result.agentType,
+		() => {
+			// Only start the watchdog when the agent actually runs (after concurrency check passes).
+			// Starting it before the check risks a spurious process.exit(1) if the container
+			// is still alive after a concurrency-blocked job finishes.
+			startWatchdog(pc.project.watchdogTimeoutMs);
+			return withPMScope(pc.project, () =>
 				runAgentExecutionPipeline(result, pc.project, pc.config, {
 					logLabel: 'Sentry agent',
 					skipPrepareForAgent: true,
 					skipHandleFailure: true,
 				}),
-			),
+			);
+		},
+		'processSentryWebhook',
 	);
 }