From 200b191aed0148e780dd868f144fb30b553014f1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Wed, 8 Jul 2026 18:44:06 -0400 Subject: [PATCH] fix(integration-platform): classify code-based checks statically on manual run (CS-715) (#3376) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(integration-platform): classify code-based checks statically on manual run (CS-715) The manual "run a check" endpoint decided whether a run was dynamic (held as an 'inconclusive' result for the self-heal agent, and hidden from the run history) versus static (shown as failed/pass) by checking only whether an active DynamicIntegration row existed for the provider slug. Several providers ship a code-based manifest AND an active DynamicIntegration row of the same slug for their extra DB-backed checks. A code manifest always wins in the registry, so the check that actually runs is the code one and must be classified statically. Keying `isDynamic` off the DB row alone marked those code-check runs 'inconclusive', so their results were withheld from the manual run while the scheduled and server-run paths (which guard on the code manifest) kept showing them correctly. Add a registry helper `isCodeManifest` and gate the manual-run `isDynamic` on it, mirroring the scheduled/server-run paths: a provider with a code manifest is never dynamic; only a provider with no code manifest is. Add unit tests covering the bundled manifests, the mixed code+dynamic providers, and the dynamic-only case. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CRPjA4fQxdXuELyfTjYF8b * fix(integration-platform): guard self-heal re-run classification for code-based checks (CS-715) Extend the isCodeManifest guard from the manual run-check path to the self-heal agent's re-run (rerunAndPersistCheck). A code-based provider that also has a dynamic integration row of the same slug (github/vercel/aikido/rippling) was re-held as 'inconclusive' on re-run — and since the agent cannot patch a code check, it stayed hidden in a loop. It is now classified statically and its real result is shown. Dynamic integrations are unaffected (isCodeManifest is false for them, so the dynamic lookup runs exactly as before). Tests: - self-heal re-run reveals a code-based check as 'failed' (not re-held), even when a dynamic row shares the slug. - manual run-check treats a code-based provider as static under the same condition; adds the isCodeManifest mock the controller spec now requires. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CRPjA4fQxdXuELyfTjYF8b --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../task-integrations.controller.spec.ts | 60 ++++++++++++++- .../task-integrations.controller.ts | 20 ++++- ...internal-integration-debug.service.spec.ts | 39 ++++++++++ .../internal-integration-debug.service.ts | 18 ++++- packages/integration-platform/src/index.ts | 1 + .../__tests__/is-code-manifest.test.ts | 76 +++++++++++++++++++ .../src/registry/index.ts | 15 ++++ packages/integration-platform/src/types.ts | 8 ++ 8 files changed, 228 insertions(+), 9 deletions(-) create mode 100644 packages/integration-platform/src/registry/__tests__/is-code-manifest.test.ts diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts index 9320a51618..540aa9085a 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts @@ -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; @@ -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(() => @@ -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', diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts index 19acb78c37..b41a12db51 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts @@ -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'; @@ -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; diff --git a/apps/api/src/integration-platform/services/internal-integration-debug.service.spec.ts b/apps/api/src/integration-platform/services/internal-integration-debug.service.spec.ts index 013cf76b99..17033797f1 100644 --- a/apps/api/src/integration-platform/services/internal-integration-debug.service.spec.ts +++ b/apps/api/src/integration-platform/services/internal-integration-debug.service.spec.ts @@ -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', diff --git a/apps/api/src/integration-platform/services/internal-integration-debug.service.ts b/apps/api/src/integration-platform/services/internal-integration-debug.service.ts index 1cc7e97fe5..139e01af9b 100644 --- a/apps/api/src/integration-platform/services/internal-integration-debug.service.ts +++ b/apps/api/src/integration-platform/services/internal-integration-debug.service.ts @@ -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, @@ -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, diff --git a/packages/integration-platform/src/index.ts b/packages/integration-platform/src/index.ts index b50fed9c72..a3d92f852f 100644 --- a/packages/integration-platform/src/index.ts +++ b/packages/integration-platform/src/index.ts @@ -70,6 +70,7 @@ export { getIntegrationIds, getManifest, getOAuthConfig, + isCodeManifest, registry, requiresOAuth, } from './registry'; diff --git a/packages/integration-platform/src/registry/__tests__/is-code-manifest.test.ts b/packages/integration-platform/src/registry/__tests__/is-code-manifest.test.ts new file mode 100644 index 0000000000..2e8bb8c220 --- /dev/null +++ b/packages/integration-platform/src/registry/__tests__/is-code-manifest.test.ts @@ -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); + }); + }); +}); diff --git a/packages/integration-platform/src/registry/index.ts b/packages/integration-platform/src/registry/index.ts index 2d0dc43322..314ab3beae 100644 --- a/packages/integration-platform/src/registry/index.ts +++ b/packages/integration-platform/src/registry/index.ts @@ -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()); } @@ -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 */ diff --git a/packages/integration-platform/src/types.ts b/packages/integration-platform/src/types.ts index e56123ce2b..47b9c7b7f3 100644 --- a/packages/integration-platform/src/types.ts +++ b/packages/integration-platform/src/types.ts @@ -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[];