Skip to content

feat(config): introduce CredentialResolver interface for test-friendly DI#1012

Merged
aaight merged 1 commit intodevfrom
feature/credential-resolver-interface
Mar 23, 2026
Merged

feat(config): introduce CredentialResolver interface for test-friendly DI#1012
aaight merged 1 commit intodevfrom
feature/credential-resolver-interface

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 23, 2026

Summary

  • Introduces CredentialResolver interface with resolve(projectId, key) and resolveAll(projectId) methods
  • Adds DbCredentialResolver (production, reads from DB via existing repository calls) and EnvCredentialResolver (worker context, reads from process.env + CASCADE_CREDENTIAL_KEYS)
  • Exports setCredentialResolver() for test-friendly dependency injection — no more process.env manipulation in tests
  • All existing exported functions (getIntegrationCredential, getIntegrationCredentialOrNull, getOrgCredential, getAllProjectCredentials) preserved as thin wrappers over getResolver()

Technical Details

The getResolver() helper auto-selects the right implementation:

  • CASCADE_CREDENTIAL_KEYS set → EnvCredentialResolver (worker context)
  • Not set → DbCredentialResolver (router/dashboard/test context)
  • Override: call setCredentialResolver(mockResolver) before tests, then setCredentialResolver(null) in afterEach to restore auto-selection

Test plan

  • All 31 original tests in tests/unit/config/provider.test.ts continue to pass unchanged
  • 13 new tests added covering: DI via setCredentialResolver, DbCredentialResolver.resolve() / resolveAll(), EnvCredentialResolver.resolve() / resolveAll()
  • Full unit test suite passes (6590 tests across 344 files)
  • TypeScript type check passes with zero errors
  • Biome lint passes with zero errors

Card

https://trello.com/c/69c19856a71e27354a2b3ccc

🤖 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 85.36585% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/config/provider.ts 85.36% 6 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 refactoring that extracts credential resolution into a proper interface/implementation pattern with dependency injection support for tests. The behavioral equivalence with the previous resolveFromEnvOrDb helper is maintained, CI is fully green, and the new tests provide solid coverage.

Notes

Minor behavioral nuance (not blocking): EnvCredentialResolver.resolve() uses process.env[key] ?? null, which would return an empty string "" if an env var exists but is empty. The old code used process.env[envKey] ? ... : notFoundValue, which treated empty strings as "not found" and returned null. This only affects getIntegrationCredentialOrNull and getOrgCredential in worker context with empty-string env vars — an edge case that shouldn't occur in practice since credentials are always non-empty values populated by the router. Not worth changing, just documenting for awareness.

The rest of the changes are straightforward:

  • Interface + two implementations (DbCredentialResolver, EnvCredentialResolver) correctly encapsulate the two resolution strategies
  • getResolver() auto-selects the right implementation based on context, matching the original branching logic
  • setCredentialResolver() provides clean test DI without process.env manipulation
  • The early !envKey guard in getIntegrationCredential produces the same error as the old code path (where dbLookup returned null → throw)
  • All existing public function signatures are preserved

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

@aaight aaight merged commit 647d239 into dev Mar 23, 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