Skip to content

Commit 200b191

Browse files
tofikwestclaude
andauthored
fix(integration-platform): classify code-based checks statically on manual run (CS-715) (#3376)
* 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRPjA4fQxdXuELyfTjYF8b --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f5f6c50 commit 200b191

8 files changed

Lines changed: 228 additions & 9 deletions

File tree

‎apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,22 @@ jest.mock('@trycompai/integration-platform', () => ({
3333
getManifest: jest.fn(),
3434
getActiveManifests: jest.fn(),
3535
runAllChecks: jest.fn(),
36+
// Default: not a code manifest, so isDynamic falls through to the
37+
// dynamicIntegration lookup (the pre-guard behavior these tests rely on).
38+
// A code-manifest case is exercised explicitly below.
39+
isCodeManifest: jest.fn(() => false),
3640
}));
3741

3842
import { db } from '@db';
39-
import { getManifest, runAllChecks } from '@trycompai/integration-platform';
43+
import {
44+
getManifest,
45+
isCodeManifest,
46+
runAllChecks,
47+
} from '@trycompai/integration-platform';
4048

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

527+
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 () => {
528+
// A code manifest wins over a dynamic integration of the same slug, so the
529+
// check is static. Pre-fix, the mere existence of the dynamic row forced the
530+
// run to be HELD as 'inconclusive' (hidden from the customer). It must be
531+
// classified static: a real finding shows 'failed' and fails the task.
532+
mockedIsCodeManifest.mockReturnValue(true);
533+
mockProviderRepository.findById.mockResolvedValue({
534+
id: 'prov_gh',
535+
slug: 'github',
536+
});
537+
// An active dynamic 'github' row exists — pre-fix this alone forced the hold.
538+
mockDynamicIntegrationFindFirst.mockResolvedValue({ id: 'din_github' });
539+
mockConnectionRepository.findById.mockResolvedValue({
540+
id: 'conn_1',
541+
organizationId: 'org_1',
542+
providerId: 'prov_gh',
543+
status: 'active',
544+
});
545+
mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([
546+
{
547+
id: 'conn_1',
548+
organizationId: 'org_1',
549+
providerId: 'prov_gh',
550+
metadata: {},
551+
variables: {},
552+
},
553+
]);
554+
mockedRunAllChecks.mockResolvedValue(failingResult());
555+
556+
const result = await controller.runCheckForTask('task_1', 'org_1', {
557+
connectionId: 'conn_1',
558+
checkId: 'aws-s3-encryption',
559+
});
560+
561+
// Not held: the finding fails the task and the run row is 'failed' (shown),
562+
// even though an active dynamic row exists for the same slug.
563+
expect(result.taskStatus).toBe('failed');
564+
expect(mockCheckRunRepository.complete).toHaveBeenCalledWith(
565+
'icr_x',
566+
expect.objectContaining({ status: 'failed' }),
567+
);
568+
});
569+
512570
it('does NOT fail the task when the only finding is excepted (goes done)', async () => {
513571
mockConnectionRepository.findById.mockResolvedValue({
514572
id: 'conn_1',

‎apps/api/src/integration-platform/controllers/task-integrations.controller.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { OrganizationId } from '../../auth/auth-context.decorator';
2626
import {
2727
getActiveManifests,
2828
getManifest,
29+
isCodeManifest,
2930
runAllChecks,
3031
} from '@trycompai/integration-platform';
3132
import { ConnectionRepository } from '../repositories/connection.repository';
@@ -397,10 +398,21 @@ export class TaskIntegrationsController {
397398
// as 'inconclusive' — both on each per-account run row (so the customer never
398399
// sees it and the self-heal agent picks it up) AND excluded from task status
399400
// below. Mirrors the scheduled path.
400-
const isDynamic = !!(await db.dynamicIntegration.findFirst({
401-
where: { slug: provider.slug, isActive: true },
402-
select: { id: true },
403-
}));
401+
//
402+
// A code-based manifest ALWAYS wins over a dynamic integration of the same
403+
// slug (registry precedence), so the check we just resolved is the CODE one —
404+
// it must be classified statically, never held as 'inconclusive'. Several
405+
// providers (github, vercel, aikido, rippling) have BOTH a code manifest and
406+
// an active DynamicIntegration row for their extra DB-backed checks; keying
407+
// `isDynamic` off the DB row alone wrongly hid every code-check finding from
408+
// the manual run (CS-715). Only a provider with NO code manifest is dynamic —
409+
// matching the scheduled and server-run paths.
410+
const isDynamic = isCodeManifest(provider.slug)
411+
? false
412+
: !!(await db.dynamicIntegration.findFirst({
413+
where: { slug: provider.slug, isActive: true },
414+
select: { id: true },
415+
}));
404416

405417
let totalFindings = 0;
406418
let totalPassing = 0;

‎apps/api/src/integration-platform/services/internal-integration-debug.service.spec.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,45 @@ describe('InternalIntegrationDebugService', () => {
544544
);
545545
});
546546

547+
it('reveals a CODE-BASED check as failed instead of re-holding it, even when a dynamic row shares the slug (CS-715)', async () => {
548+
// github is a code manifest (wins over any dynamic integration of the same
549+
// slug). A failing code check must be shown, never re-held as inconclusive —
550+
// the self-heal agent cannot patch code, so re-holding would hide it forever.
551+
mockedDb.integrationConnection.findUnique.mockResolvedValue({
552+
organizationId: 'org_1',
553+
provider: { slug: 'github' },
554+
});
555+
// An active dynamic 'github' row exists — pre-fix this alone forced a hold.
556+
mockedDb.dynamicIntegration.findFirst.mockResolvedValue({
557+
id: 'din_github',
558+
});
559+
const runChecks = jest.fn().mockResolvedValue(
560+
runResult('failed', [
561+
{
562+
resourceType: 'repository',
563+
resourceId: 'org/repo',
564+
title: 'Dependabot not enabled',
565+
description: 'not enabled',
566+
evidence: { status: 'disabled' },
567+
},
568+
]),
569+
);
570+
const repo = makeRepo();
571+
572+
const service = makeService({ runChecks }, repo);
573+
const out = await service.rerunAndPersistCheck({
574+
connectionId: 'icn_1',
575+
checkId: 'dependabot_enabled',
576+
taskId: 'task_1',
577+
});
578+
579+
expect(out.status).toBe('failed');
580+
expect(repo.complete).toHaveBeenCalledWith(
581+
'icr_new',
582+
expect.objectContaining({ status: 'failed', failedCount: 1 }),
583+
);
584+
});
585+
547586
it('refreshes the manifest cache BEFORE running (so a just-patched fix is live, not the 60s-stale code)', async () => {
548587
mockedDb.integrationConnection.findUnique.mockResolvedValue({
549588
organizationId: 'org_1',

‎apps/api/src/integration-platform/services/internal-integration-debug.service.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Injectable, Logger, NotFoundException } from '@nestjs/common';
22
import { db } from '@db';
33
import type { Prisma } from '@db';
4+
import { isCodeManifest } from '@trycompai/integration-platform';
45
import {
56
ConnectionCheckRunnerService,
67
type RunAllChecksResult,
@@ -531,10 +532,19 @@ export class InternalIntegrationDebugService {
531532
);
532533
}
533534

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

539549
const status = decideRunStatus({
540550
resultStatus: checkResult.status,

‎packages/integration-platform/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export {
7070
getIntegrationIds,
7171
getManifest,
7272
getOAuthConfig,
73+
isCodeManifest,
7374
registry,
7475
requiresOAuth,
7576
} from './registry';
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { afterEach, describe, expect, it } from 'bun:test';
2+
import type { IntegrationManifest } from '../../types';
3+
import { getManifest, isCodeManifest, registry } from '../index';
4+
5+
/**
6+
* CS-715 regression guard.
7+
*
8+
* The manual "Run a check" path classifies a run as dynamic (→ held as an
9+
* 'inconclusive', customer-hidden result handed to the self-heal agent) vs static
10+
* (→ a plain failed/pass shown to the customer). Several providers ship a
11+
* code-based manifest AND an active DynamicIntegration row of the same slug (the
12+
* latter only for extra DB-backed checks, e.g. GitHub's Code Changes / Employee
13+
* Access). Because a code manifest always WINS in the registry, the check that
14+
* actually runs is the code one and must be treated as static.
15+
*
16+
* Keying "is this dynamic?" off "does a DynamicIntegration row exist for the
17+
* slug?" hid every code-based finding (e.g. Dependabot) from the manual run.
18+
* `isCodeManifest` is the authoritative signal callers must use instead.
19+
*/
20+
describe('isCodeManifest', () => {
21+
it('returns true for every bundled (code) manifest', () => {
22+
// Every id the registry loaded from code must report as a code manifest.
23+
for (const manifest of registry.getAllManifests()) {
24+
expect(isCodeManifest(manifest.id)).toBe(true);
25+
}
26+
});
27+
28+
it('returns true for known code-based providers that also have dynamic rows', () => {
29+
// These four ship a code manifest AND (in prod) an active DynamicIntegration
30+
// row of the same slug — exactly the CS-715 case. They must stay code.
31+
for (const slug of ['github', 'vercel', 'aikido', 'rippling']) {
32+
expect(isCodeManifest(slug)).toBe(true);
33+
}
34+
});
35+
36+
it('returns false for an unknown slug', () => {
37+
expect(isCodeManifest('definitely-not-a-real-provider')).toBe(false);
38+
});
39+
40+
describe('with a dynamic manifest registered', () => {
41+
const DYNAMIC_ONLY_ID = 'cs715-dynamic-only-fixture';
42+
43+
afterEach(() => {
44+
registry.unregisterDynamic(DYNAMIC_ONLY_ID);
45+
});
46+
47+
it('treats a dynamic-only manifest (no code counterpart) as NOT a code manifest', () => {
48+
const dynamic: IntegrationManifest = {
49+
id: DYNAMIC_ONLY_ID,
50+
name: 'CS715 Dynamic Only',
51+
auth: { type: 'custom', config: {} },
52+
capabilities: ['checks'],
53+
} as unknown as IntegrationManifest;
54+
55+
registry.registerDynamic(dynamic);
56+
57+
// Resolvable as a manifest (so a run can execute it)...
58+
expect(getManifest(DYNAMIC_ONLY_ID)).toBeDefined();
59+
// ...but NOT a code manifest, so it is correctly classified dynamic.
60+
expect(isCodeManifest(DYNAMIC_ONLY_ID)).toBe(false);
61+
});
62+
63+
it('a dynamic registration cannot override a code manifest of the same id', () => {
64+
// registerDynamic is a no-op for a code id, so github stays code — the
65+
// core CS-715 invariant.
66+
registry.registerDynamic({
67+
id: 'github',
68+
name: 'Shadow GitHub',
69+
auth: { type: 'custom', config: {} },
70+
capabilities: ['checks'],
71+
} as unknown as IntegrationManifest);
72+
73+
expect(isCodeManifest('github')).toBe(true);
74+
});
75+
});
76+
});

‎packages/integration-platform/src/registry/index.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ class IntegrationRegistryImpl implements IntegrationRegistry {
106106
return this.manifests.get(id);
107107
}
108108

109+
isCodeManifest(id: string): boolean {
110+
return this.codeManifestIds.has(id);
111+
}
112+
109113
getAllManifests(): IntegrationManifest[] {
110114
return Array.from(this.manifests.values());
111115
}
@@ -163,6 +167,17 @@ export function getManifest(id: string): IntegrationManifest | undefined {
163167
return registry.getManifest(id);
164168
}
165169

170+
/**
171+
* Whether `id` is a code-based (bundled) manifest. A code manifest always wins
172+
* over a dynamic (DB-loaded) manifest of the same slug (registry precedence), so
173+
* its checks must be treated as static — never held as an "inconclusive" dynamic
174+
* result. Use this instead of "an active DynamicIntegration row exists for the
175+
* slug" when deciding whether a run is dynamic.
176+
*/
177+
export function isCodeManifest(id: string): boolean {
178+
return registry.isCodeManifest(id);
179+
}
180+
166181
/**
167182
* Get all available manifests
168183
*/

‎packages/integration-platform/src/types.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,14 @@ export interface IntegrationRegistry {
901901
/** Get manifest by ID */
902902
getManifest(id: string): IntegrationManifest | undefined;
903903

904+
/**
905+
* True when `id` is a code-based (bundled) manifest — one that can never be
906+
* overridden by a dynamic (DB-loaded) manifest of the same id. Callers use this
907+
* to classify a provider's checks as static even when a dynamic integration of
908+
* the same slug also exists (the code manifest always wins).
909+
*/
910+
isCodeManifest(id: string): boolean;
911+
904912
/** Get all manifests */
905913
getAllManifests(): IntegrationManifest[];
906914

0 commit comments

Comments
 (0)