Просмотр исходного кода

fix(evals): repair six eval-framework behavioral regressions

Restore the deterministic eval baseline by fixing false-negatives and
identity/discovery bugs across the framework:

- error-handling: correct severity-boundary fixtures so mixed-severity
  cases actually cross the >1000 warning threshold (deterministic
  error/warning/info), plus explicit 1000-vs-1001 boundary tests.
- context-loading-evaluator: emit `no-context-loaded` and
  `context-loaded-after-execution` as errors so missing/late context
  fails (was a passing warning); update focused test accordingly.
- tool-usage-evaluator: enforce dedicated tools — bash `cat`/`ls`/`ls -la`
  now fail as `bash-antipattern-*` errors (ls de-allowlisted); npm/git/
  piped commands still pass.
- test-case-loader: canonicalize legacy agent ids via normalizeAgentId
  (`openagent` -> `core/openagent`), preserving canonical/unknown ids.
- suite-validator: accept category-based agent ids, resolve test paths
  under the canonical dir, add requested-vs-declared conflict detection;
  new suite-validator.test.ts.
- validate-suites-cli: recursive suite discovery (nested <cat>/<agent>/
  config), deterministic order, fail-closed on zero suites; new
  validate-suites-cli.test.ts.

Deterministic Vitest allowlist green; validate:suites:all non-vacuous
and exits 0. No model/network/eval:sdk execution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
darrenhinde 2 недель назад
Родитель
Сommit
9a6909ec3d

+ 28 - 6
evals/framework/src/__tests__/error-handling.test.ts

@@ -419,6 +419,30 @@ describe('Severity Levels', () => {
       expect(warningViolation!.type).toBe('code-length');
     });
 
