From a169789059f00d79f758d06612bda264dee97b10 Mon Sep 17 00:00:00 2001 From: zbigniew sobiecki Date: Fri, 24 Apr 2026 15:08:02 +0200 Subject: [PATCH] test(pm-discovery): cover error branches introduced by resolvePMCredentials refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1184 extracted `promoteConfigCredentials` + `loadIntegrationAndManifest` helpers but added five guard branches none of the new tests exercised (codecov patch coverage on that PR was 58.33%, below the 80% target). This commit pins each branch directly so the patch is in the green: - UNAUTHORIZED when projectId is set but effectiveOrgId is null - NOT_FOUND when no PM integration is configured for the project - NOT_FOUND when the saved integration belongs to a different provider - configToCredentials returning a non-object (string/null/array) is silently coerced to empty — resolved bag ends up project_credentials-only - configToCredentials throwing is swallowed with a console.warn; discovery still returns project_credentials — a broken hook cannot brick the wizard All five are behavioural contracts the original `resolvePMCredentials` upheld through inline conditionals; the refactor preserved them but moved them into the helpers. These tests make the contract explicit. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/unit/api/pm-discovery.test.ts | 169 ++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/tests/unit/api/pm-discovery.test.ts b/tests/unit/api/pm-discovery.test.ts index 182607eb..d91bc041 100644 --- a/tests/unit/api/pm-discovery.test.ts +++ b/tests/unit/api/pm-discovery.test.ts @@ -504,6 +504,175 @@ describe('pmDiscoveryRouter', () => { expect(hookSpy).not.toHaveBeenCalled(); }); + + // ── Error paths introduced by the resolvePMCredentials refactor ───── + // The projectId branch now flows through two new helpers + // (promoteConfigCredentials + loadIntegrationAndManifest) with several + // guard throws. These tests pin each branch so a future refactor + // cannot silently drop one. + + it('throws UNAUTHORIZED when projectId is set but effectiveOrgId is null', async () => { + const { createFakePMManifest } = await import('../../helpers/fakePMProvider.js'); + registerPMProvider({ ...createFakePMManifest(), id: 'fake-auth' }); + + const caller = pmDiscoveryRouter.createCaller({ + effectiveOrgId: null as unknown as string, + }); + await expect( + caller.discover({ + providerId: 'fake-auth', + capability: 'currentUser', + args: {}, + projectId: 'some-project', + }), + ).rejects.toMatchObject({ code: 'UNAUTHORIZED' }); + }); + + it('throws NOT_FOUND when the project has no PM integration configured', async () => { + const { createFakePMManifest } = await import('../../helpers/fakePMProvider.js'); + registerPMProvider({ ...createFakePMManifest(), id: 'fake-missing' }); + + const { getIntegrationByProjectAndCategory } = await import( + '../../../src/db/repositories/integrationsRepository.js' + ); + vi.mocked(getIntegrationByProjectAndCategory).mockResolvedValue(null); + + const caller = pmDiscoveryRouter.createCaller({ effectiveOrgId: 'org-1' }); + await expect( + caller.discover({ + providerId: 'fake-missing', + capability: 'currentUser', + args: {}, + projectId: 'orphan-project', + }), + ).rejects.toMatchObject({ + code: 'NOT_FOUND', + message: expect.stringMatching(/No PM integration/i), + }); + }); + + it('throws NOT_FOUND when the saved integration is for a different provider', async () => { + const { createFakePMManifest } = await import('../../helpers/fakePMProvider.js'); + registerPMProvider({ ...createFakePMManifest(), id: 'fake-expected' }); + + const { getIntegrationByProjectAndCategory } = await import( + '../../../src/db/repositories/integrationsRepository.js' + ); + vi.mocked(getIntegrationByProjectAndCategory).mockResolvedValue({ + projectId: 'p', + category: 'pm', + provider: 'fake-other', + config: {}, + triggers: {}, + } as unknown as Awaited>); + + const caller = pmDiscoveryRouter.createCaller({ effectiveOrgId: 'org-1' }); + await expect( + caller.discover({ + providerId: 'fake-expected', + capability: 'currentUser', + args: {}, + projectId: 'p', + }), + ).rejects.toMatchObject({ + code: 'NOT_FOUND', + message: expect.stringMatching(/different PM provider.*fake-other/), + }); + }); + + it('treats a non-object hook return (string/null/array) as empty — resolved bag contains only project_credentials', async () => { + const { createFakePMManifest, createFakePMProvider } = await import( + '../../helpers/fakePMProvider.js' + ); + + let receivedCredentials: Record | undefined; + registerPMProvider({ + ...createFakePMManifest(), + id: 'fake-bad-hook-return', + credentialRoles: [{ role: 'api_key', label: 'API Key', envVarKey: 'FAKE_API_KEY' }], + // Hook returns a non-object: must be ignored, must not crash. + configToCredentials: () => 'not-an-object' as unknown as Record, + createDiscoveryProvider: (opts) => { + receivedCredentials = opts?.credentials ?? {}; + const { provider } = createFakePMProvider(); + return provider; + }, + }); + + const { getIntegrationByProjectAndCategory } = await import( + '../../../src/db/repositories/integrationsRepository.js' + ); + const { getIntegrationCredentialOrNull } = await import('../../../src/config/provider.js'); + vi.mocked(getIntegrationByProjectAndCategory).mockResolvedValue({ + projectId: 'p', + category: 'pm', + provider: 'fake-bad-hook-return', + config: {}, + triggers: {}, + } as unknown as Awaited>); + vi.mocked(getIntegrationCredentialOrNull).mockResolvedValue('k'); + + const caller = pmDiscoveryRouter.createCaller({ effectiveOrgId: 'org-1' }); + await caller.discover({ + providerId: 'fake-bad-hook-return', + capability: 'currentUser', + args: {}, + projectId: 'p', + }); + + expect(receivedCredentials).toEqual({ api_key: 'k' }); + }); + + it('swallows hook exceptions and continues with project_credentials (logs a warn)', async () => { + const { createFakePMManifest, createFakePMProvider } = await import( + '../../helpers/fakePMProvider.js' + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + let receivedCredentials: Record | undefined; + registerPMProvider({ + ...createFakePMManifest(), + id: 'fake-throwing-hook', + credentialRoles: [{ role: 'api_key', label: 'API Key', envVarKey: 'FAKE_API_KEY' }], + // A broken hook MUST NOT take down discovery. + configToCredentials: () => { + throw new Error('hook boom'); + }, + createDiscoveryProvider: (opts) => { + receivedCredentials = opts?.credentials ?? {}; + const { provider } = createFakePMProvider(); + return provider; + }, + }); + + const { getIntegrationByProjectAndCategory } = await import( + '../../../src/db/repositories/integrationsRepository.js' + ); + const { getIntegrationCredentialOrNull } = await import('../../../src/config/provider.js'); + vi.mocked(getIntegrationByProjectAndCategory).mockResolvedValue({ + projectId: 'p', + category: 'pm', + provider: 'fake-throwing-hook', + config: {}, + triggers: {}, + } as unknown as Awaited>); + vi.mocked(getIntegrationCredentialOrNull).mockResolvedValue('k'); + + const caller = pmDiscoveryRouter.createCaller({ effectiveOrgId: 'org-1' }); + await caller.discover({ + providerId: 'fake-throwing-hook', + capability: 'currentUser', + args: {}, + projectId: 'p', + }); + + expect(receivedCredentials).toEqual({ api_key: 'k' }); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("configToCredentials threw for provider 'fake-throwing-hook'"), + expect.any(Error), + ); + warnSpy.mockRestore(); + }); }); describe('createCustomField (plan 010/1 task 2)', () => {