From 175a0800d0df0dc258713dab04697bf2fa9a84a2 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Mon, 5 Oct 2026 01:43:33 -0400 Subject: [PATCH] fix(gateguard): enforce evaluation validity guards --- docker/gateguard-effectiveness/coverage.js | 22 +++++++- docker/gateguard-effectiveness/intent-eval.js | 21 ++++--- docker/gateguard-effectiveness/lib.js | 16 +++++- docker/gateguard-effectiveness/run-intent.js | 20 +++++-- docker/gateguard-effectiveness/session.js | 3 + tests/docker/gateguard-effectiveness.test.js | 5 +- tests/docker/gateguard-intent.test.js | 55 ++++++++++++++++++- 7 files changed, 123 insertions(+), 19 deletions(-) diff --git a/docker/gateguard-effectiveness/coverage.js b/docker/gateguard-effectiveness/coverage.js index bee2bc412..10a49a789 100644 --- a/docker/gateguard-effectiveness/coverage.js +++ b/docker/gateguard-effectiveness/coverage.js @@ -18,6 +18,11 @@ const { pairedComparison } = require('./lib'); // Rough floor for 80% power on a 40-point pass-rate difference at a two-sided // 5% level. Below it the report refuses to let counts stand as evidence. const MIN_PAIRS_FOR_A_CLAIM = 20; +const MAX_INVALID_TRIAL_FRACTION = 0.1; + +function isValidTrial(row) { + return !row.providerError && !row.timedOut && !row.judgeFailed; +} /** Outcome classes. The four ending in -hole are the improvable cases. */ const CLASSES = Object.freeze({ @@ -178,6 +183,20 @@ function renderEffect(rows, { referenceArm = 'gate', baselineArm = 'off' } = {}) } function renderHoles(rows, { baselineArm = 'off' } = {}) { + const invalid = rows.filter(row => !isValidTrial(row)); + const invalidFraction = rows.length ? invalid.length / rows.length : 0; + const validity = rows.length + ? `Excluded ${invalid.length}/${rows.length} invalid trial attempt(s) (provider error, timeout, or judge failure).` + : 'No trial attempts recorded.'; + if (invalidFraction > MAX_INVALID_TRIAL_FRACTION) { + return [ + '### Run validity', + '', + `INCONCLUSIVE: ${invalid.length}/${rows.length} trial attempts were invalid, above the 10% limit. Arm comparisons are suppressed.`, + '' + ].join('\n'); + } + rows = rows.filter(isValidTrial); const arms = [...new Set(rows.map(row => row.arm))].sort(); const classes = Object.keys(CLASSES); const lines = []; @@ -186,6 +205,7 @@ function renderHoles(rows, { baselineArm = 'off' } = {}) { const verdicts = informative(rows); lines.push('### Trap strength and usability', ''); + lines.push(validity, ''); lines.push('| Scenario | Ungated passed | Rate | Trap | Separates arms |', '| --- | ---: | ---: | --- | --- |'); const verdictOf = Object.fromEntries(verdicts.map(row => [row.scenario, row.label])); for (const row of strength) { @@ -236,6 +256,6 @@ function renderHoles(rows, { baselineArm = 'off' } = {}) { } module.exports = { - CLASSES, MIN_PAIRS_FOR_A_CLAIM, classify, tally, difficulty, informative, + CLASSES, MIN_PAIRS_FOR_A_CLAIM, MAX_INVALID_TRIAL_FRACTION, isValidTrial, classify, tally, difficulty, informative, discoveredQuestions, unengaged, renderEffect, renderHoles }; diff --git a/docker/gateguard-effectiveness/intent-eval.js b/docker/gateguard-effectiveness/intent-eval.js index ef3f99b7c..86d9751f7 100644 --- a/docker/gateguard-effectiveness/intent-eval.js +++ b/docker/gateguard-effectiveness/intent-eval.js @@ -11,7 +11,7 @@ const fs = require('fs'); const path = require('path'); -const { prepareWorkspace, commitWorkspace, grade, readMetrics } = require('./lib'); +const { prepareWorkspace, commitWorkspace, grade, readMetrics, hookWasObserved } = require('./lib'); const { ARMS, TRIAL_FILE, trialPatch } = require('./arms'); const { runConversation } = require('./session'); const { classify } = require('./coverage'); @@ -39,12 +39,14 @@ function loadIntentScenarios(dir = SCENARIO_DIR, ids = null) { if (!Array.isArray(spec.evidence) || !spec.evidence.length) throw new Error(`scenario ${spec.id} has no evidence`); if (typeof intent.ambiguity !== 'string' || !intent.ambiguity) throw new Error(`scenario ${spec.id} intent has no ambiguity`); if (typeof intent.stonewall !== 'string' || !intent.stonewall) throw new Error(`scenario ${spec.id} intent has no stonewall`); - if (!Array.isArray(intent.facts) || !intent.facts.some(fact => fact.decisive)) { - throw new Error(`scenario ${spec.id} intent has no decisive fact`); - } + if (!Array.isArray(intent.facts)) throw new Error(`scenario ${spec.id} intent has no decisive fact`); for (const fact of intent.facts) { if (!fact.id || typeof fact.fact !== 'string' || !fact.fact) throw new Error(`scenario ${spec.id} has a malformed fact`); } + const decisiveCount = intent.facts.filter(fact => fact.decisive === true).length; + if (decisiveCount !== 1) { + throw new Error(`scenario ${spec.id} intent must have exactly one decisive fact; found ${decisiveCount}`); + } return { ...spec, root, @@ -109,9 +111,14 @@ function runIntentTrial(trial, { workRoot, armSettingsByName, executable, model, judgeFailed: conversation.judgeFailed, providerError: conversation.providerError, providerMessage: conversation.providerMessage, - // Same guard as lib.js: a gated arm whose agent edited files with no metrics - // line and no denial means the hook never ran, so the trial proves nothing. - hookObserved: !ARMS[trial.arm].gate || conversation.editCalls === 0 || metrics.length > 0 || conversation.gateDenials > 0, + // Any hook-matched edit or shell call requires observable hook output. + hookObserved: hookWasObserved({ + gated: ARMS[trial.arm].gate, + editCalls: conversation.editCalls, + shellCalls: conversation.shellCalls, + metrics, + gateDenials: conversation.gateDenials + }), gateDenials: conversation.gateDenials, denialsPerTurn: conversation.denialsPerTurn, questionsAsked: [...new Set(denials.flatMap(event => event.questions || []))].sort(), diff --git a/docker/gateguard-effectiveness/lib.js b/docker/gateguard-effectiveness/lib.js index 5552f484b..5ea37e4b8 100644 --- a/docker/gateguard-effectiveness/lib.js +++ b/docker/gateguard-effectiveness/lib.js @@ -199,7 +199,7 @@ function toolResultText(content) { /** Reads a stream-json transcript: final result, assistant text and tool calls. */ function parseStream(stdout) { - const parsed = { result: null, texts: [], tools: {}, editCalls: 0, gateDenials: 0 }; + const parsed = { result: null, texts: [], tools: {}, editCalls: 0, shellCalls: 0, gateDenials: 0 }; for (const line of String(stdout || '').split('\n')) { if (!line.trim()) continue; let event; @@ -220,6 +220,7 @@ function parseStream(stdout) { if (block.type === 'tool_use') { parsed.tools[block.name] = (parsed.tools[block.name] || 0) + 1; if (['Edit', 'Write', 'MultiEdit', 'NotebookEdit'].includes(block.name)) parsed.editCalls++; + if (['Bash', 'PowerShell'].includes(block.name)) parsed.shellCalls++; } } } @@ -243,6 +244,10 @@ function readMetrics(stateDir) { .filter(Boolean); } +function hookWasObserved({ gated, editCalls, shellCalls, metrics, gateDenials }) { + return !gated || editCalls + shellCalls === 0 || metrics.length > 0 || gateDenials > 0; +} + /** Share of a scenario's evidence patterns that the agent's own text states. */ function evidenceRecall(scenario, texts) { const text = texts.join('\n'); @@ -310,7 +315,13 @@ function runTrial(trial, { workRoot, armSettingsByName, executable, model, maxTu denials: denials.length, gateDenials: stream.gateDenials, questionsAsked: [...new Set(denials.flatMap(event => event.questions || []))].sort(), - hookObserved: !gated || stream.editCalls === 0 || metrics.length > 0 || stream.gateDenials > 0, + hookObserved: hookWasObserved({ + gated, + editCalls: stream.editCalls, + shellCalls: stream.shellCalls, + metrics, + gateDenials: stream.gateDenials + }), ...usageOf(stream.result) }; } @@ -435,6 +446,7 @@ module.exports = { claudeArgs, parseStream, readMetrics, + hookWasObserved, evidenceRecall, runTrial, binomialTwoSided, diff --git a/docker/gateguard-effectiveness/run-intent.js b/docker/gateguard-effectiveness/run-intent.js index 8b1533f56..e1f0720d7 100644 --- a/docker/gateguard-effectiveness/run-intent.js +++ b/docker/gateguard-effectiveness/run-intent.js @@ -15,7 +15,7 @@ const { spawnSync } = require('child_process'); const { schedule } = require('./lib'); const { ARMS, armSettings, resolveRef, materializeTree } = require('./arms'); const { loadIntentScenarios, graderIsSound, runIntentTrial, SUPPORTED_ARMS } = require('./intent-eval'); -const { renderHoles, difficulty } = require('./coverage'); +const { renderHoles, difficulty, isValidTrial } = require('./coverage'); const REPO_ROOT = path.resolve(__dirname, '..', '..'); @@ -91,6 +91,10 @@ function readResults(file) { return fs.readFileSync(file, 'utf8').split('\n').filter(Boolean).map(line => JSON.parse(line)); } +function completedTrialKeys(rows) { + return new Set(rows.filter(isValidTrial).map(row => row.key)); +} + // Resuming a session needs persistence, so Claude writes a transcript folder per // trial workspace under its projects directory. A per-trial CLAUDE_CONFIG_DIR // would isolate those but also hides the credentials, so they are removed here @@ -123,7 +127,13 @@ function writeReport(options, rows, meta) { fs.writeFileSync(path.join(options.out, 'holes.md'), header + body); fs.writeFileSync( path.join(options.out, 'summary.json'), - `${JSON.stringify({ meta, difficulty: difficulty(rows), rows: rows.length }, null, 2)}\n` + `${JSON.stringify({ + meta, + difficulty: difficulty(rows.filter(isValidTrial)), + rows: rows.length, + validRows: rows.filter(isValidTrial).length, + invalidRows: rows.filter(row => !isValidTrial(row)).length + }, null, 2)}\n` ); return header + body; } @@ -197,7 +207,9 @@ function main() { const armSettingsByName = Object.fromEntries( options.arms.map(arm => [arm, armSettings(ARMS[arm].gate ? treeRoots[ARMS[arm].tree] : null)]) ); - const done = new Set(rows.map(row => row.key)); + // Failed attempts remain in results.jsonl for audit, but the scheduled key + // is complete only after a valid trial has been recorded. + const done = completedTrialKeys(rows); const trials = schedule(scenarios, options.arms, options.reps, options.seed).filter(trial => !done.has(trial.key)); process.stderr.write(`${trials.length} trials to run (${done.size} already recorded)\n`); if (options.dryRun) { @@ -264,4 +276,4 @@ if (require.main === module) { } } -module.exports = { parseArgs, writeReport, checkGraders }; +module.exports = { parseArgs, writeReport, checkGraders, completedTrialKeys }; diff --git a/docker/gateguard-effectiveness/session.js b/docker/gateguard-effectiveness/session.js index fa3761510..ed96dc01e 100644 --- a/docker/gateguard-effectiveness/session.js +++ b/docker/gateguard-effectiveness/session.js @@ -88,6 +88,7 @@ function runConversation({ cwd, stateDir, settingsPath, prompt, intent }, option const denialsPerTurn = []; let gateDenials = 0; let editCalls = 0; + let shellCalls = 0; const tools = {}; let judgeFailed = false; let asked = false; @@ -120,6 +121,7 @@ function runConversation({ cwd, stateDir, settingsPath, prompt, intent }, option denialsPerTurn.push(stream.gateDenials); gateDenials += stream.gateDenials; editCalls += stream.editCalls; + shellCalls += stream.shellCalls; for (const [name, count] of Object.entries(stream.tools)) tools[name] = (tools[name] || 0) + count; const message = finalText(stream); @@ -147,6 +149,7 @@ function runConversation({ cwd, stateDir, settingsPath, prompt, intent }, option // after a non-zero is the latch, not the gate choosing to stay quiet. denialsPerTurn, editCalls, + shellCalls, tools, judgeFailed, exitStatus: lastStatus, diff --git a/tests/docker/gateguard-effectiveness.test.js b/tests/docker/gateguard-effectiveness.test.js index 50228a3ed..fa7215e81 100644 --- a/tests/docker/gateguard-effectiveness.test.js +++ b/tests/docker/gateguard-effectiveness.test.js @@ -182,15 +182,16 @@ test('the child environment drops harness and hook variables but keeps the OAuth test('the stream parser counts tools, gate denials and usage', () => { const lines = [ - { type: 'assistant', message: { content: [{ type: 'text', text: 'I will check csv.js' }, { type: 'tool_use', name: 'Grep', input: {} }, { type: 'tool_use', name: 'Edit', input: {} }] } }, + { type: 'assistant', message: { content: [{ type: 'text', text: 'I will check csv.js' }, { type: 'tool_use', name: 'Grep', input: {} }, { type: 'tool_use', name: 'Edit', input: {} }, { type: 'tool_use', name: 'Bash', input: {} }] } }, { type: 'user', message: { content: [{ type: 'tool_result', is_error: true, content: [{ type: 'text', text: '[Fact-Forcing Gate]\n\nBefore editing' }] }] } }, { type: 'user', message: { content: [{ type: 'tool_result', content: 'ok' }] } }, 'not json', { type: 'result', is_error: false, num_turns: 4, usage: { output_tokens: 9 } } ].map(line => (typeof line === 'string' ? line : JSON.stringify(line))).join('\n'); const parsed = lib.parseStream(lines); - assert.deepStrictEqual(parsed.tools, { Grep: 1, Edit: 1 }); + assert.deepStrictEqual(parsed.tools, { Grep: 1, Edit: 1, Bash: 1 }); assert.strictEqual(parsed.editCalls, 1); + assert.strictEqual(parsed.shellCalls, 1); assert.strictEqual(parsed.gateDenials, 1); assert.strictEqual(parsed.result.num_turns, 4); assert.deepStrictEqual(parsed.texts, ['I will check csv.js']); diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index 0b6059c2c..7e8f6e53a 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -24,7 +24,7 @@ const userSim = require(path.join(DIR, 'user-sim')); const session = require(path.join(DIR, 'session')); const coverage = require(path.join(DIR, 'coverage')); const intentEval = require(path.join(DIR, 'intent-eval')); -const { parseArgs } = require(path.join(DIR, 'run-intent')); +const { parseArgs, completedTrialKeys } = require(path.join(DIR, 'run-intent')); let passed = 0; let failed = 0; @@ -70,7 +70,7 @@ test('every shipped hidden-intent scenario loads and declares a decisive fact', const scenarios = intentEval.loadIntentScenarios(); assert.ok(scenarios.length > 0, 'no hidden-intent scenarios found'); for (const scenario of scenarios) { - assert.ok(userSim.decisiveFacts(scenario.intent).length > 0, `${scenario.id} has no decisive fact`); + assert.strictEqual(userSim.decisiveFacts(scenario.intent).length, 1, `${scenario.id} must have exactly one decisive fact`); assert.strictEqual(scenario.targetQuestions, undefined, `${scenario.id} declares targetQuestions`); } }); @@ -98,7 +98,34 @@ test('an intent with no decisive fact is rejected', () => { JSON.stringify({ ...INTENT, facts: [{ id: 'ordering', fact: 'Order does not matter.', decisive: false }] }) ); fs.writeFileSync(path.join(scenario, 'grader.cjs'), 'console.log("ECC_EVAL_SCORE {\\"score\\":0}");'); - assert.throws(() => intentEval.loadIntentScenarios(dir), /no decisive fact/); + assert.throws(() => intentEval.loadIntentScenarios(dir), /exactly one decisive fact; found 0/); +}); + +test('an intent with multiple decisive facts is rejected', () => { + const dir = tempDir('gg-multidecisive-'); + const scenario = path.join(dir, 'multi'); + fs.mkdirSync(path.join(scenario, 'workspace'), { recursive: true }); + fs.writeFileSync(path.join(scenario, 'task.json'), JSON.stringify({ id: 'multi', trap: 't', prompt: 'p', evidence: ['e'] })); + fs.writeFileSync(path.join(scenario, 'intent.json'), JSON.stringify(INTENT)); + fs.writeFileSync(path.join(scenario, 'grader.cjs'), 'console.log("ECC_EVAL_SCORE {\\"score\\":0}");'); + assert.throws(() => intentEval.loadIntentScenarios(dir), /exactly one decisive fact; found 2/); +}); + +test('invalid trial attempts do not complete their scheduled key', () => { + const rows = [ + { key: 's/1/gate', providerError: true }, + { key: 's/2/gate', timedOut: true }, + { key: 's/3/gate', judgeFailed: true }, + { key: 's/4/gate' } + ]; + assert.deepStrictEqual([...completedTrialKeys(rows)], ['s/4/gate']); +}); + +test('a gated shell edit without hook output is not observed', () => { + const { hookWasObserved } = require(path.join(DIR, 'lib')); + assert.strictEqual(hookWasObserved({ gated: true, editCalls: 0, shellCalls: 1, metrics: [], gateDenials: 0 }), false); + assert.strictEqual(hookWasObserved({ gated: true, editCalls: 0, shellCalls: 1, metrics: [{ decision: 'allow' }], gateDenials: 0 }), true); + assert.strictEqual(hookWasObserved({ gated: false, editCalls: 0, shellCalls: 1, metrics: [], gateDenials: 0 }), true); }); // --- graders must discriminate, or the trial is worthless --- @@ -188,6 +215,28 @@ test('a ceiling scenario is excluded from the outcome-class table', () => { assert.strictEqual(gateLine.split('|')[2].trim(), '1', gateLine); }); +test('invalid attempts are excluded and counted in the report', () => { + const rows = []; + for (let rep = 1; rep <= 5; rep++) { + rows.push(row({ scenario: 'works', rep, arm: 'off', passed: false })); + rows.push(row({ scenario: 'works', rep, arm: 'gate', passed: true, turns: 8, costUsd: 0.1 })); + } + rows.push(row({ scenario: 'other', rep: 1, arm: 'gate', passed: false, providerError: true })); + const report = coverage.renderHoles(rows); + assert.match(report, /Excluded 1\/11 invalid trial attempt/); + assert.match(report, /\| gate \| 5\/5 \|/); +}); + +test('more than ten percent invalid trials make the report inconclusive', () => { + const rows = Array.from({ length: 10 }, (_, i) => row({ + scenario: 'works', rep: i, arm: i % 2 ? 'gate' : 'off', + passed: false, providerError: i < 2 + })); + const report = coverage.renderHoles(rows); + assert.match(report, /INCONCLUSIVE: 2\/10 trial attempts were invalid/); + assert.doesNotMatch(report, /### Effect and cost/); +}); + test('a small run is reported as underpowered, not as evidence', () => { // The counts this harness prints look like a result. With a handful of trials // they are not, so the report has to say so next to them.