Skip to content

refactor: code review cleanups for trigger system and router#504

Merged
zbigniewsobiecki merged 1 commit intodevfrom
refactor/code-review-cleanups
Feb 23, 2026
Merged

refactor: code review cleanups for trigger system and router#504
zbigniewsobiecki merged 1 commit intodevfrom
refactor/code-review-cleanups

Conversation

@zbigniewsobiecki
Copy link
Copy Markdown
Member

Summary

Comprehensive code quality improvements identified through code review of the trigger system refactoring and router dispatch architecture:

  • Structured logging: Replace 35+ raw console.log/warn/error calls with structured logger across all router handlers (github, trello, jira, index, queue), enabling log-level control and JSON metadata
  • Type deduplication: Create shared JiraWebhookPayload and STATUS_TO_AGENT in src/triggers/jira/types.ts, eliminating duplicate definitions across 3 JIRA trigger files
  • Error handling consistency: Change Trello card-moved and label-added triggers to return null instead of throwing on missing card ID, matching every other trigger handler
  • Regex fix: Fix isAgentLogFilename regex that couldn't handle multi-hyphen agent names (e.g. respond-to-review-2026-01-02T16-30-24-339Z.zip)
  • Code deduplication: Extract shared withPMCredentials helper to src/pm/context.ts, eliminating near-identical implementations in router and worker
  • Dead code removal: Remove fallback job queueing for unresolvable GitHub projects (worker would always fail too)
  • Naming consistency: Rename queueJiraJobprocessJiraWebhookEvent to match Trello/GitHub naming
  • Error monitoring: Add Sentry captureException for persistent ack reaction failures
  • Lint fix: Refactor processGitHubWebhook to resolve cognitive complexity warning (17 → under 15) by extracting tryEnqueueIfBusy and resolveTriggerResult helpers
  • Test coverage: Add tests for unknown agent types in trigger config, empty nested config objects, multi-hyphen filenames, and missing card ID edge cases

Test plan

  • All 2725 tests pass (166 test files)
  • TypeScript typecheck clean
  • Biome lint clean (0 errors, 0 warnings)
  • Pre-commit hooks pass (lint + typecheck)
  • Pre-push hooks pass (full test suite)

🤖 Generated with Claude Code

- Replace 35+ raw console.log/warn/error calls with structured logger
  across router handlers (github, trello, jira, index, queue)
- Create shared JiraWebhookPayload type and STATUS_TO_AGENT constant
  in src/triggers/jira/types.ts to eliminate duplication across 3 files
- Fix Trello card-moved and label-added triggers to return null instead
  of throwing on missing card ID (consistency with other handlers)
- Fix fragile isAgentLogFilename regex that couldn't handle multi-hyphen
  agent names like respond-to-review
- Extract shared withPMCredentials helper to src/pm/context.ts,
  eliminating duplication between router and worker
- Remove dead fallback job queueing for unresolvable GitHub projects
- Rename queueJiraJob → processJiraWebhookEvent for naming consistency
- Add Sentry captureException for persistent ack reaction failures
- Add ackCommentId type-clarifying comments to job interfaces
- Refactor processGitHubWebhook to fix cognitive complexity lint warning
  (extract tryEnqueueIfBusy and resolveTriggerResult helpers)
- Add test coverage for unknown agent types, empty trigger config
  objects, multi-hyphen filenames, and missing card ID edge cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@zbigniewsobiecki zbigniewsobiecki merged commit 0763849 into dev Feb 23, 2026
5 checks passed
@zbigniewsobiecki zbigniewsobiecki deleted the refactor/code-review-cleanups branch February 23, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant