Skip to content

refactor(triggers): use registry-driven validation for all integration categories#1043

Merged
aaight merged 2 commits intodevfrom
feature/registry-driven-integration-validation
Mar 24, 2026
Merged

refactor(triggers): use registry-driven validation for all integration categories#1043
aaight merged 2 commits intodevfrom
feature/registry-driven-integration-validation

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 24, 2026

Summary

  • Replaces switch-case with registry-driven validation in src/triggers/shared/integration-validation.ts — uses integrationRegistry.getByCategory() instead of hardcoded category branches
  • Adds alerting validation — the alerting agent now properly validates that Sentry is configured before running (was silently passing via default: return null)
  • Extensible by design — new integration categories are automatically validated without code changes to this file
  • SCM persona token check preserved — type-narrows SCM integrations to SCMIntegration to call hasPersonaToken() as before

Technical Details

Key changes in src/triggers/shared/integration-validation.ts

  • Removed validatePmIntegration() and validateScmIntegration() helper functions (category-specific switch branches)
  • Added isScmIntegration() type guard to narrow registry results to SCMIntegration for persona token checking
  • Added notConfiguredError() helper for building user-friendly error messages per category
  • Added validateScmPersonaToken() helper to reduce cognitive complexity
  • Added validateCategory() helper that centralizes per-category validation logic
  • validateIntegrations() now simply maps required categories through validateCategory() and awaits in parallel
  • Removed imports of hasScmIntegration, hasScmPersonaToken, and hasPmIntegration — validation now goes through the registry

Key changes in tests/unit/triggers/shared/integration-validation.test.ts

  • Updated mocks to use integrationRegistry.getByCategory instead of individual hasPmIntegration/hasScmIntegration/hasScmPersonaToken functions
  • Added mockIntegration() helper for building typed mock integration objects
  • Added new test group: alerting integration validation covering both pass and fail scenarios

Test plan

  • Unit tests: npx vitest run tests/unit/triggers/shared/integration-validation.test.ts — 17 tests pass
  • Full unit suite: npm test — 6876 tests pass across 357 files
  • Type checking: npx tsc --noEmit — zero errors
  • Lint: npm run lint — zero warnings/errors

Card: https://trello.com/c/Ij85dvbs/548-as-a-developer-i-want-integration-validationts-to-use-the-unified-registry-so-that-alerting-validation-is-supported

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/triggers/shared/integration-validation.ts 88.88% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

The new registry-driven validateIntegrations() calls
integrationRegistry.getByCategory() which requires integrations to be
registered first. Without importing the bootstrap module, the registry
is empty and all validations report "none is registered" instead of
actually checking project credentials against the database.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Mar 24, 2026

CI Failures Resolved

Fixes Applied

  • tests/integration/integration-validation.test.ts: Added import '../../src/integrations/bootstrap.js' to populate the integrationRegistry before tests run.

    The new registry-driven validateIntegrations() calls integrationRegistry.getByCategory(), which requires integrations (Trello, JIRA, GitHub, Sentry) to be registered first. Without this bootstrap import, the registry was empty and all validations reported "none is registered" instead of querying actual project credentials in the database.

Verification

  • 17 unit tests pass (tests/unit/triggers/shared/integration-validation.test.ts)
  • 34 integration tests pass (tests/integration/integration-validation.test.ts)
  • Lint and type checks pass (confirmed by pre-commit and pre-push hooks)
  • Changes pushed to branch

🕵️ claude-code · claude-sonnet-4-6 · run details

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Clean refactor that replaces hardcoded switch-case validation with registry-driven lookup. The change correctly closes the gap where the alerting category silently passed validation via default: return null, and makes the system extensible to new integration categories without code changes.

All CI checks pass. The implementation correctly uses the existing integrationRegistry.getByCategory() API, the SCMIntegration type guard is sound, and the two-tier error messaging ("none is registered" vs "none is configured") gives clearer diagnostics than before.

LGTM — no blocking issues found.

🕵️ claude-code · claude-opus-4-6 · run details

@aaight aaight merged commit 28e740d into dev Mar 24, 2026
9 checks passed
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.

2 participants