From 82eb1f3c069a14f2b9b5213285b0503f7cc5a1bb Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 15 Sep 2026 15:02:18 +0000 Subject: [PATCH] fix(compliance): correct two validate_input gates and coverage floors Require validate_input for CTV experience and premium display validation while retaining controller workflow prerequisites. Verify source and generated bundle selection, unavailable-fixture neutrality and genuine validator failures. Reconcile only the 32 controller seed passes removed from brand and governance in both synchronized matrix floors. Prove the tenants lack the validator through live tool discovery and exact old-gate adapter replay. Preserve all other floors. Related issue: #7404 Required order: #7506 web+worker deploy, #7510, refreshed #7507 deploy, then public-card verification. This patch alone does not complete the tracking issue. --- .../fix-validate-input-controller-gates.md | 8 + .../workflows/training-agent-storyboards.yml | 9 +- scripts/run-storyboards-matrix.sh | 8 +- .../training-agent-tool-catalog-drift.test.ts | 57 ++++- .../ctv-experience-validate-input.yaml | 1 - .../premium-display-canonical-validation.yaml | 1 - .../compliance-validate-input-gates.test.ts | 207 ++++++++++++++++++ tests/run-storyboards-sharding.test.cjs | 15 +- 8 files changed, 290 insertions(+), 16 deletions(-) create mode 100644 .changeset/fix-validate-input-controller-gates.md create mode 100644 tests/addie/compliance-validate-input-gates.test.ts diff --git a/.changeset/fix-validate-input-controller-gates.md b/.changeset/fix-validate-input-controller-gates.md new file mode 100644 index 00000000000..372ccd8698f --- /dev/null +++ b/.changeset/fix-validate-input-controller-gates.md @@ -0,0 +1,8 @@ +--- +"adcontextprotocol": patch +--- + +Gate the 3.2 CTV experience and premium display validation storyboards on +validate_input so controller-only agents are not selected. Retain the controller +workflow requirement for fixture setup. This is a narrow part of #7404; it does +not complete the hosted compliance landing, deployment, or public-card checks. diff --git a/.github/workflows/training-agent-storyboards.yml b/.github/workflows/training-agent-storyboards.yml index d8a5462d7ec..2b6637373bd 100644 --- a/.github/workflows/training-agent-storyboards.yml +++ b/.github/workflows/training-agent-storyboards.yml @@ -73,6 +73,11 @@ jobs: # Selection comes from each tenant's live get_adcp_capabilities # declaration; out-of-scope and quarantined storyboards are reported # separately from the result rows graded by these floors. + # /brand and /governance do not implement validate_input. Correcting + # the CTV and premium-display gates removes 14 + 18 controller seed + # passes, not validator coverage (those checks all skipped). Subtract + # only those 32 false-coverage steps: 115 -> 83 and 160 -> 128. + # Clean-storyboard, other-tenant, and 3.0-compat floors stay unchanged. - surface: current tenant: signals min_clean_storyboards: 45 @@ -80,7 +85,7 @@ jobs: - surface: current tenant: governance min_clean_storyboards: 47 - min_passing_steps: 160 + min_passing_steps: 128 - surface: current tenant: creative min_clean_storyboards: 49 @@ -92,7 +97,7 @@ jobs: - surface: current tenant: brand min_clean_storyboards: 45 - min_passing_steps: 115 + min_passing_steps: 83 - surface: current tenant: si min_clean_storyboards: 42 diff --git a/scripts/run-storyboards-matrix.sh b/scripts/run-storyboards-matrix.sh index 641fe1f8548..b33b5b14b76 100755 --- a/scripts/run-storyboards-matrix.sh +++ b/scripts/run-storyboards-matrix.sh @@ -267,12 +267,16 @@ else # Ratcheted from the first capability-resolved replay. The runner reports # declared-scope applicability and quarantines separately from these # clean-result-row and passing-step regression floors. + # /brand and /governance do not implement validate_input. The corrected + # CTV + premium-display gates remove 14 + 18 controller seed passes; all + # validator checks previously skipped. Only subtract those 32 false-coverage + # steps (115 -> 83, 160 -> 128); retain every clean/other/3.0-compat floor. "signals:45:80" "sales:133:632" - "governance:47:160" + "governance:47:128" "creative:49:209" "creative-builder:50:184" - "brand:45:115" + "brand:45:83" "si:42:50" ) fi diff --git a/server/tests/integration/training-agent-tool-catalog-drift.test.ts b/server/tests/integration/training-agent-tool-catalog-drift.test.ts index c60aed306f3..1a7641a5099 100644 --- a/server/tests/integration/training-agent-tool-catalog-drift.test.ts +++ b/server/tests/integration/training-agent-tool-catalog-drift.test.ts @@ -20,6 +20,10 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import express from 'express'; import http from 'node:http'; import { AddressInfo } from 'node:net'; +import { readFileSync } from 'node:fs'; +import YAML from 'yaml'; +import { runStoryboard, type Storyboard } from '@adcp/sdk/testing'; +import { TRAINING_AGENT_CURRENT_ADCP_VERSION } from '../../src/training-agent/types.js'; vi.hoisted(() => { process.env.PUBLIC_TEST_AGENT_TOKEN = 'tool-catalog-drift-token'; @@ -43,7 +47,7 @@ interface ToolListResponse { result?: { tools?: Array<{ name: string }> }; } -async function listTools(baseUrl: string, tenantId: string): Promise { +async function listTools(baseUrl: string, tenantId: string, includeUtility = false): Promise { const url = `${baseUrl}/api/training-agent/${tenantId}/mcp`; // MCP requires an `initialize` handshake before tools/list works. await fetch(url, { @@ -84,7 +88,7 @@ async function listTools(baseUrl: string, tenantId: string): Promise { ]); return (body.result?.tools ?? []) .map(t => t.name) - .filter(name => !NON_PROTOCOL_TOOLS.has(name)) + .filter(name => includeUtility || !NON_PROTOCOL_TOOLS.has(name)) .sort(); } @@ -141,6 +145,55 @@ describe('tool-catalog drift detection', () => { } }); + it.each(['brand', 'governance'])('/%s loses only controller setup passes from the two corrected validator gates', async tenant => { + // Query the actual router: the catalog alone cannot prove SDK registration + // or adapter behavior. Neither tenant implements the validator capability. + const tools = await listTools(baseUrl, tenant, true); + expect(tools).toContain('comply_test_controller'); + expect(tools).not.toContain('validate_input'); + expect(toolsForTenant(tenant)).not.toContain('validate_input'); + for (const [name, seedPasses] of [ + ['ctv-experience-validate-input', 14], + ['premium-display-canonical-validation', 18], + ] as const) { + const storyboard = YAML.parse(readFileSync(new URL( + `../../../static/compliance/source/universal/${name}.yaml`, import.meta.url, + ), 'utf8')) as Storyboard; + const options = { + auth: { type: 'bearer' as const, token: 'tool-catalog-drift-token' }, + allow_http: true, + wireAdcpVersion: TRAINING_AGENT_CURRENT_ADCP_VERSION, + agentTools: tools, + }; + const url = `${baseUrl}/api/training-agent/${tenant}/mcp`; + // Reconstruct just the old OR gate to explain the intentional floor + // correction. Production source and all runtime wiring stay untouched. + const previous = await runStoryboard(url, { + ...storyboard, required_tools: ['validate_input', 'comply_test_controller'], + }, options); + expect(previous.failed_count).toBe(0); + expect(previous.passed_count).toBe(seedPasses); + const steps = previous.phases.flatMap(phase => phase.steps); + const passed = steps.filter(step => step.passed && !step.skipped); + expect(passed).toHaveLength(seedPasses); + expect(passed.every(step => step.task === 'comply_test_controller' + && step.step_id.startsWith('seed_product.'))).toBe(true); + const validators = steps.filter(step => step.task === 'validate_input'); + expect(validators).toHaveLength(storyboard.phases.reduce((n, phase) => n + phase.steps.length, 0)); + expect(validators.every(step => step.skipped && step.skip_reason === 'missing_tool')).toBe(true); + + expect(storyboard.required_tools).toEqual(['validate_input']); + expect(storyboard.requires).toEqual(['controller']); + const corrected = await runStoryboard(url, storyboard, options); + expect(corrected.passed_count).toBe(0); + expect(corrected.failed_count).toBe(0); + expect(corrected.skipped_count).toBe(1); + expect(corrected.phases.flatMap(phase => phase.steps)).toEqual([ + expect.objectContaining({ skipped: true, skip_reason: 'missing_tool' }), + ]); + } + }, 60_000); + it('keeps AdCP 3.2 product-discovery tools out of the 3.0 compatibility catalog', () => { const compatibilityCatalog = toolsForTenant('sales', { storyboardCompat: { version: '3.0' }, diff --git a/static/compliance/source/universal/ctv-experience-validate-input.yaml b/static/compliance/source/universal/ctv-experience-validate-input.yaml index 9b85c8a41d6..7c026212963 100644 --- a/static/compliance/source/universal/ctv-experience-validate-input.yaml +++ b/static/compliance/source/universal/ctv-experience-validate-input.yaml @@ -56,7 +56,6 @@ track: creative introduced_in: "3.2" required_tools: - validate_input - - comply_test_controller requires: - controller diff --git a/static/compliance/source/universal/premium-display-canonical-validation.yaml b/static/compliance/source/universal/premium-display-canonical-validation.yaml index b9bdc471db8..ebbd3ef421b 100644 --- a/static/compliance/source/universal/premium-display-canonical-validation.yaml +++ b/static/compliance/source/universal/premium-display-canonical-validation.yaml @@ -7,7 +7,6 @@ track: creative introduced_in: "3.2" required_tools: - validate_input - - comply_test_controller requires: - controller diff --git a/tests/addie/compliance-validate-input-gates.test.ts b/tests/addie/compliance-validate-input-gates.test.ts new file mode 100644 index 00000000000..8e018316ea0 --- /dev/null +++ b/tests/addie/compliance-validate-input-gates.test.ts @@ -0,0 +1,207 @@ +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import YAML from 'yaml'; +import { + getComplianceStoryboardById, + runStoryboard, + type ComplianceResult, + type Storyboard, + type TestScenario, + type TestStepResult, +} from '@adcp/sdk/testing'; +import { + complianceResultToDbInput, + deriveStoryboardStatuses, +} from '../../server/src/addie/services/compliance-testing.js'; + +const require = createRequire(import.meta.url); +const { buildTo } = require('../../scripts/build-compliance.cjs'); +const { version } = require('../../package.json'); +const sourceDir = path.resolve('static/compliance/source'); +const targets = ['ctv-experience-validate-input', 'premium-display-canonical-validation']; +let bundleDir: string; + +beforeAll(() => { + bundleDir = mkdtempSync(path.join(tmpdir(), 'validate-input-gates-')); + // Exercise the publication builder and SDK cache loader without rewriting + // any immutable released bundle or depending on a stale local latest build. + buildTo(bundleDir, version, sourceDir); +}); + +afterAll(() => rmSync(bundleDir, { recursive: true, force: true })); + +function loadStoryboard(name: string, surface: string): Storyboard { + const source = YAML.parse(readFileSync(path.join(sourceDir, 'universal', `${name}.yaml`), 'utf8')); + if (surface === 'source') return source; + const published = getComplianceStoryboardById(source.id, { complianceDir: bundleDir, version }); + expect(published).toBeDefined(); + expect(published!.adcp_version).toBe(version); + // The loader stamps the release version. Omit only that transport setting + // for offline injected clients; all authored gates, fixtures and checks stay. + return { ...published!, adcp_version: undefined }; +} + +function runOptions(tools: string[], client: object) { + return { + agentTools: tools, + _profile: { tools }, + _client: client, + }; +} + +type ProjectedStep = TestStepResult & { step_id: string }; + +function complianceResult(storyboard: Storyboard, steps: ProjectedStep[]): ComplianceResult { + return { + agent_url: 'https://agent.example/mcp', + adcp_version: version, + agent_profile: { name: 'Test validator', tools: ['validate_input', 'comply_test_controller'] }, + // Deliberately stale failure labels: the adapter must grade step evidence. + overall_status: 'failing', + tracks: [{ + track: 'creative', + label: 'Creative', + status: 'fail', + duration_ms: 1, + skipped_scenarios: [], + observations: [], + scenarios: [{ + agent_url: 'https://agent.example/mcp', + // The runner emits storyboard/phase IDs beyond the legacy scenario enum. + scenario: `${storyboard.id}/${storyboard.phases[0].id}` as TestScenario, + overall_passed: false, + steps, + summary: 'Failed', + total_duration_ms: 1, + tested_at: '2026-09-15T00:00:00.000Z', + }], + }], + tested_tracks: [], + skipped_tracks: [], + summary: { + tracks_passed: 0, tracks_failed: 1, tracks_skipped: 0, + tracks_partial: 0, tracks_silent: 0, headline: 'Failed', + }, + observations: [], + notices: [], + total_duration_ms: 1, + tested_at: '2026-09-15T00:00:00.000Z', + }; +} + +describe.each(['source', 'published bundle'])('3.2 validate_input gates: %s', surface => { + describe.each(targets)('%s', name => { + it('excludes controller-only agents and keeps the validator gate separate from controller setup', async () => { + const storyboard = loadStoryboard(name, surface); + expect(storyboard.introduced_in).toBe('3.2'); + expect(storyboard.required_tools).toEqual(['validate_input']); + expect(storyboard.requires).toEqual(['controller']); + expect(storyboard.prerequisites?.controller_seeding).toBe(true); + const validateInput = vi.fn(); + const complyTestController = vi.fn(); + const executeTask = vi.fn(); + const client = { resetContext() {}, validateInput, complyTestController, executeTask }; + + const excluded = await runStoryboard('https://agent.example/mcp', storyboard, + runOptions(['comply_test_controller'], client)); + expect(excluded.phases[0].steps[0].skip_reason).toBe('missing_tool'); + expect(excluded.failed_count).toBe(0); + + const needsFixtures = await runStoryboard('https://agent.example/mcp', storyboard, + runOptions(['validate_input'], client)); + expect(needsFixtures.phases[0].steps[0].skip_reason).toBe('missing_test_controller'); + expect(needsFixtures.failed_count).toBe(0); + expect(validateInput).not.toHaveBeenCalled(); + expect(complyTestController).not.toHaveBeenCalled(); + expect(executeTask).not.toHaveBeenCalled(); + }); + + it('executes the validator with setup available and preserves a genuine validation failure', async () => { + const storyboard = loadStoryboard(name, surface); + // Use the real positive vector and its validations. Fixtures are treated + // as pre-seeded so this test isolates validator evidence from transport. + const phase = storyboard.phases[0]; + const step = phase.steps[0]; + const focused = { ...storyboard, phases: [{ ...phase, steps: [step] }] }; + const validateInput = vi.fn(async (_task: string, request: { targets: unknown[]; context?: unknown }) => ({ + success: true, + data: { + adcp_version: '3.2', + status: 'completed', + context: request.context, + results: [{ target: request.targets[0], result_kind: 'validated_pass' }], + }, + })); + const options = { + ...runOptions(['validate_input', 'comply_test_controller'], { resetContext() {}, executeTask: validateInput }), + skip_controller_seeding: true, + }; + const passed = await runStoryboard('https://agent.example/mcp', focused, options); + expect(validateInput).toHaveBeenCalledTimes(1); + expect(validateInput.mock.calls[0][0]).toBe('validate_input'); + expect(passed.overall_passed).toBe(true); + expect(passed.failed_count).toBe(0); + expect(passed.passed_count).toBe(1); + + validateInput.mockImplementationOnce(async (_task, request) => ({ + success: true, + data: { + adcp_version: '3.2', + status: 'completed', + context: request.context, + results: [{ + target: request.targets[0], result_kind: 'validated_fail', + violations: [{ rule: 'manifest_structure', field: 'manifest.assets' }], + }], + }, + })); + const failed = await runStoryboard('https://agent.example/mcp', focused, options); + expect(validateInput).toHaveBeenCalledTimes(2); + expect(failed.overall_passed).toBe(false); + expect(failed.failed_count).toBe(1); + const failure = failed.phases[0].steps[0]; + expect(failure.validations).toEqual(expect.arrayContaining([ + expect.objectContaining({ check: 'response_schema', passed: true }), + expect.objectContaining({ check: 'field_value', passed: false }), + ])); + const result = complianceResult(storyboard, [{ + step: failure.title, step_id: failure.step_id, task: failure.task, + passed: failure.passed, error: failure.error, duration_ms: failure.duration_ms, + }]); + expect(deriveStoryboardStatuses(result)[0]).toMatchObject({ + status: 'failing', failure_count: 1, first_failed_step_task: 'validate_input', + }); + expect(complianceResultToDbInput(result, result.agent_url, 'production').tracks_json[0].status).toBe('fail'); + }); + + it('keeps fixture-unavailable coverage untested without masking independent validator failures', () => { + const storyboard = loadStoryboard(name, surface); + const [first, second] = storyboard.phases[0].steps; + // SDK projected preflight gap plus its dependent skip; no seller verdict. + const gapSteps: ProjectedStep[] = [{ + step: first.title, step_id: first.id, task: first.task, + passed: false, skipped: true, skip_reason: 'fixture_unavailable', duration_ms: 0, + }, { + step: second.title, step_id: second.id, task: second.task, + passed: false, skipped: true, skip_reason: 'prerequisite_failed', duration_ms: 0, + }]; + const result = complianceResult(storyboard, gapSteps); + expect(deriveStoryboardStatuses(result)).toEqual([{ + storyboard_id: storyboard.id, status: 'untested', steps_passed: 0, steps_total: 0, + }]); + expect(complianceResultToDbInput(result, result.agent_url, 'production').tracks_json[0]).toMatchObject({ + status: 'skip', has_coverage_gap_skip: true, + }); + + const mixed = complianceResult(storyboard, [{ + step: first.title, step_id: first.id, task: 'validate_input', + passed: false, error: 'Expected validated_pass, received validated_fail', duration_ms: 1, + }, ...gapSteps]); + expect(deriveStoryboardStatuses(mixed)[0]).toMatchObject({ status: 'failing', failure_count: 1 }); + expect(complianceResultToDbInput(mixed, mixed.agent_url, 'production').tracks_json[0].status).toBe('fail'); + }); + }); +}); diff --git a/tests/run-storyboards-sharding.test.cjs b/tests/run-storyboards-sharding.test.cjs index 033ffbc6d98..ab7d95610b2 100644 --- a/tests/run-storyboards-sharding.test.cjs +++ b/tests/run-storyboards-sharding.test.cjs @@ -300,19 +300,18 @@ test('creative-builder uses bounded isolated children in local and CI matrices', test('current training-agent floors are ratcheted and mirrored by local and CI runners', () => { const workflow = fs.readFileSync(STORYBOARD_WORKFLOW, 'utf8'); const matrixRunner = fs.readFileSync(MATRIX_RUNNER, 'utf8'); - // governance and brand dropped one passing step each when - // canonical_format_validate_input stopped listing comply_test_controller in - // required_tools: both tenants expose the controller but not validate_input, - // so the storyboard is no longer selected for them and the single step it - // contributed is gone. Deliberate de-ratchet, not a regression — the - // clean-storyboard floors are untouched. + // The CTV and premium-display validate_input gates previously admitted + // controller-only /brand and /governance: 14 + 18 product seeds passed while + // every validator check skipped. The live tool-catalog drift test replays + // both gates and proves that exact loss. Subtract only those 32 setup passes; + // pin all clean floors, other tenants, and the frozen 3.0 floors below. const baselines = [ ['signals', 45, 80], ['sales', 133, 632], - ['governance', 47, 160], + ['governance', 47, 160 - (14 + 18)], ['creative', 49, 209], ['creative-builder', 50, 184], - ['brand', 45, 115], + ['brand', 45, 115 - (14 + 18)], ['si', 42, 50], ];