refactor(pm): generalize PM integration layer for plug-and-play extensibility

[aaight]

Summary

Implements the core generalization steps of the PM integration refactor to eliminate per-provider branching and make the layer truly plug-and-play.

Card: https://trello.com/c/cW0No7p6/606-refactor-pm-integration-layer-for-plug-and-play-extensibility

Changes

• Step 1 — Generic pmConfig field (schema.ts, configMapper.ts): Adds pmConfig: z.record(z.unknown()).optional() to ProjectConfigSchema and populates it in mapProjectRow() from the active provider's integration config. Coexists with per-provider fields (trello, jira, linear) for backward compatibility.

• Step 2 — Unified PM lookup (configRepository.ts, configCache.ts, provider.ts): Adds findProjectByPMIdentifierFromDb(provider, identifier) and findProjectWithConfigByPMIdentifierFromDb() using a generic SQL query keyed by PM_IDENTIFIER_KEYS (boardId/projectKey/teamId). Adds getProjectByPMIdentifier/setProjectByPMIdentifier unified cache in ConfigCache. Exports findProjectByPMIdentifier() and loadProjectConfigByPMIdentifier() from provider.ts.

• Step 3 — Generic getPMConfig() accessor (pm/config.ts): Adds getPMConfig(project) returning the generic pmConfig field when populated, falling back to per-provider typed fields for backward compat. Updates getCostFieldId() to use this generic path.

• Step 4 — pmIdentifier field in RouterProjectConfig (router/config.ts): Adds pmIdentifier?: string to RouterProjectConfig and populates it in loadProjectConfig() from the active provider's config (boardId/projectKey/teamId). Adapters can now route by pmIdentifier instead of per-provider fields.

• Step 7 — Unified PMJob type (router/queue.ts, worker-entry.ts): Adds PMJob { type: 'pm', source: string, ... } to the CascadeJob union. Adds PMJobData to worker-entry.ts and handles case 'pm': dispatch via pmRegistry.getOrNull(source) → processPMWebhook(). Per-provider job types are retained for Redis backward compat.

• Step 9 — Open WebhookPlatform type (webhookVerification.ts, credentials.ts): Changes WebhookPlatform from a closed union to string and updates resolveWebhookSecret parameter type accordingly, allowing new PM providers to plug in without touching these files.

Test plan

• TypeScript (tsc --noEmit) — no errors
• All 378 unit test files pass (7512 tests)
• Lint — no errors (5 pre-existing warnings unchanged)
• New tests added:
  • tests/unit/pm/config.test.ts — getPMConfig() tests (7 new cases)
  • tests/unit/db/repositories/configMapper.test.ts — pmConfig population tests (5 new cases)
  • tests/unit/config/configCache.test.ts — projectByPMIdentifier cache tests (7 new cases)
  • tests/unit/router/config.test.ts — pmIdentifier field in RouterProjectConfig
  • tests/unit/router/queue-pm-job.test.ts — PMJob type and discriminated union tests (5 new cases)

🤖 Generated with Claude Code

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

Well-structured incremental refactor that adds generic PM abstractions alongside existing per-provider code. The approach of coexisting with backward-compatible fields is sound. A few design observations worth discussing before this pattern is extended further.

Architecture & Design

• [SHOULD_FIX] pmIdentifier derivation duplicates provider-key knowledge: The mapping of provider to identifier key name (trello->boardId, jira->projectKey, linear->teamId) now exists in three places: configRepository.ts (PM_IDENTIFIER_KEYS), router/config.ts (inline if/else chain at lines 108-114), and the configMapper.ts (activePmConfig priority chain). When a new provider is added, all three locations must be updated in lockstep. Consider extracting the PM_IDENTIFIER_KEYS map to a shared location (e.g., src/pm/constants.ts) and reusing it in router/config.ts to derive pmIdentifier generically via pmConfig[PM_IDENTIFIER_KEYS[pmType]].

• [SHOULD_FIX] resolveWebhookSecret fallback assumes Trello for unknown providers: With WebhookPlatform opened from a closed union to string (Step 9), any unrecognized provider string now falls through to the Trello-specific api_secret credential resolution (line 99 of credentials.ts). A new PM provider would silently get the wrong credential. The function should have an explicit default that returns null for unknown providers, with Trello handled in its own if branch. This is the kind of bug the closed union was protecting against.

Code Issues

Should Fix

• src/pm/config.ts:105-119 — The fallback branch in getCostFieldId() after getPMConfig() is dead code. Since getPMConfig() itself falls back to the per-provider fields when pmConfig is not set, it will return a value whenever any PM config exists — making the per-provider fallback (lines 112-119) unreachable. This isn't a bug (the result is correct either way), but dead code is confusing and should be removed or documented with a comment explaining it's defensive.

Questions

• The new findProjectByPMIdentifier and loadProjectConfigByPMIdentifier functions are exported but have no callers in this PR. Are these intended to be consumed in a follow-up PR? If so, it might be worth mentioning in the PR description which step(s) will consume them.

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

