Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,22 @@ jest.mock('@trycompai/integration-platform', () => ({
getManifest: jest.fn(),
getActiveManifests: jest.fn(),
runAllChecks: jest.fn(),
// Default: not a code manifest, so isDynamic falls through to the
// dynamicIntegration lookup (the pre-guard behavior these tests rely on).
// A code-manifest case is exercised explicitly below.
isCodeManifest: jest.fn(() => false),
}));

import { db } from '@db';
import { getManifest, runAllChecks } from '@trycompai/integration-platform';
import {
getManifest,
isCodeManifest,
runAllChecks,
} from '@trycompai/integration-platform';

const mockedGetManifest = getManifest as jest.Mock;
const mockedRunAllChecks = runAllChecks as jest.Mock;
const mockedIsCodeManifest = isCodeManifest as jest.Mock;
// Grab through the module reference to avoid the `unbound-method` lint rule.
const mockedTask = db.task as unknown as {
findUnique: jest.Mock;
Expand Down Expand Up @@ -265,6 +274,12 @@ describe('TaskIntegrationsController', () => {
slug: 'aws',
});
mockedGetManifest.mockReturnValue(MANIFEST);
// Default: treat the provider as NOT a code manifest, so isDynamic falls
// through to the dynamicIntegration lookup (pre-guard behavior). The
// code-manifest case is exercised explicitly in its own test. Set here so a
// test that opts into `true` never leaks into the next (clearAllMocks keeps
// implementations).
mockedIsCodeManifest.mockReturnValue(false);
// Default: no active exceptions (existing tests behave as before).
mockFindingExceptionFindMany.mockResolvedValue([]);
mockCheckRunRepository.create.mockImplementation(() =>
Expand Down Expand Up @@ -509,6 +524,49 @@ describe('TaskIntegrationsController', () => {
);
});

it('treats a CODE-BASED provider as static even when a dynamic row shares its slug — shows the real fail, never held (CS-715)', async () => {
// A code manifest wins over a dynamic integration of the same slug, so the
// check is static. Pre-fix, the mere existence of the dynamic row forced the
// run to be HELD as 'inconclusive' (hidden from the customer). It must be
// classified static: a real finding shows 'failed' and fails the task.
mockedIsCodeManifest.mockReturnValue(true);
mockProviderRepository.findById.mockResolvedValue({
id: 'prov_gh',
slug: 'github',
});
// An active dynamic 'github' row exists — pre-fix this alone forced the hold.
mockDynamicIntegrationFindFirst.mockResolvedValue({ id: 'din_github' });
mockConnectionRepository.findById.mockResolvedValue({
id: 'conn_1',
organizationId: 'org_1',
providerId: 'prov_gh',
status: 'active',
});
mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([
{
id: 'conn_1',
organizationId: 'org_1',
providerId: 'prov_gh',
metadata: {},
variables: {},
},
]);
mockedRunAllChecks.mockResolvedValue(failingResult());

const result = await controller.runCheckForTask('task_1', 'org_1', {
connectionId: 'conn_1',
checkId: 'aws-s3-encryption',
});

// Not held: the finding fails the task and the run row is 'failed' (shown),
// even though an active dynamic row exists for the same slug.
expect(result.taskStatus).toBe('failed');
expect(mockCheckRunRepository.complete).toHaveBeenCalledWith(
'icr_x',
expect.objectContaining({ status: 'failed' }),
);
});

it('does NOT fail the task when the only finding is excepted (goes done)', async () => {
mockConnectionRepository.findById.mockResolvedValue({
id: 'conn_1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { OrganizationId } from '../../auth/auth-context.decorator';
import {
getActiveManifests,
getManifest,
isCodeManifest,
runAllChecks,
} from '@trycompai/integration-platform';
import { ConnectionRepository } from '../repositories/connection.repository';
Expand Down Expand Up @@ -397,10 +398,21 @@ export class TaskIntegrationsController {
// as 'inconclusive' — both on each per-account run row (so the customer never
// sees it and the self-heal agent picks it up) AND excluded from task status
// below. Mirrors the scheduled path.
const isDynamic = !!(await db.dynamicIntegration.findFirst({
where: { slug: provider.slug, isActive: true },
select: { id: true },
}));
//
// A code-based manifest ALWAYS wins over a dynamic integration of the same
// slug (registry precedence), so the check we just resolved is the CODE one —
// it must be classified statically, never held as 'inconclusive'. Several
// providers (github, vercel, aikido, rippling) have BOTH a code manifest and
// an active DynamicIntegration row for their extra DB-backed checks; keying
// `isDynamic` off the DB row alone wrongly hid every code-check finding from
// the manual run (CS-715). Only a provider with NO code manifest is dynamic —
// matching the scheduled and server-run paths.
const isDynamic = isCodeManifest(provider.slug)
? false
: !!(await db.dynamicIntegration.findFirst({
where: { slug: provider.slug, isActive: true },
select: { id: true },
}));

let totalFindings = 0;
let totalPassing = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,45 @@ describe('InternalIntegrationDebugService', () => {
);
});

it('reveals a CODE-BASED check as failed instead of re-holding it, even when a dynamic row shares the slug (CS-715)', async () => {
// github is a code manifest (wins over any dynamic integration of the same
// slug). A failing code check must be shown, never re-held as inconclusive —
// the self-heal agent cannot patch code, so re-holding would hide it forever.
mockedDb.integrationConnection.findUnique.mockResolvedValue({
organizationId: 'org_1',
provider: { slug: 'github' },
});
// An active dynamic 'github' row exists — pre-fix this alone forced a hold.
mockedDb.dynamicIntegration.findFirst.mockResolvedValue({
id: 'din_github',
});
const runChecks = jest.fn().mockResolvedValue(
runResult('failed', [
{
resourceType: 'repository',
resourceId: 'org/repo',
title: 'Dependabot not enabled',
description: 'not enabled',
evidence: { status: 'disabled' },
},
]),
);
const repo = makeRepo();

const service = makeService({ runChecks }, repo);
const out = await service.rerunAndPersistCheck({
connectionId: 'icn_1',
checkId: 'dependabot_enabled',
taskId: 'task_1',
});

expect(out.status).toBe('failed');
expect(repo.complete).toHaveBeenCalledWith(
'icr_new',
expect.objectContaining({ status: 'failed', failedCount: 1 }),
);
});

it('refreshes the manifest cache BEFORE running (so a just-patched fix is live, not the 60s-stale code)', async () => {
mockedDb.integrationConnection.findUnique.mockResolvedValue({
organizationId: 'org_1',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injectable, Logger, NotFoundException } from '@nestjs/common';
import { db } from '@db';
import type { Prisma } from '@db';
import { isCodeManifest } from '@trycompai/integration-platform';
import {
ConnectionCheckRunnerService,
type RunAllChecksResult,
Expand Down Expand Up @@ -531,10 +532,19 @@ export class InternalIntegrationDebugService {
);
}

const isDynamic = !!(await db.dynamicIntegration.findFirst({
where: { slug: connection.provider?.slug ?? '', isActive: true },
select: { id: true },
}));
// A code-based manifest wins over a dynamic integration of the same slug, so
// its checks are static and must NEVER be re-held as 'inconclusive' here —
// otherwise the self-heal agent's re-run would re-hide a code check it cannot
// patch, looping it as hidden forever (CS-715). Mirrors the run-check paths:
// only a provider with NO code manifest is dynamic. Dynamic integrations are
// unaffected (isCodeManifest is false for them → same DB check as before).
const providerSlug = connection.provider?.slug ?? '';
const isDynamic = isCodeManifest(providerSlug)
? false
: !!(await db.dynamicIntegration.findFirst({
where: { slug: providerSlug, isActive: true },
select: { id: true },
}));

const status = decideRunStatus({
resultStatus: checkResult.status,
Expand Down
1 change: 1 addition & 0 deletions packages/integration-platform/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export {
getIntegrationIds,
getManifest,
getOAuthConfig,
isCodeManifest,
registry,
requiresOAuth,
} from './registry';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { afterEach, describe, expect, it } from 'bun:test';
import type { IntegrationManifest } from '../../types';
import { getManifest, isCodeManifest, registry } from '../index';

/**
* CS-715 regression guard.
*
* The manual "Run a check" path classifies a run as dynamic (→ held as an
* 'inconclusive', customer-hidden result handed to the self-heal agent) vs static
* (→ a plain failed/pass shown to the customer). Several providers ship a
* code-based manifest AND an active DynamicIntegration row of the same slug (the
* latter only for extra DB-backed checks, e.g. GitHub's Code Changes / Employee
* Access). Because a code manifest always WINS in the registry, the check that
* actually runs is the code one and must be treated as static.
*
* Keying "is this dynamic?" off "does a DynamicIntegration row exist for the
* slug?" hid every code-based finding (e.g. Dependabot) from the manual run.
* `isCodeManifest` is the authoritative signal callers must use instead.
*/
describe('isCodeManifest', () => {
it('returns true for every bundled (code) manifest', () => {
// Every id the registry loaded from code must report as a code manifest.
for (const manifest of registry.getAllManifests()) {
expect(isCodeManifest(manifest.id)).toBe(true);
}
});

it('returns true for known code-based providers that also have dynamic rows', () => {
// These four ship a code manifest AND (in prod) an active DynamicIntegration
// row of the same slug — exactly the CS-715 case. They must stay code.
for (const slug of ['github', 'vercel', 'aikido', 'rippling']) {
expect(isCodeManifest(slug)).toBe(true);
}
});

it('returns false for an unknown slug', () => {
expect(isCodeManifest('definitely-not-a-real-provider')).toBe(false);
});

describe('with a dynamic manifest registered', () => {
const DYNAMIC_ONLY_ID = 'cs715-dynamic-only-fixture';

afterEach(() => {
registry.unregisterDynamic(DYNAMIC_ONLY_ID);
});

it('treats a dynamic-only manifest (no code counterpart) as NOT a code manifest', () => {
const dynamic: IntegrationManifest = {
id: DYNAMIC_ONLY_ID,
name: 'CS715 Dynamic Only',
auth: { type: 'custom', config: {} },
capabilities: ['checks'],
} as unknown as IntegrationManifest;

registry.registerDynamic(dynamic);

// Resolvable as a manifest (so a run can execute it)...
expect(getManifest(DYNAMIC_ONLY_ID)).toBeDefined();
// ...but NOT a code manifest, so it is correctly classified dynamic.
expect(isCodeManifest(DYNAMIC_ONLY_ID)).toBe(false);
});

it('a dynamic registration cannot override a code manifest of the same id', () => {
// registerDynamic is a no-op for a code id, so github stays code — the
// core CS-715 invariant.
registry.registerDynamic({
id: 'github',
name: 'Shadow GitHub',
auth: { type: 'custom', config: {} },
capabilities: ['checks'],
} as unknown as IntegrationManifest);

expect(isCodeManifest('github')).toBe(true);
});
});
});
15 changes: 15 additions & 0 deletions packages/integration-platform/src/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ class IntegrationRegistryImpl implements IntegrationRegistry {
return this.manifests.get(id);
}

isCodeManifest(id: string): boolean {
return this.codeManifestIds.has(id);
}

getAllManifests(): IntegrationManifest[] {
return Array.from(this.manifests.values());
}
Expand Down Expand Up @@ -163,6 +167,17 @@ export function getManifest(id: string): IntegrationManifest | undefined {
return registry.getManifest(id);
}

/**
* Whether `id` is a code-based (bundled) manifest. A code manifest always wins
* over a dynamic (DB-loaded) manifest of the same slug (registry precedence), so
* its checks must be treated as static — never held as an "inconclusive" dynamic
* result. Use this instead of "an active DynamicIntegration row exists for the
* slug" when deciding whether a run is dynamic.
*/
export function isCodeManifest(id: string): boolean {
return registry.isCodeManifest(id);
}

/**
* Get all available manifests
*/
Expand Down
8 changes: 8 additions & 0 deletions packages/integration-platform/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,14 @@ export interface IntegrationRegistry {
/** Get manifest by ID */
getManifest(id: string): IntegrationManifest | undefined;

/**
* True when `id` is a code-based (bundled) manifest — one that can never be
* overridden by a dynamic (DB-loaded) manifest of the same id. Callers use this
* to classify a provider's checks as static even when a dynamic integration of
* the same slug also exists (the code manifest always wins).
*/
isCodeManifest(id: string): boolean;

/** Get all manifests */
getAllManifests(): IntegrationManifest[];

Expand Down
Loading