Skip to content

feat(pm): extend PMIntegration from IntegrationModule and register PM providers in unified registry#1022

Merged
aaight merged 1 commit intodevfrom
feature/pm-integration-extends-integration-module
Mar 23, 2026
Merged

feat(pm): extend PMIntegration from IntegrationModule and register PM providers in unified registry#1022
aaight merged 1 commit intodevfrom
feature/pm-integration-extends-integration-module

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 23, 2026

Summary

  • Bridges PM providers (Trello, JIRA) to the unified IntegrationModule interface so they participate in the integrationRegistry
  • PMIntegration now extends IntegrationModule with category: 'pm' and hasIntegration() required
  • TrelloIntegration and JiraIntegration implement hasIntegration() by delegating to PROVIDER_CREDENTIAL_ROLES (same logic as standalone hasPmIntegration())
  • src/pm/bootstrap.ts and src/pm/index.ts register Trello/JIRA into both pmRegistry and integrationRegistry
  • hasPmIntegration() function is kept unchanged for backward compatibility
  • GitHubWebhookIntegration updated to satisfy new interface requirements (adds category: 'pm' and a no-op hasIntegration() since it is a PM-pipeline adapter, not a real PM provider)

Trello card: https://trello.com/c/q6ESNAaG/544-as-a-developer-i-want-pmintegration-to-extend-integrationmodule-so-that-pm-providers-participate-in-the-unified-registry

Test plan

  • TrelloIntegration.hasIntegration() returns true when provider is trello with required credentials
  • TrelloIntegration.hasIntegration() returns false when provider is not trello or credentials are missing
  • JiraIntegration.hasIntegration() returns true when provider is jira with required credentials
  • JiraIntegration.hasIntegration() returns false when provider is not jira or credentials are missing
  • Both providers expose category: 'pm'
  • All existing PM tests continue to pass (351 test files, 6749 tests)
  • TypeScript typecheck passes with zero errors
  • Lint passes with zero errors

🤖 Generated with Claude Code

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 73.46939% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pm/bootstrap.ts 0.00% 11 Missing ⚠️
src/triggers/github/integration.ts 50.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

LGTM — Clean integration of PM providers into the unified IntegrationModule registry. The interface extension is well-designed, hasIntegration() implementations correctly delegate to the same PROVIDER_CREDENTIAL_ROLES data source as the existing hasPmIntegration(), all integrationRegistry.register() calls are properly guarded against the registry's duplicate-throw behavior, and test coverage is thorough.

Notes

  • The category: 'pm' on GitHubWebhookIntegration is a type system constraint from implementing PMIntegration (which narrows IntegrationModule.category). The comment correctly explains it's not a real PM provider and won't be registered in the integration registry.
  • src/pm/index.ts still has unguarded pmRegistry.register() calls (unlike bootstrap.ts which guards both). This is a pre-existing pattern — pmRegistry uses Map.set() (silent overwrite) so it won't throw, but when both files are imported in the same process (router imports bootstrap.js, then triggers import pm/index.js), the pmRegistry and integrationRegistry will hold different object instances for the same provider. Since the integration classes are stateless, this has no behavioral impact, but it's worth noting for future awareness.

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

@aaight aaight merged commit d338e64 into dev Mar 23, 2026
8 of 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