[nhopeatall]

Summary

LGTM — Well-structured incremental refactoring that adds generic PM infrastructure alongside existing per-provider code for backward compatibility. All CI checks pass, test coverage is solid, and the changes are additive (no existing behavior is modified).

Notes

Architecture & Design

The approach of laying down generic infrastructure (pmConfig, pmIdentifier, PM_IDENTIFIER_KEYS, PMJob, unified cache/lookup functions) while keeping per-provider code intact is the right strategy for a gradual migration. The PM_IDENTIFIER_KEYS constant centralizes the provider→key mapping, and new providers only need to add an entry there.

A few observations (not blocking):

1. pmConfig object sharing in configMapper.ts: activePmConfig is a direct cast of the input provider config object (same reference), while the per-provider fields (project.trello, etc.) get new copies via buildTrelloConfig() etc. This isn't a bug because Zod validation creates fresh objects downstream, but it's a subtle inconsistency. A shallow clone ({ ...jiraConfig }) would make the intent clearer.

2. WebhookPlatform type widening: Opening this from a closed union to string achieves the plug-and-play goal, but it also means typos in platform strings won't be caught at compile time. The risk is minimal since all existing call sites use literal strings, but consider keeping the known values as a documented union (type BuiltInPlatform = ...) alongside the open string if type safety becomes a concern later.

3. resolveWebhookSecret behavior change (credentials.ts): The old code had an implicit fallback to Trello's api_secret for unknown providers — the new code correctly returns null. This is a good fix that eliminates a potential security issue where an unknown provider would silently get Trello credentials.

Code Quality

• SQL query in pmIdentifierWhereClause correctly parameterizes both provider and key via drizzle's sql template tag — no injection risk
• Cache key format ${provider}:${identifier} properly isolates entries by provider, and the invalidate() method correctly clears the new cache
• getPMConfig() fallback chain is correct and matches the existing pm.type derivation logic
• getCostFieldId() refactoring is behaviorally equivalent to the old per-provider branching
• Worker case 'pm' dispatch correctly validates the provider via pmRegistry.getOrNull() before calling processPMWebhook()
• New functions (findProjectByPMIdentifier, loadProjectConfigByPMIdentifier) follow existing patterns — caching for findProject*, no caching for loadProjectConfig*
• Tests are well-structured with good edge case coverage (negative cache, provider isolation, invalidation)

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

[zbigniewsobiecki]

@aaight add proper tests to hit the coverage target

[aaight]

Added tests to cover the four files Codecov flagged as under-covered. All 138 affected tests pass; full suite now has 8053 tests green.

What was added:

• tests/unit/worker-entry.test.ts — LinearJob dispatch routing to processLinearWebhook, and three PMJob tests: happy-path routing through pmRegistry → processPMWebhook, and the unknown-provider error branch

• tests/unit/config/provider.test.ts — findProjectByPMIdentifier() (cache hit, cache miss/DB fetch, negative cache, per-provider) and loadProjectConfigByPMIdentifier() (returns result, returns undefined, does not consult cache)

• tests/unit/db/repositories/configRepository.test.ts — findProjectByPMIdentifierFromDb() for trello/jira/linear providers, unknown-provider no-match, and findProjectWithConfigByPMIdentifierFromDb() (found + not found)

• tests/unit/router/platformClients.test.ts — resolveWebhookSecret() direct tests for all five provider branches (github, jira, sentry, linear, trello) plus the unknown-provider return null guard added in Step 9

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

[nhopeatall]

Summary

LGTM — Well-structured incremental refactor that adds generic PM abstractions alongside existing per-provider code, maintaining full backward compatibility. All CI checks pass, test coverage is thorough (34+ new test cases), and the implementation is consistent with existing codebase patterns.

Notes

Architecture: The layered approach is sound — each step (generic pmConfig field, unified PM lookup, getPMConfig() accessor, pmIdentifier in router config, PMJob union member, open WebhookPlatform type) builds incrementally and coexists with per-provider code. This avoids a big-bang migration while making the path clear for new providers to plug in.

Key observations verified:

• pmIdentifierWhereClause correctly passes the JSONB key as a bind parameter — PostgreSQL's ->> operator accepts parameterized text on the RHS, so this works identically to the hardcoded per-provider queries.
• mapProjectRow precedence for activePmConfig (jira > linear > trello) matches the pmType derivation order — no inconsistency.
• loadProjectConfigByPMIdentifier correctly delegates directly to DB without caching, matching the pattern of all other loadProjectConfigBy* functions.
• resolveWebhookSecret now explicitly handles Trello as a named branch instead of the previous fall-through default, and returns null for unknown providers — a defensive improvement over the old behavior of silently using Trello credentials for unknown platforms.
• getPMConfig() backward-compat fallback correctly derives from per-provider typed fields when pmConfig is not populated (test fixtures, legacy projects).
• The PMJob worker dispatch properly validates against pmRegistry and provides a clear error message with registered providers listed.

No blocking or should-fix issues found.

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