+    it('does not report a warning when code length is exactly 1000', () => {
+      // Arrange
+      const code = 'x'.repeat(1000);
+
+      // Act
+      const result = validateCode(code);
+
+      // Assert
+      expect(code).toHaveLength(1000);
+      expect(result.violations.some(v => v.severity === 'warning')).toBe(false);
+    });
+
+    it('reports a warning when code length is exactly 1001', () => {
+      // Arrange
+      const code = 'x'.repeat(1001);
+
+      // Act
+      const result = validateCode(code);
+
+      // Assert
+      expect(code).toHaveLength(1001);
+      expect(result.violations.some(v => v.severity === 'warning')).toBe(true);
+    });
+
     it('passes with info-level violations', () => {
       // Arrange
       const code = 'const x = 10;'; // No strict mode
@@ -435,23 +459,21 @@ describe('Severity Levels', () => {
 
     it('reports multiple violations with different severities', () => {
       // Arrange
-      const code = 'eval("bad");'.repeat(100); // error + warning + info
+      const code = 'eval("bad");' + 'x'.repeat(989);
 
       // Act
       const result = validateCode(code);
 
       // Assert
+      expect(code).toHaveLength(1001);
       expect(result.passed).toBe(false); // Error causes failure
-      expect(result.violations.length).toBeGreaterThanOrEqual(2);
-      
       const severities = result.violations.map(v => v.severity);
-      expect(severities).toContain('error');
-      expect(severities).toContain('warning');
+      expect(severities).toEqual(['error', 'warning', 'info']);
     });
 
     it('distinguishes between severity levels correctly', () => {
       // Arrange
-      const code = 'eval("x");'.repeat(100);
+      const code = 'eval("x");'.repeat(101); // 1010 chars: crosses the >1000 warning boundary
 
       // Act
       const result = validateCode(code);

+ 3 - 3
evals/framework/src/evaluators/__tests__/context-loading-evaluator.test.ts

@@ -98,11 +98,11 @@ describe('ContextLoadingEvaluator', () => {
 
       const result = await evaluator.evaluate(timeline, mockSessionInfo);
 
-      // Context loading violation is a warning, not error, so passed is still true
-      // But contextLoadedBeforeExecution should be false
+      // No context loaded before execution is an error, so passed is false
+      // And contextLoadedBeforeExecution should be false
       expect(result.metadata?.contextLoadedBeforeExecution).toBe(false);
       expect(result.violations.length).toBeGreaterThan(0);
-      expect(result.violations[0].severity).toBe('warning');
+      expect(result.violations[0].severity).toBe('error');
     });
   });
 

+ 16 - 10
evals/framework/src/evaluators/__tests__/tool-usage-enhanced.test.ts

@@ -41,7 +41,7 @@ describe('ToolUsageEvaluator - Enhanced Features', () => {
       const result = await evaluator.evaluate(timeline, mockSessionInfo);
       
       expect(result.violations.length).toBeGreaterThan(0);
-      expect(result.violations[0].type).toBe('suboptimal-tool-usage');
+      expect(result.violations[0].type).toBe('bash-antipattern-edit');
       expect(result.violations[0].evidence.suggestedTool).toBe('edit');
     });
 
@@ -179,7 +179,7 @@ describe('ToolUsageEvaluator - Enhanced Features', () => {
   });
 
   describe('Severity Levels', () => {
-    it('should use warning severity for cat usage', async () => {
+    it('should use error severity for cat usage', async () => {
       const timeline: TimelineEvent[] = [
         {
           timestamp: 1000,
@@ -192,11 +192,13 @@ describe('ToolUsageEvaluator - Enhanced Features', () => {
       ];
 
       const result = await evaluator.evaluate(timeline, mockSessionInfo);
-      
-      expect(result.violations[0].severity).toBe('warning');
+
+      expect(result.violations[0].severity).toBe('error');
+      expect(result.violations[0].type).toBe('bash-antipattern-read');
+      expect(result.passed).toBe(false);
     });
 
-    it('should use info severity for ls usage', async () => {
+    it('should use error severity for ls usage', async () => {
       const timeline: TimelineEvent[] = [
         {
           timestamp: 1000,
@@ -209,8 +211,10 @@ describe('ToolUsageEvaluator - Enhanced Features', () => {
       ];
 
       const result = await evaluator.evaluate(timeline, mockSessionInfo);
-      
-      expect(result.violations[0].severity).toBe('info');
+
+      expect(result.violations[0].severity).toBe('error');
+      expect(result.violations[0].type).toBe('bash-antipattern-list');
+      expect(result.passed).toBe(false);
     });
   });
 
@@ -267,7 +271,7 @@ describe('ToolUsageEvaluator - Enhanced Features', () => {
       expect(result.passed).toBe(true);
     });
 
-    it('should allow ls -la for detailed info', async () => {
+    it('should flag ls -la as a dedicated-listing-tool violation', async () => {
       const timeline: TimelineEvent[] = [
         {
           timestamp: 1000,
@@ -280,8 +284,10 @@ describe('ToolUsageEvaluator - Enhanced Features', () => {
       ];
 
       const result = await evaluator.evaluate(timeline, mockSessionInfo);
-      
-      expect(result.passed).toBe(true);
+
+      expect(result.passed).toBe(false);
+      expect(result.violations.length).toBeGreaterThan(0);
+      expect(result.violations[0].type).toBe('bash-antipattern-list');
     });
 
     it('should allow echo to stdout (no redirection)', async () => {

+ 2 - 2
evals/framework/src/evaluators/context-loading-evaluator.ts

@@ -369,7 +369,7 @@ export class ContextLoadingEvaluator extends BaseEvaluator {
       violations.push(
         this.createViolation(
           'no-context-loaded',
-          'warning',
+          'error',
           'Task execution started without loading any context files',
           firstExecution.timestamp,
           {
@@ -402,7 +402,7 @@ export class ContextLoadingEvaluator extends BaseEvaluator {
       violations.push(
         this.createViolation(
           'context-loaded-after-execution',
-          'warning',
+          'error',
           'Some executions happened before context was loaded',
           firstExecution.timestamp,
           {

+ 10 - 11
evals/framework/src/evaluators/tool-usage-evaluator.ts

@@ -33,11 +33,11 @@ export class ToolUsageEvaluator extends BaseEvaluator {
   // Patterns for detecting suboptimal bash usage
   // NOTE: grep, rg, npm, git, and other valid bash commands are ALLOWED
   private bashAntiPatterns = [
-    { 
-      pattern: /\bcat\s+([^\s|>]+)(?!\s*[|>])/i, 
-      tool: 'read', 
+    {
+      pattern: /\bcat\s+([^\s|>]+)(?!\s*[|>])/i,
+      tool: 'read',
       message: 'Use Read tool instead of cat for reading files',
-      severity: 'warning' as const,
+      severity: 'error' as const,
       example: 'read(filePath: "path/to/file")'
     },
     { 
@@ -54,11 +54,11 @@ export class ToolUsageEvaluator extends BaseEvaluator {
       severity: 'warning' as const,
       example: 'read(filePath: "file", offset: -10)'
     },
-    { 
-      pattern: /\bls\s+(?!-[al]+\s)([^\s]*)/i, 
-      tool: 'list', 
-      message: 'Use List tool instead of ls (unless ls -la for detailed info)',
-      severity: 'info' as const,
+    {
+      pattern: /\bls(\s+|$)/i,
+      tool: 'list',
+      message: 'Use List tool (or Glob for patterns) instead of ls',
+      severity: 'error' as const,
       example: 'list(path: "directory")'
     },
     { 
@@ -116,7 +116,6 @@ export class ToolUsageEvaluator extends BaseEvaluator {
     /^\s*mv\s+/i,            // moving files
     /^\s*cp\s+/i,            // copying files
     /^\s*chmod\s+/i,         // permissions
-    /^\s*ls\s+-[la]+/i,      // ls -la for detailed directory info
     /^\s*cd\s+/i,            // navigation
     /^\s*pwd\s*/i,           // current directory
     /^\s*which\s+/i,         // command location
@@ -195,7 +194,7 @@ export class ToolUsageEvaluator extends BaseEvaluator {
         // Add violation with appropriate severity
         violations.push(
           this.createViolation(
-            'suboptimal-tool-usage',
+            `bash-antipattern-${antiPattern.tool}`,
             antiPattern.severity,
             antiPattern.message,
             bashCall.timestamp,

+ 217 - 0
evals/framework/src/sdk/__tests__/suite-validator.test.ts

@@ -0,0 +1,217 @@
+/**
+ * Tests for SuiteValidator category-based identity and path resolution.
+ *
+ * Uses the REAL evals/agents directory as agentsDir so that the canonical
+ * `core/openagent` core suite and its nested test paths are exercised
+ * end-to-end. Conflict / missing-test / duplicate-id cases use small temp
+ * fixture files whose paths are passed directly to the validator.
+ *
+ * NOTE: Validation is filesystem-only. No network, model, or agent execution.
+ */
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { SuiteValidator } from '../suite-validator.js';
+import { join, dirname } from 'path';
+import { fileURLToPath } from 'url';
+import { mkdtempSync, writeFileSync, rmSync } from 'fs';
+import { tmpdir } from 'os';
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = dirname(__filename);
+
+// Real agents directory (matches the layout used by the validate:suites CLI)
+const agentsDir = join(__dirname, '../../../../agents');
+const coreSuitePath = join(
+  agentsDir,
+  'core/openagent/config/core-suite.json'
+);
+
+/**
+ * Minimal but schema-valid suite object used for temp fixtures.
+ */
+function makeSuite(overrides: Record<string, unknown> = {}): Record<string, unknown> {
+  return {
+    name: 'Fixture Suite',
+    description: 'Temporary fixture suite for validator tests',
+    version: '1.0.0',
+    agent: 'core/openagent',
+    totalTests: 1,
+    estimatedRuntime: '1-2 minutes',
+    tests: [
+      {
+        id: 1,
+        name: 'Approval Gate',
+        path: '01-critical-rules/approval-gate/05-approval-before-execution-positive.yaml',
+        category: 'critical-rules',
+        priority: 'critical'
+      }
+    ],
+    ...overrides
+  };
+}
+
+describe('SuiteValidator - category-based identities', () => {
+  let tmpDir: string;
+
+  beforeAll(() => {
+    tmpDir = mkdtempSync(join(tmpdir(), 'suite-validator-'));
+  });
+
+  afterAll(() => {
+    rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  function writeFixture(name: string, data: unknown): string {
+    const filePath = join(tmpDir, name);
+    writeFileSync(filePath, JSON.stringify(data, null, 2), 'utf8');
+    return filePath;
+  }
+
+  describe('canonical id resolution (criterion 1)', () => {
+    it('accepts canonical core/openagent suite and resolves its test paths', () => {
+      // Arrange
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act
+      const result = validator.validateSuiteFile('core/openagent', coreSuitePath);
+
+      // Assert
+      expect(result.valid).toBe(true);
+      expect(result.errors).toHaveLength(0);
+      expect(result.suite?.agent).toBe('core/openagent');
+      // Required tests were located under evals/agents/core/openagent/tests
+      expect(result.missingTests).toHaveLength(0);
+      expect(result.suite?.tests.length).toBeGreaterThan(0);
+    });
+  });
+
+  describe('legacy id resolution (criterion 2)', () => {
+    it('accepts legacy "openagent" request and resolves to the canonical dir', () => {
+      // Arrange
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act: request with the legacy flat id against the canonical suite file
+      const result = validator.validateSuiteFile('openagent', coreSuitePath);
+
+      // Assert: legacy and canonical forms of the SAME agent must match
+      expect(result.valid).toBe(true);
+      expect(result.errors).toHaveLength(0);
+      // Paths still resolved under core/openagent/tests (nothing missing)
+      expect(result.missingTests).toHaveLength(0);
+    });
+  });
+
+  describe('conflict detection (criterion 3)', () => {
+    it('fails with an "agent" field error when requested agent differs from declared', () => {
+      // Arrange: suite declares a DIFFERENT agent than the one requested
+      const fixturePath = writeFixture(
+        'conflict.json',
+        makeSuite({ agent: 'core/opencoder' })
+      );
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act
+      const result = validator.validateSuiteFile('core/openagent', fixturePath);
+
+      // Assert
+      expect(result.valid).toBe(false);
+      const agentError = result.errors.find(e => e.field === 'agent');
+      expect(agentError).toBeDefined();
+      expect(agentError?.message).toContain('does not match requested agent');
+    });
+
+    it('does NOT flag a conflict for legacy vs canonical forms of the same agent', () => {
+      // Arrange: suite declares canonical, request uses legacy flat id
+      const fixturePath = writeFixture(
+        'no-conflict.json',
+        makeSuite({ agent: 'core/openagent' })
+      );
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act
+      const result = validator.validateSuiteFile('openagent', fixturePath);
+
+      // Assert: no agent-field conflict error
+      const agentError = result.errors.find(e => e.field === 'agent');
+      expect(agentError).toBeUndefined();
+      expect(result.valid).toBe(true);
+    });
+  });
+
+  describe('required test presence and unique ids (criterion 4)', () => {
+    it('fails when a required test file is missing', () => {
+      // Arrange: required test with a path that does not exist
+      const fixturePath = writeFixture(
+        'missing-test.json',
+        makeSuite({
+          tests: [
+            {
+              id: 1,
+              name: 'Nonexistent Test',
+              path: 'does-not-exist/nope.yaml',
+              category: 'critical-rules',
+              priority: 'critical',
+              required: true
+            }
+          ]
+        })
+      );
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act
+      const result = validator.validateSuiteFile('core/openagent', fixturePath);
+
+      // Assert
+      expect(result.valid).toBe(false);
+      expect(result.missingTests).toContain('does-not-exist/nope.yaml');
+      expect(
+        result.errors.some(e => e.message.includes('Required test file not found'))
+      ).toBe(true);
+    });
+
+    it('fails when duplicate test IDs are present', () => {
+      // Arrange: two tests sharing id 1 (both point at a real file so the
+      // failure is attributable to the duplicate id, not a missing path)
+      const realPath =
+        '01-critical-rules/approval-gate/05-approval-before-execution-positive.yaml';
+      const fixturePath = writeFixture(
+        'dup-ids.json',
+        makeSuite({
+          totalTests: 2,
+          tests: [
+            { id: 1, name: 'A', path: realPath, category: 'critical-rules', priority: 'critical' },
+            { id: 1, name: 'B', path: realPath, category: 'critical-rules', priority: 'high' }
+          ]
+        })
+      );
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act
+      const result = validator.validateSuiteFile('core/openagent', fixturePath);
+
+      // Assert
+      expect(result.valid).toBe(false);
+      expect(
+        result.errors.some(e => e.message.includes('Duplicate test IDs'))
+      ).toBe(true);
+    });
+  });
+
+  describe('schema acceptance matrix', () => {
+    it('accepts all supported core agent id forms and rejects unknown flat ids', () => {
+      // Arrange
+      const validator = new SuiteValidator(agentsDir);
+
+      // Act + Assert: supported forms validate at the data/schema level
+      for (const agent of ['openagent', 'opencoder', 'core/openagent', 'core/opencoder']) {
+        const res = validator.validateSuiteData(makeSuite({ agent }));
+        expect(res.valid, `expected ${agent} to be schema-valid`).toBe(true);
+      }
+
+      // Unknown flat id (no slash, not normalizable) is rejected by the schema
+      const bad = validator.validateSuiteData(makeSuite({ agent: 'totally-unknown-agent' }));
+      expect(bad.valid).toBe(false);
+      expect(bad.errors.some(e => e.field === 'agent')).toBe(true);
+    });
+  });
+});

+ 124 - 0
evals/framework/src/sdk/__tests__/validate-suites-cli.test.ts

@@ -0,0 +1,124 @@
+/**
+ * Tests for the validate-suites CLI helpers.
+ *
+ * Exercises the pure, exported discovery and exit-code functions using temp
+ * directory fixtures (mkdtempSync). No CLI/process.exit is triggered because
+ * the module uses an ESM entrypoint guard, so importing it is side-effect free.
+ *
+ * NOTE: Filesystem-only. No network, model, paid API, or agent execution.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { mkdtempSync, mkdirSync, rmSync } from 'fs';
+import { join, dirname } from 'path';
+import { fileURLToPath } from 'url';
+import { tmpdir } from 'os';
+
+import {
+  discoverAgents,
+  computeExitCode
+} from '../validate-suites-cli.js';
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = dirname(__filename);
+
+// Real agents directory (matches the layout used by the validate:suites CLI).
+const realAgentsDir = join(__dirname, '../../../../agents');
+
+describe('validate-suites-cli - discoverAgents', () => {
+  let tmpRoot: string;
+
+  beforeEach(() => {
+    tmpRoot = mkdtempSync(join(tmpdir(), 'discover-agents-'));
+  });
+
+  afterEach(() => {
+    rmSync(tmpRoot, { recursive: true, force: true });
+  });
+
+  it('recursively finds category-based agent config dirs and ignores non-agent siblings', () => {
+    // Arrange: nested <category>/<agent>/config layout.
+    mkdirSync(join(tmpRoot, 'core', 'openagent', 'config'), { recursive: true });
+    mkdirSync(join(tmpRoot, 'core', 'opencoder', 'config'), { recursive: true });
+    mkdirSync(join(tmpRoot, 'development', 'frontend-specialist', 'config'), { recursive: true });
+    // Sibling directory WITHOUT a config subdir -> must be ignored.
+    mkdirSync(join(tmpRoot, 'content', 'copywriter', 'docs'), { recursive: true });
+    // Noise directories that must be skipped.
+    mkdirSync(join(tmpRoot, '.hidden', 'config'), { recursive: true });
+    mkdirSync(join(tmpRoot, 'node_modules', 'pkg', 'config'), { recursive: true });
+
+    // Act
+    const agents = discoverAgents(tmpRoot);
+
+    // Assert: category-based ids, sorted, deterministic; no non-agent dirs.
+    expect(agents).toEqual([
+      'core/opencoder',
+      'core/openagent',
+      'development/frontend-specialist'
+    ].sort());
+    expect(agents).not.toContain('content/copywriter');
+    expect(agents).not.toContain('.hidden');
+    expect(agents.some(a => a.includes('node_modules'))).toBe(false);
+  });
+
+  it('returns a sorted, deterministic order', () => {
+    // Arrange
+    mkdirSync(join(tmpRoot, 'zeta', 'zzz', 'config'), { recursive: true });
+    mkdirSync(join(tmpRoot, 'alpha', 'aaa', 'config'), { recursive: true });
+    mkdirSync(join(tmpRoot, 'core', 'mmm', 'config'), { recursive: true });
+
+    // Act
+    const agents = discoverAgents(tmpRoot);
+
+    // Assert
+    expect(agents).toEqual([...agents].sort());
+    expect(agents).toEqual(['alpha/aaa', 'core/mmm', 'zeta/zzz']);
+  });
+
+  it('returns an empty array for an empty tree', () => {
+    // Arrange: tmpRoot exists but has no agent config dirs.
+    // Act
+    const agents = discoverAgents(tmpRoot);
+
+    // Assert
+    expect(agents).toEqual([]);
+  });
+
+  it('returns an empty array for a nonexistent directory', () => {
+    // Act
+    const agents = discoverAgents(join(tmpRoot, 'does-not-exist'));
+
+    // Assert
+    expect(agents).toEqual([]);
+  });
+
+  it('discovers core/openagent in the real agents directory', () => {
+    // Act
+    const agents = discoverAgents(realAgentsDir);
+
+    // Assert
+    expect(agents).toContain('core/openagent');
+  });
+});
+
+describe('validate-suites-cli - computeExitCode (fail-closed)', () => {
+  it('returns nonzero when zero suites were discovered', () => {
+    // Assert
+    expect(computeExitCode({ totalSuites: 0, invalidSuites: 0 })).not.toBe(0);
+  });
+
+  it('returns nonzero when any suite is invalid', () => {
+    // Assert
+    expect(computeExitCode({ totalSuites: 5, invalidSuites: 1 })).not.toBe(0);
+  });
+
+  it('returns zero only when total > 0 and every suite is valid', () => {
+    // Assert
+    expect(computeExitCode({ totalSuites: 3, invalidSuites: 0 })).toBe(0);
+  });
+
+  it('treats zero suites as a failure even with no invalid suites', () => {
+    // Assert: fail-closed — empty discovery must not be reported as success.
+    expect(computeExitCode({ totalSuites: 0, invalidSuites: 0 })).toBe(1);
+  });
+});

+ 40 - 9
evals/framework/src/sdk/suite-validator.ts

@@ -7,6 +7,7 @@
 import { z } from 'zod';
 import { readFileSync, existsSync } from 'fs';
 import { join } from 'path';
+import { normalizeAgentId } from '../config.js';
 
 /**
  * Zod schema for test definition
@@ -39,7 +40,14 @@ const TestSuiteSchema = z.object({
   name: z.string().min(1),
   description: z.string().min(1),
   version: z.string().regex(/^\d+\.\d+\.\d+$/, 'Version must be semver (e.g., 1.0.0)'),
-  agent: z.enum(['openagent', 'opencoder']),
+  // Accept both legacy flat ids (e.g. "openagent") and canonical category-based
+  // ids (e.g. "core/openagent"). A value is valid when it is already
+  // category-based (contains "/") or maps to a known canonical id via
+  // normalizeAgentId. Arbitrary/unknown flat ids are rejected.
+  agent: z.string().min(1).refine(
+    (val) => val.includes('/') || normalizeAgentId(val) !== val,
+    { message: 'Agent must be a known agent id (e.g., "openagent") or a category-based path (e.g., "core/openagent")' }
+  ),
   totalTests: z.number().int().positive(),
   estimatedRuntime: z.string().regex(/^\d+-\d+ (minutes|seconds|hours)$/),
   coverage: z.record(z.boolean()).optional(),
@@ -144,9 +152,21 @@ export class SuiteValidator {
     
     const suite = parseResult.data;
     result.suite = suite;
-    
-    // Validate test paths exist
-    const testsDir = join(this.agentsDir, agent, 'tests');
+
+    // Conflict detection: the requested agent must resolve to the same
+    // canonical id as the agent declared in the suite. Legacy vs canonical
+    // forms of the SAME agent (e.g. "openagent" vs "core/openagent") match.
+    if (suite.agent && normalizeAgentId(agent) !== normalizeAgentId(suite.agent)) {
+      result.valid = false;
+      result.errors.push({
+        field: 'agent',
+        message: `Suite agent "${suite.agent}" (resolves to "${normalizeAgentId(suite.agent)}") does not match requested agent "${agent}" (resolves to "${normalizeAgentId(agent)}")`
+      });
+      return result;
+    }
+
+    // Validate test paths exist (resolve legacy ids to canonical dirs)
+    const testsDir = join(this.agentsDir, normalizeAgentId(agent), 'tests');
     
     if (!existsSync(testsDir)) {
       result.valid = false;
@@ -216,8 +236,19 @@ export class SuiteValidator {
       missingTests: [],
       suite
     };
-    
-    const testsDir = join(this.agentsDir, agent, 'tests');
+
+    // Conflict detection: requested agent must resolve to the same canonical
+    // id as the declared suite agent (legacy vs canonical forms still match).
+    if (suite.agent && normalizeAgentId(agent) !== normalizeAgentId(suite.agent)) {
+      result.valid = false;
+      result.errors.push({
+        field: 'agent',
+        message: `Suite agent "${suite.agent}" (resolves to "${normalizeAgentId(suite.agent)}") does not match requested agent "${agent}" (resolves to "${normalizeAgentId(agent)}")`
+      });
+      return result;
+    }
+
+    const testsDir = join(this.agentsDir, normalizeAgentId(agent), 'tests');
     
     if (!existsSync(testsDir)) {
       result.valid = false;
@@ -317,9 +348,9 @@ export class SuiteValidator {
     const content = readFileSync(suitePath, 'utf8');
     const suite = JSON.parse(content) as TestSuite;
     
-    // Ensure agent field is set
+    // Ensure agent field is set (canonicalize the requested id)
     if (!suite.agent) {
-      suite.agent = agent as 'openagent' | 'opencoder';
+      suite.agent = normalizeAgentId(agent);
     }
     
     return suite;
@@ -329,7 +360,7 @@ export class SuiteValidator {
    * Get absolute paths for all tests in a suite
    */
   getTestPaths(agent: string, suite: TestSuite): string[] {
-    const testsDir = join(this.agentsDir, agent, 'tests');
+    const testsDir = join(this.agentsDir, normalizeAgentId(agent), 'tests');
     const paths: string[] = [];
     
     for (const test of suite.tests) {

+ 8 - 1
evals/framework/src/sdk/test-case-loader.ts

@@ -1,6 +1,7 @@
 import { readFile } from 'fs/promises';
 import { parse as parseYaml } from 'yaml';
 import { TestCaseSchema, TestSuiteSchema, type TestCase, type TestSuite } from './test-case-schema.js';
+import { normalizeAgentId } from '../config.js';
 
 /**
  * Load a single test case from a YAML file
@@ -11,7 +12,13 @@ export async function loadTestCase(filePath: string): Promise<TestCase> {
   
   try {
     const testCase = TestCaseSchema.parse(data);
-    
+
+    // Canonicalize legacy agent identifiers to category-based IDs (e.g. "openagent" → "core/openagent").
+    // Unknown identifiers are preserved unchanged by normalizeAgentId.
+    if (testCase.agent) {
+      testCase.agent = normalizeAgentId(testCase.agent);
+    }
+
     // Warn about deprecated schema
     if (testCase.expected && !testCase.behavior && !testCase.expectedViolations) {
       console.warn(`⚠️  Test ${testCase.id} uses deprecated "expected" schema.`);

+ 141 - 57
evals/framework/src/sdk/validate-suites-cli.ts

@@ -1,15 +1,15 @@
 #!/usr/bin/env node
 /**
  * CLI tool to validate test suite JSON files
- * 
+ *
  * Usage:
  *   npm run validate:suites
- *   npm run validate:suites -- openagent
+ *   npm run validate:suites -- core/openagent
  *   npm run validate:suites -- --all
  */
 
 import { SuiteValidator } from './suite-validator.js';
-import { readFileSync, existsSync, readdirSync } from 'fs';
+import { existsSync, readdirSync } from 'fs';
 import { join, dirname } from 'path';
 import { fileURLToPath } from 'url';
 
@@ -25,7 +25,7 @@ const colors = {
   blue: '\x1b[34m'
 };
 
-interface ValidationStats {
+export interface ValidationStats {
   totalSuites: number;
   validSuites: number;
   invalidSuites: number;
@@ -33,18 +33,110 @@ interface ValidationStats {
   totalWarnings: number;
 }
 
+/**
+ * Recursively discover agent ids under a nested agents directory.
+ *
+ * An "agent" is any directory that CONTAINS a `config` subdirectory. Agents
+ * live at a category-based path such as `core/openagent`, so the returned ids
+ * are the relative path from `agentsDir` to each such directory. Discovery:
+ *   - recurses through category directories (which have no direct `config`),
+ *   - stops descending once a `config` subdir is found (agents don't nest),
+ *   - skips hidden dirs and `node_modules`,
+ *   - returns a SORTED array for deterministic ordering.
+ */
+export function discoverAgents(agentsDir: string): string[] {
+  const results: string[] = [];
+
+  function walk(dir: string, relPrefix: string): void {
+    let entries;
+    try {
+      entries = readdirSync(dir, { withFileTypes: true });
+    } catch {
+      return;
+    }
+
+    for (const entry of entries) {
+      if (!entry.isDirectory()) continue;
+
+      const name = entry.name;
+      if (name.startsWith('.') || name === 'node_modules') continue;
+
+      const rel = relPrefix ? `${relPrefix}/${name}` : name;
+      const full = join(dir, name);
+
+      if (existsSync(join(full, 'config'))) {
+        // This directory is an agent; record it and do not descend further.
+        results.push(rel);
+      } else {
+        walk(full, rel);
+      }
+    }
+  }
+
+  walk(agentsDir, '');
+  return results.sort();
+}
+
+/**
+ * Collect suite JSON files for a single agent.
+ *
+ * Looks in `<config>/suites/*.json` (new location) and `<config>/*.json`
+ * (legacy location, excluding the suite schema). Returns absolute paths.
+ */
+export function collectSuiteFiles(agentsDir: string, agentId: string): string[] {
+  const agentConfigDir = join(agentsDir, agentId, 'config');
+  const suiteFiles: string[] = [];
+
+  if (!existsSync(agentConfigDir)) {
+    return suiteFiles;
+  }
+
+  // New location: suites directory
+  const suitesDir = join(agentConfigDir, 'suites');
+  if (existsSync(suitesDir)) {
+    readdirSync(suitesDir)
+      .filter(f => f.endsWith('.json'))
+      .forEach(f => suiteFiles.push(join(suitesDir, f)));
+  }
+
+  // Legacy location: JSON files directly in config directory
+  readdirSync(agentConfigDir)
+    .filter(f => f.endsWith('.json') && f !== 'suite-schema.json')
+    .forEach(f => {
+      const filePath = join(agentConfigDir, f);
+      if (!suiteFiles.includes(filePath)) {
+        suiteFiles.push(filePath);
+      }
+    });
+
+  return suiteFiles;
+}
+
+/**
+ * Fail-closed exit-code rule.
+ *
+ * Returns nonzero when NO suites were discovered (`totalSuites === 0`) OR when
+ * any discovered suite is invalid (`invalidSuites > 0`). Returns 0 only when at
+ * least one suite was discovered and every one of them validated.
+ */
+export function computeExitCode(stats: Pick<ValidationStats, 'totalSuites' | 'invalidSuites'>): number {
+  if (stats.totalSuites === 0) return 1;
+  if (stats.invalidSuites > 0) return 1;
+  return 0;
+}
+
 function validateSuite(agent: string, suitePath: string, agentsDir: string): boolean {
   const suiteName = suitePath.split('/').pop()?.replace('.json', '') || 'unknown';
-  
+
   console.log(`${colors.blue}Validating:${colors.reset} ${agent}/${suiteName}`);
-  
+
   const validator = new SuiteValidator(agentsDir);
   const result = validator.validateSuiteFile(agent, suitePath);
-  
+
   if (result.valid) {
     const testCount = result.suite?.tests.length || 0;
     console.log(`  ${colors.green}✅ Valid${colors.reset} (${testCount} tests)`);
-    
+
     if (result.warnings.length > 0) {
       result.warnings.forEach(warn => {
         console.log(`  ${colors.yellow}⚠️  ${warn}${colors.reset}`);
@@ -52,14 +144,14 @@ function validateSuite(agent: string, suitePath: string, agentsDir: string): boo
     }
   } else {
     console.log(`  ${colors.red}❌ Invalid${colors.reset} (${result.errors.length} errors, ${result.warnings.length} warnings)`);
-    
+
     result.errors.forEach(err => {
       console.log(`     ${colors.red}Error:${colors.reset} ${err.field}: ${err.message}`);
       if (err.value) {
         console.log(`       Value: ${err.value}`);
       }
     });
-    
+
     if (result.missingTests.length > 0) {
       console.log(`  ${colors.red}Missing test files (${result.missingTests.length}):${colors.reset}`);
       result.missingTests.forEach(path => {
@@ -67,9 +159,9 @@ function validateSuite(agent: string, suitePath: string, agentsDir: string): boo
       });
     }
   }
-  
+
   console.log();
-  
+
   return result.valid;
 }
 
@@ -77,12 +169,12 @@ function main() {
   const args = process.argv.slice(2);
   const validateAll = args.includes('--all');
   const agent = validateAll ? null : (args[0] || 'openagent');
-  
+
   console.log(`${colors.blue}🔍 Validating Test Suites${colors.reset}\n`);
-  
+
   const projectRoot = join(__dirname, '../../../..');
   const agentsDir = join(projectRoot, 'evals', 'agents');
-  
+
   const stats: ValidationStats = {
     totalSuites: 0,
     validSuites: 0,
@@ -90,54 +182,35 @@ function main() {
     totalErrors: 0,
     totalWarnings: 0
   };
-  
-  const agentsToValidate = validateAll 
-    ? readdirSync(agentsDir).filter(f => {
-        const agentPath = join(agentsDir, f);
-        return existsSync(join(agentPath, 'config'));
-      })
+
+  const agentsToValidate = validateAll
+    ? discoverAgents(agentsDir)
     : [agent!];
-  
+
+  if (validateAll && agentsToValidate.length === 0) {
+    console.log(`${colors.yellow}⚠️  No agents with a config directory found under: ${agentsDir}${colors.reset}\n`);
+  }
+
   for (const agentName of agentsToValidate) {
     const agentConfigDir = join(agentsDir, agentName, 'config');
-    
+
     if (!existsSync(agentConfigDir)) {
       console.log(`${colors.yellow}⚠️  No config directory for agent: ${agentName}${colors.reset}\n`);
       continue;
     }
-    
-    // Check for suites directory
-    const suitesDir = join(agentConfigDir, 'suites');
-    const suiteFiles: string[] = [];
-    
-    if (existsSync(suitesDir)) {
-      const files = readdirSync(suitesDir);
-      files.filter(f => f.endsWith('.json')).forEach(f => {
-        suiteFiles.push(join(suitesDir, f));
-      });
-    }
-    
-    // Check for suite files in config directory (legacy location)
-    const configFiles = readdirSync(agentConfigDir);
-    configFiles
-      .filter(f => f.endsWith('.json') && f !== 'suite-schema.json')
-      .forEach(f => {
-        const filePath = join(agentConfigDir, f);
-        if (!suiteFiles.includes(filePath)) {
-          suiteFiles.push(filePath);
-        }
-      });
-    
+
+    const suiteFiles = collectSuiteFiles(agentsDir, agentName);
+
     if (suiteFiles.length === 0) {
       console.log(`${colors.yellow}⚠️  No test suites found for agent: ${agentName}${colors.reset}\n`);
       continue;
     }
-    
+
     // Validate each suite
     for (const suiteFile of suiteFiles) {
       stats.totalSuites++;
       const isValid = validateSuite(agentName, suiteFile, agentsDir);
-      
+
       if (isValid) {
         stats.validSuites++;
       } else {
@@ -145,27 +218,38 @@ function main() {
       }
     }
   }
-  
+
   // Print summary
   console.log(`${colors.blue}${'='.repeat(55)}${colors.reset}`);
   console.log(`${colors.blue}Summary${colors.reset}`);
   console.log(`${colors.blue}${'='.repeat(55)}${colors.reset}`);
   console.log(`Total suites:    ${stats.totalSuites}`);
   console.log(`${colors.green}Valid suites:    ${stats.validSuites}${colors.reset}`);
-  
+
   if (stats.invalidSuites > 0) {
     console.log(`${colors.red}Invalid suites:  ${stats.invalidSuites}${colors.reset}`);
   }
-  
+
   console.log();
-  
-  if (stats.invalidSuites > 0) {
-    console.log(`${colors.red}❌ Validation failed${colors.reset}`);
-    process.exit(1);
+
+  const exitCode = computeExitCode(stats);
+
+  if (exitCode !== 0) {
+    if (stats.totalSuites === 0) {
+      console.log(`${colors.red}❌ Validation failed: no test suites were discovered${colors.reset}`);
+    } else {
+      console.log(`${colors.red}❌ Validation failed${colors.reset}`);
+    }
+    process.exit(exitCode);
   } else {
     console.log(`${colors.green}✅ All suites valid${colors.reset}`);
-    process.exit(0);
+    process.exit(exitCode);
   }
 }
 
-main();
+// ESM entrypoint guard: only run the CLI when this module is executed directly
+// (e.g. via `tsx src/sdk/validate-suites-cli.ts`). Importing it in tests must
+// NOT trigger main()/process.exit().
+if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) {
+  main();
+}