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], ];