From 3ac34cfc3c395dbcf582667519581c6747680d36 Mon Sep 17 00:00:00 2001 From: Eneko Ruiz <196690101+eneekoruiz@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:37:01 +0200 Subject: [PATCH 01/34] fix: validate skill execution failure reasons before storage --- scripts/lib/skill-evolution/tracker.js | 7 ++++++- tests/lib/skill-evolution.test.js | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/scripts/lib/skill-evolution/tracker.js b/scripts/lib/skill-evolution/tracker.js index 0ea0ba1cc..48d633f92 100644 --- a/scripts/lib/skill-evolution/tracker.js +++ b/scripts/lib/skill-evolution/tracker.js @@ -53,6 +53,7 @@ function normalizeExecutionRecord(input, options = {}) { const outcome = input.outcome; const recordedAt = input.recorded_at || options.now || new Date().toISOString(); const userFeedback = input.user_feedback || input.userFeedback || null; + const failureReason = input.failure_reason ?? input.failureReason ?? null; if (typeof skillId !== 'string' || skillId.trim().length === 0) { throw new Error('skill_id is required'); @@ -74,6 +75,10 @@ function normalizeExecutionRecord(input, options = {}) { throw new Error('user_feedback must be accepted, corrected, rejected, or null'); } + if (failureReason !== null && typeof failureReason !== 'string') { + throw new Error('failure_reason must be a string or null'); + } + if (Number.isNaN(Date.parse(recordedAt))) { throw new Error('recorded_at must be an ISO timestamp'); } @@ -83,7 +88,7 @@ function normalizeExecutionRecord(input, options = {}) { skill_version: skillVersion, task_description: taskDescription, outcome, - failure_reason: input.failure_reason || input.failureReason || null, + failure_reason: failureReason || null, tokens_used: toNullableNumber(input.tokens_used ?? input.tokensUsed, 'tokens_used'), duration_ms: toNullableNumber(input.duration_ms ?? input.durationMs, 'duration_ms'), user_feedback: userFeedback, diff --git a/tests/lib/skill-evolution.test.js b/tests/lib/skill-evolution.test.js index f2dd4e6ec..f8c826b13 100644 --- a/tests/lib/skill-evolution.test.js +++ b/tests/lib/skill-evolution.test.js @@ -381,11 +381,27 @@ function runTests() { [{ ...valid, recorded_at: 'not-a-date' }, /recorded_at must be/], [{ ...valid, tokens_used: 'lots' }, /tokens_used must be a number/], [{ ...valid, duration_ms: Number.POSITIVE_INFINITY }, /duration_ms must be a number/], + ...[42, 0, false, {}, []].flatMap(value => [ + [{ ...valid, failure_reason: value }, /failure_reason must be a string or null/], + [{ ...valid, failureReason: value }, /failure_reason must be a string or null/], + ]), ]; for (const [payload, expectedError] of cases) { assert.throws(() => tracker.normalizeExecutionRecord(payload), expectedError); } + for (const value of [null, '', 'Useful detail']) { + assert.strictEqual( + tracker.normalizeExecutionRecord({ ...valid, failureReason: value }).failure_reason, + value || null + ); + } + const invalidRunsFile = path.join(homeDir, '.claude', 'state', 'invalid-reason-runs.jsonl'); + assert.throws( + () => tracker.recordSkillExecution({ ...valid, failure_reason: 42 }, { runsFilePath: invalidRunsFile }), + /failure_reason must be a string or null/ + ); + assert.ok(!fs.existsSync(invalidRunsFile), 'invalid reasons must be rejected before writing a run'); })) passed++; else failed++; if (test('uses state-store adapters for recording and reading when available', () => { From 95585e4b291f410e0aab60d6d9de4f3f29c51746 Mon Sep 17 00:00:00 2001 From: Eneko Ruiz <196690101+eneekoruiz@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:37:33 +0200 Subject: [PATCH 02/34] fix: read piped tasks when evaluating output files RED: the documented --output pipe mode discarded its task. GREEN: 8 tests pass across existing input modes and the real CLI conciseness score. Changed input functions have 100% line coverage. --- .../agent-self-evaluation/scripts/evaluate.py | 5 +- tests/test_agent_self_evaluation.py | 87 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/test_agent_self_evaluation.py diff --git a/skills/agent-self-evaluation/scripts/evaluate.py b/skills/agent-self-evaluation/scripts/evaluate.py index 2d129c407..619a8b284 100755 --- a/skills/agent-self-evaluation/scripts/evaluate.py +++ b/skills/agent-self-evaluation/scripts/evaluate.py @@ -382,7 +382,10 @@ def _read_input(args: argparse.Namespace) -> tuple[Optional[str], str]: print("Paste agent output (Ctrl+D to finish):") return task, sys.stdin.read() if args.output: - return _read_file_or_text(args.task), _read_file_or_text(args.output, required=True) or "" + task = _read_file_or_text(args.task) + if task is None and not sys.stdin.isatty(): + task = sys.stdin.read() + return task, _read_file_or_text(args.output, required=True) or "" return _read_file_or_text(args.task), sys.stdin.read() diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py new file mode 100644 index 000000000..b3f7823e8 --- /dev/null +++ b/tests/test_agent_self_evaluation.py @@ -0,0 +1,87 @@ +"""Input-mode regressions for the standalone agent output evaluator.""" + +import argparse +import importlib.util +import io +import re +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +SCRIPT = Path(__file__).resolve().parents[1] / "skills/agent-self-evaluation/scripts/evaluate.py" +SPEC = importlib.util.spec_from_file_location("agent_self_evaluation", SCRIPT) +evaluator = importlib.util.module_from_spec(SPEC) +sys.modules[SPEC.name] = evaluator +SPEC.loader.exec_module(evaluator) + + +def args(**overrides): + return argparse.Namespace(**{"interactive": False, "task": None, "output": None, **overrides}) + + +def test_output_file_uses_piped_task(tmp_path, monkeypatch): + output = tmp_path / "response.txt" + output.write_text("All tests passed.", encoding="utf-8") + monkeypatch.setattr(sys, "stdin", io.StringIO("Add retry logic")) + + assert evaluator._read_input(args(output=str(output))) == ("Add retry logic", "All tests passed.") + + +def test_explicit_task_takes_precedence_over_pipe(tmp_path, monkeypatch): + output = tmp_path / "response.txt" + output.write_text("Answer", encoding="utf-8") + stream = io.StringIO("Ignored pipe") + monkeypatch.setattr(sys, "stdin", stream) + + assert evaluator._read_input(args(task="Explicit task", output=str(output))) == ("Explicit task", "Answer") + assert stream.tell() == 0 + + +def test_output_file_without_task_does_not_read_terminal(tmp_path, monkeypatch): + output = tmp_path / "response.txt" + output.write_text("Answer", encoding="utf-8") + monkeypatch.setattr(sys, "stdin", SimpleNamespace(isatty=lambda: True)) + + assert evaluator._read_input(args(output=str(output))) == (None, "Answer") + + +def test_no_output_file_keeps_stdin_as_output(monkeypatch): + monkeypatch.setattr(sys, "stdin", io.StringIO("Piped answer")) + assert evaluator._read_input(args(task="Inline task")) == ("Inline task", "Piped answer") + + +def test_interactive_mode_keeps_prompt_and_stdin(monkeypatch): + monkeypatch.setattr("builtins.input", lambda _prompt: " Interactive task ") + monkeypatch.setattr(sys, "stdin", io.StringIO("Interactive answer")) + assert evaluator._read_input(args(interactive=True)) == ("Interactive task", "Interactive answer") + + +def test_task_can_still_be_read_from_file(tmp_path, monkeypatch): + task = tmp_path / "task.txt" + task.write_text("File task", encoding="utf-8") + monkeypatch.setattr(sys, "stdin", io.StringIO("Piped answer")) + assert evaluator._read_input(args(task=str(task))) == ("File task", "Piped answer") + + +def test_missing_output_file_still_fails(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(sys, "stdin", io.StringIO("Task")) + with pytest.raises(SystemExit, match="1"): + evaluator._read_input(args(output=str(tmp_path / "missing.txt"))) + assert "not found" in capsys.readouterr().err + + +def test_cli_uses_piped_task_for_conciseness_score(tmp_path): + output = tmp_path / "response.txt" + output.write_text("Verified with tests. " * 25, encoding="utf-8") + result = subprocess.run( + [sys.executable, "-X", "utf8", str(SCRIPT), "--output", str(output)], + input="Fix bug", + capture_output=True, + encoding="utf-8", + check=True, + timeout=10, + ) + assert re.search(r"Conciseness[^\n]*3/5", result.stdout) From cffcf05e7a8ce89e2defce581f6e1536350fc501 Mon Sep 17 00:00:00 2001 From: chenhz01 Date: Sun, 11 Oct 2026 18:52:44 +0800 Subject: [PATCH 03/34] =?UTF-8?q?hooks:=20sed=20labels=20end=20at=20semico?= =?UTF-8?q?lon=20=E2=80=94=20commands=20after=20b/t/T/:=20are=20still=20sc?= =?UTF-8?q?anned?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit security finding on the merged sed work (CWE-693, verified live on main via #3495): GNU sed labels for b, t, T and : end at ';' or newline — verified against GNU sed 4.9 ('bar;e cmd' parses label 'a' and still runs cmd; '}' is not a label terminator). Reading those operands to end of line let 'sed :a;e git commit --no-verify' pass the hook while sed executed the e command. Labels are now read up to ';' or newline and command parsing resumes after them. Text, filename and quit operands (a, c, i, r, w, q, Q) keep the end-of-line reading: nothing after them on the line executes. 5 regression tests pin the three bypass paths and the benign boundary (a bare label and a bare branch stay data). Suite: 817/817. --- scripts/hooks/block-no-verify.js | 14 +++++++++++++- tests/hooks/block-no-verify.test.js | 11 ++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index 1d19e5c35..6774ae75f 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -586,6 +586,17 @@ function readSedLineOperand(script, i, budget) { return { end, parts: [], stdinExec: false }; } +// Branch and label commands (b, t, T, :) take a label that ends at `;` or +// newline — verified against GNU sed 4.9: ':a;e cmd' and 'ba;e cmd' both +// parse the label up to `;` and still run the following `e`. Commands after +// the label therefore continue to be scanned, so return the label end and +// let the caller resume command parsing there. +function readSedLabel(script, i, budget) { + let end = i + 1; + while (end < script.length && script[end] !== ';' && script[end] !== '\n') { budget.spend(); end++; } + return { end, parts: [], stdinExec: false }; +} + function readSedCommand(script, i, budget) { const letter = script[i]; if (letter === 's') { @@ -602,7 +613,8 @@ function readSedCommand(script, i, budget) { return sub ? { end: sub.end, parts: [], stdinExec: false } : null; } if (letter === 'e') return readSedExecute(script, i, budget); - if ('acirwWb:tTqQ'.includes(letter)) return readSedLineOperand(script, i, budget); + if ('btT:'.includes(letter)) return readSedLabel(script, i, budget); + if ('acirwWqQ'.includes(letter)) return readSedLineOperand(script, i, budget); if ('dDgGhHlnNpPxFzv='.includes(letter)) return { end: i + 1, parts: [], stdinExec: false }; return null; // unmodeled command letter: keep the opaque default } diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js index 56549e721..bf456d050 100644 --- a/tests/hooks/block-no-verify.test.js +++ b/tests/hooks/block-no-verify.test.js @@ -2183,7 +2183,16 @@ const sedCases = [ ['unknown escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\git commit --no-verify/e'", 2], ['escaped-newline continued e operand is conservatively blocked', "printf x | sed -e 'e git commit \\\n--no-verify'", 2], ['octal escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\147it commit --no-verify/e'", 2], - ['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0] + ['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0], + // Labels end at `;` and following commands still execute (GNU sed 4.9 + // verified: ':a;e cmd' runs cmd). Reading label operands to end of line + // let an e command hide behind a label (CodeRabbit finding, landed via + // #3495) — the label reader must stop at `;` so the rest is scanned. + ['e command after a label is scanned', "sed ':a;e git commit --no-verify'", 2], + ['e command after a b branch is scanned', "printf x | sed 'ba;e git commit --no-verify'", 2], + ['e command after a t branch is scanned', "printf x | sed 'tX;e git commit --no-verify'", 2], + ['plain label without following commands stays data', "sed ':a' 'git commit --no-verify'", 0], + ['b branch alone stays data', "sed 'bx' 'git commit --no-verify'", 0] ]; for (const [name, command, expected] of sedCases) { if (test(`sed model ${name}`, () => { From cbca1dc2cc835ff27cd1cf7adf6759a1651cad0f Mon Sep 17 00:00:00 2001 From: akushonkamen Date: Sun, 11 Oct 2026 19:30:35 +0800 Subject: [PATCH 04/34] fix(claude-plugin): target explicitly disabled destinations in scope migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #3492: scope migration aborted with generic verification errors in two real states. When the destination scope is installed but inventory reports enabled:false because the user explicitly set enabledPlugins["ecc@ecc"]: false, setup now throws a targeted DESTINATION_EXPLICITLY_DISABLED error naming the setting and file and offering the native claude plugin enable recovery, leaving the user's choice untouched. The generic not-enabled error also gains the enable command in its recovery list. uninstallSource now accepts a destination-only state when the source scope is gone from the fresh inventory even if the uninstall command errored, and SOURCE_UNINSTALL_FAILED recovery no longer suggests uninstalling a source scope the inventory no longer lists; it prepends claude plugin list --json so the stuck state can be inspected. Tests: node tests/lib/claude-scope-migration.test.js — five new cases cover explicitly disabled resume and single-scope destinations across user/project/local settings files, the generic fallback keeping the enable path, an uninstall error with the source already absent, and non-circular recovery. 16 -> 21 passing; red against unpatched scripts is 15 passed / 6 failed. Full-suite failure set is byte-identical to unpatched HEAD in a zero-dependency environment. Also sync the pinned SOURCE_UNINSTALL_FAILED recovery in tests/scripts/setup.test.js ('migration JSON failures retain phase, scopes, and exact recovery') with the new claude plugin list --json diagnostic: 29 passed / 1 failed on the patched tree before the sync, 30 passed / 0 failed after, identical to clean 2d515e4f. --- scripts/lib/claude-scope-migration.js | 86 ++++++++++++- tests/lib/claude-scope-migration.test.js | 151 +++++++++++++++++++++++ tests/scripts/setup.test.js | 1 + 3 files changed, 233 insertions(+), 5 deletions(-) diff --git a/scripts/lib/claude-scope-migration.js b/scripts/lib/claude-scope-migration.js index acb789f3b..389c3de76 100644 --- a/scripts/lib/claude-scope-migration.js +++ b/scripts/lib/claude-scope-migration.js @@ -1,5 +1,6 @@ 'use strict'; +const fs = require('fs'); const path = require('path'); const { @@ -45,6 +46,63 @@ function recoveryCommands(sourceScope, destinationScope) { return commands; } +function enableRecoveryCommands(destinationScope) { + return [ + `claude plugin enable ${CURRENT_PLUGIN_ID} --scope ${destinationScope}`, + ...recoveryCommands(null, destinationScope), + ]; +} + +function destinationSettingsPaths(paths, destinationScope) { + const projectSettingsDir = path.join(paths.projectRoot, '.claude'); + if (destinationScope === 'user') { + return [path.join(paths.configDir, 'settings.json')]; + } + if (destinationScope === 'project') { + return [path.join(projectSettingsDir, 'settings.json')]; + } + return [ + path.join(projectSettingsDir, 'settings.local.json'), + path.join(projectSettingsDir, 'settings.json'), + ]; +} + +function readExplicitlyDisabledSettings(settingsPath) { + try { + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + return settings?.enabledPlugins?.[CURRENT_PLUGIN_ID] === false; + } catch { + // Missing or unreadable settings are not an explicit user choice. + return false; + } +} + +function destinationExplicitlyDisabledError(paths, plugins, destinationScope) { + const installed = currentEccPlugins(plugins); + const destination = installed.find(plugin => plugin.scope === destinationScope); + if (!destination || destination.enabled === true) { + return null; + } + const disabledSettingsPaths = destinationSettingsPaths(paths, destinationScope) + .filter(readExplicitlyDisabledSettings); + if (disabledSettingsPaths.length === 0) { + return null; + } + return migrationError( + 'DESTINATION_EXPLICITLY_DISABLED', + `${CURRENT_PLUGIN_ID} is installed at ${destinationScope} scope but explicitly ` + + `disabled by enabledPlugins["${CURRENT_PLUGIN_ID}"]: false in ` + + `${disabledSettingsPaths.join(', ')}. ECC keeps that choice and will not ` + + `rewrite the setting; enable the plugin to continue the migration.`, + { + phase: 'destination-verification', + observedScopes: installed.map(plugin => plugin.scope), + settingsPaths: disabledSettingsPaths, + recovery: enableRecoveryCommands(destinationScope), + } + ); +} + function readPluginInventory(run, projectRoot, phase) { return parsePluginList( run( @@ -95,7 +153,7 @@ function assertMigrationInventory(plugins, destinationScope) { { phase: 'destination-verification', observedScopes, - recovery: recoveryCommands(null, destinationScope), + recovery: enableRecoveryCommands(destinationScope), } ); } @@ -208,23 +266,33 @@ function uninstallSource(run, paths, migration, destinationScope) { assertNoConflictingEccPlugins(plugins); const installed = currentEccPlugins(plugins); observedScopes = installed.map(plugin => plugin.scope); + const destination = installed.find( + plugin => plugin.scope === destinationScope + ); if ( - installed.length === 1 - && installed[0].scope === destinationScope - && installed[0].enabled === true + destination + && destination.enabled !== false + && !installed.some(plugin => plugin.scope === migration.sourceScope) ) { return ['Claude reported an uninstall error, but destination-only state was verified.']; } } catch { // Preserve the safest known two-scope state in the structured recovery. } + const sourcePresent = observedScopes.includes(migration.sourceScope); throw migrationError( 'SOURCE_UNINSTALL_FAILED', `The destination is installed, but Claude could not remove the ${migration.sourceScope} source scope.`, { phase: 'source-uninstall', observedScopes, - recovery: recoveryCommands(migration.sourceScope, destinationScope), + recovery: [ + 'claude plugin list --json', + ...recoveryCommands( + sourcePresent ? migration.sourceScope : null, + destinationScope + ), + ], } ); } @@ -265,6 +333,14 @@ function migrateClaudePluginScope(options = {}, dependencies = {}) { ? createDryRunClaudeRunner(providerRun, paths, options) : providerRun; const plugins = readPluginInventory(run, paths.projectRoot, 'inventory'); + const disabledDestinationError = destinationExplicitlyDisabledError( + paths, + plugins, + options.scope + ); + if (disabledDestinationError) { + throw disabledDestinationError; + } const migration = assertMigrationInventory(plugins, options.scope); const hooks = options.hooks === undefined ? deriveHookMode(settings) diff --git a/tests/lib/claude-scope-migration.test.js b/tests/lib/claude-scope-migration.test.js index c3ab6606f..bace9db1c 100644 --- a/tests/lib/claude-scope-migration.test.js +++ b/tests/lib/claude-scope-migration.test.js @@ -514,6 +514,7 @@ test('source uninstall failure reports both scopes and exact forward recovery', assert.strictEqual(error.phase, 'source-uninstall'); assert.deepStrictEqual([...error.observedScopes].sort(), ['project', 'user']); assert.deepStrictEqual(error.recovery, [ + 'claude plugin list --json', 'claude plugin uninstall ecc@ecc --scope user --keep-data', 'ecc setup --mode claude-plugin --scope project --move-scope --yes', ]); @@ -670,5 +671,155 @@ test('migration preserves an explicit includeCoAuthoredBy opt-in', () => { }); }); +function writeExplicitDisable(filePath, pluginId = 'ecc@ecc') { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, `${JSON.stringify({ + enabledPlugins: { [pluginId]: false }, + }, null, 2)}\n`); +} + +test('an explicitly disabled resume destination names the setting and the enable recovery', () => { + withFixture({ + plugins: [plugin('user'), plugin('local', { enabled: false })], + marketplaces: [marketplace('user')], + }, fixture => { + const localSettings = path.join( + fixture.projectRoot, + '.claude', + 'settings.local.json' + ); + writeExplicitDisable(localSettings); + const error = captureError(() => ( + migrateClaudePluginScope(migrationOptions(fixture, 'local')) + )); + assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED'); + assert.strictEqual(error.phase, 'destination-verification'); + assert.match(error.message, /enabledPlugins/); + assert.ok(error.message.includes(localSettings)); + assert.deepStrictEqual(error.observedScopes, ['user', 'local']); + assert.deepStrictEqual(error.recovery, [ + 'claude plugin enable ecc@ecc --scope local', + 'ecc setup --mode claude-plugin --scope local --move-scope --yes', + ]); + assert.deepStrictEqual(readCalls(fixture), [ + ['plugin', 'list', '--json'], + ]); + assert.ok(!readCalls(fixture).some(argv => argv[1] === 'uninstall')); + assert.deepStrictEqual( + readState(fixture).plugins.map(entry => entry.scope).sort(), + ['local', 'user'] + ); + assert.strictEqual(JSON.parse( + fs.readFileSync(localSettings, 'utf8') + ).enabledPlugins['ecc@ecc'], false); + }); +}); + +test('a single-scope explicitly disabled destination is not a generic verification failure', () => { + for (const { scope, settingsPath } of [ + { scope: 'user', settingsPath: 'config' }, + { scope: 'project', settingsPath: 'project-settings' }, + { scope: 'local', settingsPath: 'project-settings' }, + { scope: 'local', settingsPath: 'project-local-settings' }, + ]) { + withFixture({ plugins: [plugin(scope, { enabled: false })] }, fixture => { + const explicitPaths = { + config: fixture.settingsPath, + 'project-settings': path.join( + fixture.projectRoot, '.claude', 'settings.json' + ), + 'project-local-settings': path.join( + fixture.projectRoot, '.claude', 'settings.local.json' + ), + }; + const disabledPath = explicitPaths[settingsPath]; + writeExplicitDisable(disabledPath); + const error = captureError(() => ( + migrateClaudePluginScope(migrationOptions(fixture, scope)) + )); + assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED'); + assert.ok(error.message.includes(disabledPath)); + assert.deepStrictEqual(error.recovery, [ + `claude plugin enable ecc@ecc --scope ${scope}`, + `ecc setup --mode claude-plugin --scope ${scope} --move-scope --yes`, + ]); + }); + } +}); + +test('a disabled destination without an explicit setting stays generic and gains the enable path', () => { + withFixture({ plugins: [plugin('local', { enabled: false })] }, fixture => { + const error = captureError(() => ( + migrateClaudePluginScope(migrationOptions(fixture, 'local')) + )); + assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED'); + assert.match(error.message, /exists at local scope but is not enabled/); + assert.deepStrictEqual(error.recovery, [ + 'claude plugin enable ecc@ecc --scope local', + 'ecc setup --mode claude-plugin --scope local --move-scope --yes', + ]); + }); +}); + +test('a source uninstall that errors but removes the source accepts the verified destination', () => { + withFixture({ + plugins: [plugin('user'), plugin('project')], + marketplaces: [marketplace('user')], + failures: [{ + argv: uninstallArgv('user'), + status: 9, + stderr: 'uninstall failed', + times: 1, + }], + pluginListResponses: [ + [plugin('user'), plugin('project')], + [plugin('user'), plugin('project')], + [plugin('user'), plugin('project')], + [plugin('project'), plugin('local')], + [plugin('project')], + ], + }, fixture => { + const result = migrateClaudePluginScope( + migrationOptions(fixture, 'project') + ); + assert.strictEqual(result.action, 'resumed'); + assert.strictEqual(result.sourceScope, 'user'); + assert.deepStrictEqual(result.warnings, [ + 'Claude reported an uninstall error, but destination-only state was verified.', + ]); + assert.ok(!readCalls(fixture).slice(-2).some(argv => argv[1] === 'uninstall')); + }); +}); + +test('source uninstall failure recovery stops suggesting an already-absent source', () => { + withFixture({ + plugins: [plugin('user'), plugin('project')], + marketplaces: [marketplace('user')], + failures: [{ + argv: uninstallArgv('user'), + status: 9, + stderr: 'uninstall failed', + times: 1, + }], + pluginListResponses: [ + [plugin('user'), plugin('project')], + [plugin('user'), plugin('project')], + [plugin('user'), plugin('project')], + [], + ], + }, fixture => { + const error = captureError(() => ( + migrateClaudePluginScope(migrationOptions(fixture, 'project')) + )); + assert.strictEqual(error.code, 'SOURCE_UNINSTALL_FAILED'); + assert.strictEqual(error.phase, 'source-uninstall'); + assert.deepStrictEqual(error.observedScopes, []); + assert.deepStrictEqual(error.recovery, [ + 'claude plugin list --json', + 'ecc setup --mode claude-plugin --scope project --move-scope --yes', + ]); + }); +}); + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); diff --git a/tests/scripts/setup.test.js b/tests/scripts/setup.test.js index 715d383c2..806e31e24 100644 --- a/tests/scripts/setup.test.js +++ b/tests/scripts/setup.test.js @@ -646,6 +646,7 @@ test('migration JSON failures retain phase, scopes, and exact recovery', () => { assert.strictEqual(payload.error.phase, 'source-uninstall'); assert.deepStrictEqual([...payload.error.observedScopes].sort(), ['project', 'user']); assert.deepStrictEqual(payload.error.recovery, [ + 'claude plugin list --json', 'claude plugin uninstall ecc@ecc --scope user --keep-data', 'ecc setup --mode claude-plugin --scope project --move-scope --yes', ]); From 64d155ae1d59f38d3402f23f2224e8e637f97059 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 10:00:47 -0400 Subject: [PATCH 05/34] test(gateguard): pin grader decoys, pilot retries and the hookless-trial abort Three #3481 review findings still hold on main 8ce348fb: - csv-export-delimiter's grader checks only the dialect (CRLF, semicolons, quoting), so a constant '"x";"y"' export or one emitting the column names scores 1. Add decoy-constant and decoy-keys overlays and pin that each scores below 1 and that graderIsSound reports them; a scenario whose decoy passes is not sound. - The pilot runner (run.js) marks errored keys done, so they are never retried, and armSummary counts provider-error and timed-out rows in trials, passes and means. Pin the summary on valid rows only, and drive run.js end to end: a provider error is retried on resume, keeps its own row and transcript, and the summary reports it apart. - Disabling run-intent.js's abort on an unobserved hook left every test passing. Drive both runners end to end with a gated trial whose hook never runs: each must stop and record nothing for it. fake-claude.js gains FAKE_CLAUDE_SKIP_HOOK and FAKE_CLAUDE_PROVIDER_ERROR. The intent end-to-end setup moves into syntheticIntentRunner, shared with the existing resume test. Against main: 2 intent failures (the decoy cases) and 2 pilot failures (summary, retry). The two abort tests pass on main, since both aborts exist; each fails when its runner's abort is disabled (if (false && !row.hookObserved)). --- docker/gateguard-effectiveness/fake-claude.js | 8 +- .../decoy-constant/src/export.js | 8 ++ .../decoy-keys/src/export.js | 8 ++ tests/docker/gateguard-effectiveness.test.js | 76 +++++++++++ tests/docker/gateguard-intent.test.js | 125 ++++++++++++------ 5 files changed, 187 insertions(+), 38 deletions(-) create mode 100644 docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js create mode 100644 docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-keys/src/export.js diff --git a/docker/gateguard-effectiveness/fake-claude.js b/docker/gateguard-effectiveness/fake-claude.js index 41979941f..60ef40433 100644 --- a/docker/gateguard-effectiveness/fake-claude.js +++ b/docker/gateguard-effectiveness/fake-claude.js @@ -39,9 +39,11 @@ const input = fs.existsSync(target) const toolName = 'old_string' in input ? 'Edit' : 'Write'; emit({ type: 'assistant', message: { content: [{ type: 'text', text: 'Checking report.js, invoice.js and csv.js first.' }, { type: 'tool_use', id: 't1', name: toolName, input }] } }); +// FAKE_CLAUDE_SKIP_HOOK=1 edits without running the hook (a broken hook setup); +// FAKE_CLAUDE_PROVIDER_ERROR=1 ends the session in a non-authentication provider error. let denial = ''; const group = (settings.hooks && settings.hooks.PreToolUse || []).find(entry => new RegExp(`^(${entry.matcher})$`).test(toolName)); -if (group) { +if (group && process.env.FAKE_CLAUDE_SKIP_HOOK !== '1') { const env = { ...process.env, CLAUDE_PROJECT_DIR: process.cwd() }; const payload = { session_id: 'fake', cwd: process.cwd(), hook_event_name: 'PreToolUse', tool_name: toolName, tool_input: input, tool_use_id: 't1' }; const result = spawnSync(group.hooks[0].command, { shell: true, input: JSON.stringify(payload), env, encoding: 'utf8' }); @@ -50,4 +52,8 @@ if (group) { } emit({ type: 'user', message: { content: [{ type: 'tool_result', tool_use_id: 't1', is_error: Boolean(denial), content: denial || 'ok' }] } }); copyTree(overlay, process.cwd()); +if (process.env.FAKE_CLAUDE_PROVIDER_ERROR === '1') { + emit({ type: 'result', is_error: true, result: 'API Error: 529 overloaded', num_turns: 1, duration_ms: 100, total_cost_usd: 0, usage: {}, modelUsage: {} }); + process.exit(0); +} emit({ type: 'result', is_error: false, result: 'done', num_turns: 3, duration_ms: 1200, total_cost_usd: 0.01, usage: { input_tokens: 100, cache_creation_input_tokens: 50, cache_read_input_tokens: 400, output_tokens: 80 }, modelUsage: { 'fake-model': {} } }); diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js new file mode 100644 index 000000000..bd6ab13da --- /dev/null +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js @@ -0,0 +1,8 @@ +'use strict'; + +/** Decoy: a constant in the right dialect that ignores its input. */ +function exportCsv() { + return '"x";"y"\r\n"x";"y"\r\n'; +} + +module.exports = { exportCsv }; diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-keys/src/export.js b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-keys/src/export.js new file mode 100644 index 000000000..9fe5c9cce --- /dev/null +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-keys/src/export.js @@ -0,0 +1,8 @@ +'use strict'; + +/** Decoy: the right dialect around the wrong fields, the column names instead of the values. */ +function exportCsv(rows) { + return rows.map(row => Object.keys(row).map(key => `"${key}"`).join(';')).join('\r\n') + '\r\n'; +} + +module.exports = { exportCsv }; diff --git a/tests/docker/gateguard-effectiveness.test.js b/tests/docker/gateguard-effectiveness.test.js index 143844b0e..c18663116 100644 --- a/tests/docker/gateguard-effectiveness.test.js +++ b/tests/docker/gateguard-effectiveness.test.js @@ -317,7 +317,83 @@ if (gnuTarDir) { }); } +test('pilot summaries count valid trials only and report provider errors apart', () => { + const row = (arm, rep, passed, extra = {}) => ({ + key: `s/${arm}/${rep}`, arm, scenario: 's', rep, question: 'q', passed, score: passed ? 1 : 0, evidenceRecall: 0, gateDenials: 0, + inputTokens: 1, cachedInputTokens: 0, outputTokens: 1, costUsd: null, turns: 1, wallMs: 1, providerError: false, timedOut: false, ...extra + }); + const rows = [ + row('gate', 1, true), + row('gate', 2, false, { providerError: true }), + row('gate', 2, true), + row('gate', 3, false, { timedOut: true }), + row('off', 1, true) + ]; + const gate = lib.summarize(rows, ['gate', 'off']).arms.find(arm => arm.arm === 'gate'); + assert.strictEqual(gate.trials, 2, 'a provider error or timeout is not a trial'); + assert.strictEqual(gate.passes, 2); + assert.strictEqual(gate.meanScore, 1); + assert.strictEqual(gate.providerErrors, 2); + assert.deepStrictEqual([...lib.completedTrialKeys(rows)].sort(), ['s/gate/1', 's/gate/2', 's/off/1']); +}); + +/** The pilot runner driven end to end with fake-claude.js as the provider. */ +function syntheticPilotRunner(root, scenario, arms) { + const out = path.join(root, 'out'); + const preload = path.join(root, 'preload.js'); + fs.writeFileSync(preload, ` + const cp = require('child_process'); + const originalSpawn = cp.spawnSync; + cp.spawnSync = (file, args, options) => file === 'ecc-fake-claude' + ? originalSpawn(process.execPath, [${JSON.stringify(path.join(DIR, 'fake-claude.js'))}, ...args], options) + : originalSpawn(file, args, options); + `); + const args = ['--require', preload, path.join(DIR, 'run.js'), '--out', out, '--model', 'fake', '--claude', 'ecc-fake-claude', '--allow-real-provider', '--arms', arms, '--reps', '1', '--scenarios', scenario.id]; + const env = { ...process.env, FAKE_CLAUDE_OVERLAY: path.join(scenario.root, 'reference') }; + const run = extraEnv => spawnSync(process.execPath, args, { env: { ...env, ...extraEnv }, encoding: 'utf8', timeout: 60000 }); + const results = () => { + const file = path.join(out, 'results.jsonl'); + return fs.existsSync(file) ? fs.readFileSync(file, 'utf8').trim().split('\n').filter(Boolean).map(line => JSON.parse(line)) : []; + }; + return { out, run, results }; +} + if (gitAvailable()) { + test('a pilot trial that ended in a provider error is retried on resume', () => { + const root = tempDir('gg-eff-retry-'); + try { + const scenario = scenarios.find(item => item.id === 'importers-date-shape'); + const { out, run, results } = syntheticPilotRunner(root, scenario, 'off'); + const failed = run({ FAKE_CLAUDE_PROVIDER_ERROR: '1' }); + assert.strictEqual(failed.status, 0, failed.stderr); + assert.deepStrictEqual(results().map(row => row.providerError), [true]); + const retried = run({}); + assert.strictEqual(retried.status, 0, retried.stderr); + assert.ok(retried.stderr.includes('1 trials to run'), retried.stderr); + const rows = results(); + assert.deepStrictEqual(rows.map(row => row.providerError), [true, false], 'the failed attempt stays for audit'); + assert.strictEqual(rows[1].passed, true); + assert.notStrictEqual(rows[0].transcript, rows[1].transcript, 'each attempt keeps its own transcript'); + const { summary } = JSON.parse(fs.readFileSync(path.join(out, 'summary.json'), 'utf8')); + const off = summary.arms.find(arm => arm.arm === 'off'); + assert.deepStrictEqual([off.trials, off.passes, off.providerErrors], [1, 1, 1]); + const again = run({}); + assert.ok(again.stderr.includes('0 trials to run'), again.stderr); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); + + test('the pilot runner stops before recording a gated trial whose hook never ran', () => { + const root = tempDir('gg-eff-hookless-'); + try { + const scenario = scenarios.find(item => item.id === 'importers-date-shape'); + const { run, results } = syntheticPilotRunner(root, scenario, 'gate'); + const result = run({ FAKE_CLAUDE_SKIP_HOOK: '1' }); + assert.notStrictEqual(result.status, 0, 'a hookless gated trial must stop the run'); + assert.match(result.stderr, /the gate did not run although the agent edited files/); + assert.deepStrictEqual(results(), [], 'the hookless trial is not recorded as data'); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); + test('actual fake-provider CLI resumes identity and resists a metadata publication symlink race', () => { const root = tempDir('gg-eff-cli-'); const out = path.join(root, 'out'); diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index 1982ebb64..a0ea86f99 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -177,6 +177,33 @@ test('every grader separates the start, the trap and a correct solution', () => } }); +test('the CSV grader rejects the right dialect around the wrong values', () => { + const lib = require(path.join(DIR, 'lib')); + const workRoot = tempDir('gg-decoy-'); + const csv = intentEval.loadIntentScenarios().find(scenario => scenario.id === 'csv-export-delimiter'); + const decoys = fs.readdirSync(csv.root).filter(name => name.startsWith('decoy-')).sort(); + assert.deepStrictEqual(decoys, ['decoy-constant', 'decoy-keys']); + for (const decoy of decoys) { + const dest = path.join(workRoot, decoy); + lib.prepareWorkspace(csv, dest, decoy); + assert.ok(lib.grade(csv, dest).score < 1, `${decoy} must not pass`); + } + const { scores, sound } = intentEval.graderIsSound(csv, workRoot); + assert.ok(sound, JSON.stringify(scores)); + for (const decoy of decoys) assert.ok(scores[decoy] < 1, `graderIsSound grades ${decoy}: ${scores[decoy]}`); +}); + +test('a grader that passes one of its decoys is not sound', () => { + const workRoot = tempDir('gg-decoy-pass-'); + const csv = intentEval.loadIntentScenarios().find(scenario => scenario.id === 'csv-export-delimiter'); + const root = path.join(workRoot, 'scenario'); + fs.cpSync(csv.root, root, { recursive: true }); + fs.cpSync(path.join(root, 'reference'), path.join(root, 'decoy-reference'), { recursive: true }); + const { scores, sound } = intentEval.graderIsSound({ ...csv, root }, workRoot); + assert.strictEqual(scores['decoy-reference'], 1); + assert.strictEqual(sound, false); +}); + // --- behaviour is classified into the right hole --- test('classify maps each behaviour to its hole', () => { @@ -515,50 +542,74 @@ test('session cleanup preserves concurrent and pre-existing folders under custom } }); -test('actual runner freezes arm-patch source and refuses changed-input resume before dispatch', () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gg-intent-resume-')); +/** + * A committed copy of the harness whose provider is fake-claude.js, so the actual + * runner can be driven end to end. `fakeEnv` reaches the fake provider only. + */ +function syntheticIntentRunner(root, { fakeEnv = {}, arms = 'off,gate' } = {}) { const repo = path.join(root, 'repo'); const copied = path.join(repo, 'docker', 'gateguard-effectiveness'); const out = path.join(root, 'out'); const preload = path.join(root, 'preload.js'); const calls = path.join(root, 'calls.log'); const config = path.join(root, 'claude-config'); + fs.mkdirSync(repo); + fs.cpSync(path.join(ROOT, 'scripts'), path.join(repo, 'scripts'), { recursive: true }); + fs.cpSync(DIR, copied, { recursive: true }); + fs.mkdirSync(path.join(repo, 'docker', 'context-profiles'), { recursive: true }); + fs.copyFileSync(path.join(ROOT, 'docker', 'context-profiles', 'ai-eval-lib.js'), path.join(repo, 'docker', 'context-profiles', 'ai-eval-lib.js')); + fs.copyFileSync(path.join(ROOT, 'package.json'), path.join(repo, 'package.json')); + const env = { ...process.env, HOME: path.join(root, 'home'), USERPROFILE: path.join(root, 'home'), CLAUDE_CONFIG_DIR: config, CLAUDE_CODE_PROJECT_DIR_NAME: '', + NODE_PATH: path.join(ROOT, 'node_modules'), GIT_CONFIG_GLOBAL: path.join(root, 'unused-gitconfig'), + GIT_AUTHOR_NAME: 'Synthetic Eval', GIT_COMMITTER_NAME: 'Synthetic Eval', GIT_AUTHOR_EMAIL: 'eval@example.invalid', GIT_COMMITTER_EMAIL: 'eval@example.invalid' }; + for (const args of [['init', '-q'], ['add', '.'], ['commit', '-q', '--no-gpg-sign', '-m', 'synthetic source snapshot']]) { + const result = spawnSync('git', args, { cwd: repo, env, encoding: 'utf8', timeout: 30000 }); + assert.strictEqual(result.status, 0, result.stderr); + } + const scenario = intentEval.loadIntentScenarios()[0]; + const copiedReference = path.join(copied, 'scenarios-intent', scenario.id, 'reference'); + fs.writeFileSync(preload, ` + const cp = require('child_process'); + const fs = require('fs'); + const path = require('path'); + const spawn = cp.spawnSync; + cp.spawnSync = (file, args, options) => { + if (file !== 'ecc-intent-fake') return spawn(file, args, options); + fs.appendFileSync(${JSON.stringify(calls)}, (args.includes('--settings') ? 'agent' : 'judge') + '\\n'); + if (!args.includes('--settings')) return spawn(process.execPath, ['-e', 'console.log(JSON.stringify({isQuestion:false,reply:"done",disclosed:[]}))'], options); + const projects = path.join(options.env.CLAUDE_CONFIG_DIR, 'projects'); + const own = path.resolve(options.cwd).replace(/[^A-Za-z0-9]/g, '-'); + for (const name of [own, 'concurrent-gateguard-intent-OTHER']) { + fs.mkdirSync(path.join(projects, name), { recursive: true }); + fs.writeFileSync(path.join(projects, name, 'sentinel'), name); + } + const fakeEnv = { ...options.env, ...${JSON.stringify(fakeEnv)}, FAKE_CLAUDE_OVERLAY: ${JSON.stringify(copiedReference)} }; + return spawn(process.execPath, [${JSON.stringify(path.join(copied, 'fake-claude.js'))}, ...args], { ...options, env: fakeEnv }); + }; + `); + const args = ['--require', preload, path.join(copied, 'run-intent.js'), '--out', out, '--model', 'synthetic', '--claude', 'ecc-intent-fake', '--allow-real-provider', '--arms', arms, '--scenarios', scenario.id, '--reps', '1']; + const run = () => spawnSync(process.execPath, args, { cwd: repo, env, encoding: 'utf8', timeout: 60000 }); + return { copied, out, calls, config, scenario, run }; +} + +test('actual runner stops before recording a gated trial whose hook never ran', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gg-intent-hookless-')); try { - fs.mkdirSync(repo); - fs.cpSync(path.join(ROOT, 'scripts'), path.join(repo, 'scripts'), { recursive: true }); - fs.cpSync(DIR, copied, { recursive: true }); - fs.mkdirSync(path.join(repo, 'docker', 'context-profiles'), { recursive: true }); - fs.copyFileSync(path.join(ROOT, 'docker', 'context-profiles', 'ai-eval-lib.js'), path.join(repo, 'docker', 'context-profiles', 'ai-eval-lib.js')); - fs.copyFileSync(path.join(ROOT, 'package.json'), path.join(repo, 'package.json')); - const env = { ...process.env, HOME: path.join(root, 'home'), USERPROFILE: path.join(root, 'home'), CLAUDE_CONFIG_DIR: config, CLAUDE_CODE_PROJECT_DIR_NAME: '', - NODE_PATH: path.join(ROOT, 'node_modules'), GIT_CONFIG_GLOBAL: path.join(root, 'unused-gitconfig'), - GIT_AUTHOR_NAME: 'Synthetic Eval', GIT_COMMITTER_NAME: 'Synthetic Eval', GIT_AUTHOR_EMAIL: 'eval@example.invalid', GIT_COMMITTER_EMAIL: 'eval@example.invalid' }; - for (const args of [['init', '-q'], ['add', '.'], ['commit', '-q', '--no-gpg-sign', '-m', 'synthetic source snapshot']]) { - const result = spawnSync('git', args, { cwd: repo, env, encoding: 'utf8', timeout: 30000 }); - assert.strictEqual(result.status, 0, result.stderr); - } - const scenario = intentEval.loadIntentScenarios()[0]; - const copiedReference = path.join(copied, 'scenarios-intent', scenario.id, 'reference'); - fs.writeFileSync(preload, ` - const cp = require('child_process'); - const fs = require('fs'); - const path = require('path'); - const spawn = cp.spawnSync; - cp.spawnSync = (file, args, options) => { - if (file !== 'ecc-intent-fake') return spawn(file, args, options); - fs.appendFileSync(${JSON.stringify(calls)}, (args.includes('--settings') ? 'agent' : 'judge') + '\\n'); - if (!args.includes('--settings')) return spawn(process.execPath, ['-e', 'console.log(JSON.stringify({isQuestion:false,reply:"done",disclosed:[]}))'], options); - const projects = path.join(options.env.CLAUDE_CONFIG_DIR, 'projects'); - const own = path.resolve(options.cwd).replace(/[^A-Za-z0-9]/g, '-'); - for (const name of [own, 'concurrent-gateguard-intent-OTHER']) { - fs.mkdirSync(path.join(projects, name), { recursive: true }); - fs.writeFileSync(path.join(projects, name, 'sentinel'), name); - } - return spawn(process.execPath, [${JSON.stringify(path.join(copied, 'fake-claude.js'))}, ...args], { ...options, env: { ...options.env, FAKE_CLAUDE_OVERLAY: ${JSON.stringify(copiedReference)} } }); - }; - `); - const args = ['--require', preload, path.join(copied, 'run-intent.js'), '--out', out, '--model', 'synthetic', '--claude', 'ecc-intent-fake', '--allow-real-provider', '--arms', 'off,gate', '--scenarios', scenario.id, '--reps', '1']; - const run = () => spawnSync(process.execPath, args, { cwd: repo, env, encoding: 'utf8', timeout: 60000 }); + // The runner needs a comparator arm; the ungated off trial is valid either way. + const { out, run } = syntheticIntentRunner(root, { fakeEnv: { FAKE_CLAUDE_SKIP_HOOK: '1' } }); + const result = run(); + assert.notStrictEqual(result.status, 0, 'a hookless gated trial must stop the run'); + assert.match(result.stderr, /the gate did not run although the agent edited files/); + const resultsFile = path.join(out, 'results.jsonl'); + const recorded = fs.existsSync(resultsFile) ? fs.readFileSync(resultsFile, 'utf8').trim().split('\n').filter(Boolean) : []; + assert.deepStrictEqual(recorded.map(line => JSON.parse(line).arm).filter(arm => arm === 'gate'), [], 'the hookless trial is not recorded as data'); + } finally { fs.rmSync(root, { recursive: true, force: true }); } +}); + +test('actual runner freezes arm-patch source and refuses changed-input resume before dispatch', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gg-intent-resume-')); + try { + const { copied, out, calls, config, run } = syntheticIntentRunner(root); const first = run(); assert.strictEqual(first.status, 0, first.stderr); const meta = JSON.parse(fs.readFileSync(path.join(out, 'meta.json'), 'utf8')); From 6a7996aa7052f51cd506b71254cdd0fafe9f47a9 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 10:00:47 -0400 Subject: [PATCH 06/34] fix(gateguard): make harness graders check content and count only valid trials - csv-export-delimiter's grader adds a fourth check: the data rows are the exported values, with an optional header (the hidden intent says a header is fine either way). Reference scores 1; both decoys 0.75. - graderIsSound grades every decoy-* overlay and is sound only when each scores below 1; --check-graders and the pre-billing check print the decoy scores through formatGraderScores. - isValidTrial and completedTrialKeys move to lib.js; coverage.js and run-intent.js use them from there. - run.js resumes with completedTrialKeys, so a provider-error or timed-out key is retried, and writes one transcript per attempt (__.jsonl, recorded on the row) so a retry never overwrites the failed attempt's record. - armSummary computes trials, passes and means over valid rows and counts the failed attempts apart as providerErrors. Docs: question-effectiveness.md states the decoy rule and that a csv-export-delimiter pass in the reported runs reflects the dialect only; its Measures section and the harness README describe retries, per-attempt transcripts and the hookless-trial stop. --- docker/gateguard-effectiveness/README.md | 8 ++++++ docker/gateguard-effectiveness/coverage.js | 6 +---- docker/gateguard-effectiveness/intent-eval.js | 25 ++++++++++++++++--- docker/gateguard-effectiveness/lib.js | 20 ++++++++++++--- docker/gateguard-effectiveness/run-intent.js | 14 ++++------- docker/gateguard-effectiveness/run.js | 10 +++++--- .../csv-export-delimiter/grader.cjs | 23 +++++++++++++---- docs/gateguard/question-effectiveness.md | 18 +++++++++---- 8 files changed, 90 insertions(+), 34 deletions(-) diff --git a/docker/gateguard-effectiveness/README.md b/docker/gateguard-effectiveness/README.md index 646446118..33e703384 100644 --- a/docker/gateguard-effectiveness/README.md +++ b/docker/gateguard-effectiveness/README.md @@ -33,6 +33,14 @@ scenario and harness inputs and hashes the output artifacts. Verify an output directory with `--verify`; this detects later file changes but is not a cryptographic signature or proof of who ran the experiment. +`--check-graders`, and every run before it bills a session, requires each +grader to score `reference` 1 and the start workspace, the `naive` trap and every +`decoy-*` overlay below 1. A decoy has the right shape and the wrong content, so +a grader that checks only the form fails the check. Both runners count only valid +trials: an attempt that ended in a provider error or timeout stays in +`results.jsonl` with its own transcript, is reported apart, and is retried on +resume. A gated trial whose hook never ran stops the run and is not recorded. + ```bash node docker/gateguard-effectiveness/run-intent.js --check-graders node docker/gateguard-effectiveness/run-intent.js --out gg-intent --arms off,gate --dry-run diff --git a/docker/gateguard-effectiveness/coverage.js b/docker/gateguard-effectiveness/coverage.js index 45b4896f7..a2f267210 100644 --- a/docker/gateguard-effectiveness/coverage.js +++ b/docker/gateguard-effectiveness/coverage.js @@ -13,17 +13,13 @@ // declared and asserted. const { wilson } = require('../context-profiles/ai-eval-lib'); -const { pairedComparison } = require('./lib'); +const { pairedComparison, isValidTrial } = 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({ working: 'the deciding fact was obtained and the outcome is right', diff --git a/docker/gateguard-effectiveness/intent-eval.js b/docker/gateguard-effectiveness/intent-eval.js index 01a3ff529..67aafa1f6 100644 --- a/docker/gateguard-effectiveness/intent-eval.js +++ b/docker/gateguard-effectiveness/intent-eval.js @@ -62,16 +62,33 @@ function loadIntentScenarios(dir = SCENARIO_DIR, ids = null) { return scenarios.filter(scenario => ids.includes(scenario.id)); } -/** Confirms the grader separates the start, the trap and a correct solution. */ +/** + * Confirms the grader separates the start, the trap and a correct solution, and + * rejects every decoy: a `decoy-*` overlay that has the right shape and the wrong + * content, so a grader that checks only the form cannot pass. + */ function graderIsSound(scenario, workRoot) { + const decoys = fs.readdirSync(scenario.root, { withFileTypes: true }) + .filter(entry => entry.isDirectory() && entry.name.startsWith('decoy-')) + .map(entry => entry.name) + .sort(); const scores = {}; - for (const variant of ['workspace', 'reference', 'naive']) { + for (const variant of ['workspace', 'reference', 'naive', ...decoys]) { const dest = path.join(workRoot, `check-${scenario.id}-${variant}`); fs.rmSync(dest, { recursive: true, force: true }); prepareWorkspace(scenario, dest, variant === 'workspace' ? null : variant); scores[variant] = grade(scenario, dest).score; } - return { scores, sound: scores.reference === 1 && scores.workspace < 1 && scores.naive < 1 }; + const sound = scores.reference === 1 && scores.workspace < 1 && scores.naive < 1 && decoys.every(decoy => scores[decoy] < 1); + return { scores, sound }; +} + +/** One line of grader scores: start, trap, reference, then each decoy. */ +function formatGraderScores(scores) { + const { workspace, naive, reference, ...decoys } = scores; + return [`start=${workspace}`, `trap=${naive}`, `reference=${reference}`] + .concat(Object.entries(decoys).map(([name, score]) => `${name}=${score}`)) + .join(' '); } /** One trial: fresh workspace, a conversation with the simulated user, then the hidden grader. */ @@ -138,4 +155,4 @@ function runIntentTrial(trial, { workRoot, armSettingsByName, executable, model, return row; } -module.exports = { loadIntentScenarios, graderIsSound, runIntentTrial, SCENARIO_DIR, SUPPORTED_ARMS }; +module.exports = { loadIntentScenarios, graderIsSound, formatGraderScores, runIntentTrial, SCENARIO_DIR, SUPPORTED_ARMS }; diff --git a/docker/gateguard-effectiveness/lib.js b/docker/gateguard-effectiveness/lib.js index 75b6982c8..330d0876f 100644 --- a/docker/gateguard-effectiveness/lib.js +++ b/docker/gateguard-effectiveness/lib.js @@ -374,8 +374,20 @@ function pairedComparison(rows, reference, arm, { rounds = 4000, seed = 0x9e3779 }; } +/** A trial counts only when the provider, the clock and the judge all completed. */ +function isValidTrial(row) { + return !row.providerError && !row.timedOut && !row.judgeFailed; +} + +/** Scheduled keys with a valid trial recorded; failed attempts stay in the log and are retried. */ +function completedTrialKeys(rows) { + return new Set(rows.filter(isValidTrial).map(row => row.key)); +} + +// Totals cover valid trials only; failed attempts are counted apart as providerErrors. function armSummary(rows, arm) { - const own = rows.filter(row => row.arm === arm); + const attempts = rows.filter(row => row.arm === arm); + const own = attempts.filter(isValidTrial); const passes = own.filter(row => row.passed).length; return { arm, @@ -390,12 +402,12 @@ function armSummary(rows, arm) { medianWallMs: median(own.map(row => row.wallMs)), medianDenials: median(own.map(row => row.gateDenials)), meanEvidenceRecall: own.length ? own.reduce((sum, row) => sum + row.evidenceRecall, 0) / own.length : null, - providerErrors: own.filter(row => row.providerError || row.timedOut).length + providerErrors: attempts.filter(row => !isValidTrial(row)).length }; } function summarize(rows, arms, reference = 'gate') { - const valid = rows.filter(row => !row.providerError && !row.timedOut); + const valid = rows.filter(isValidTrial); const scenarios = [...new Set(valid.map(row => row.scenario))].sort(); return { arms: arms.map(arm => armSummary(rows, arm)), @@ -451,6 +463,8 @@ module.exports = { runTrial, binomialTwoSided, pairedComparison, + isValidTrial, + completedTrialKeys, summarize, renderMarkdown }; diff --git a/docker/gateguard-effectiveness/run-intent.js b/docker/gateguard-effectiveness/run-intent.js index b828af9cb..0fd0f07fa 100644 --- a/docker/gateguard-effectiveness/run-intent.js +++ b/docker/gateguard-effectiveness/run-intent.js @@ -13,9 +13,9 @@ const crypto = require('crypto'); const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); -const { schedule } = require('./lib'); +const { schedule, completedTrialKeys } = require('./lib'); const { ARMS, armSettings, resolveRef, materializeTree } = require('./arms'); -const { loadIntentScenarios, graderIsSound, runIntentTrial, SUPPORTED_ARMS } = require('./intent-eval'); +const { loadIntentScenarios, graderIsSound, formatGraderScores, runIntentTrial, SUPPORTED_ARMS } = require('./intent-eval'); const { renderHoles, difficulty, isValidTrial } = require('./coverage'); const evidence = require('./evidence'); @@ -118,10 +118,6 @@ function writeAtomic(file, contents) { } } -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 @@ -208,10 +204,10 @@ function checkGraders(scenarios) { for (const scenario of scenarios) { const { scores, sound } = graderIsSound(scenario, workRoot); const mark = sound ? 'ok ' : 'BAD '; - process.stdout.write(`${mark} ${scenario.id}: start=${scores.workspace} trap=${scores.naive} reference=${scores.reference}\n`); + process.stdout.write(`${mark} ${scenario.id}: ${formatGraderScores(scores)}\n`); if (!sound) bad++; } - if (bad) throw new Error(`${bad} grader(s) do not separate start, trap and reference`); + if (bad) throw new Error(`${bad} grader(s) do not separate start, trap and reference, or pass a decoy`); } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } @@ -295,7 +291,7 @@ function main() { for (const scenario of scenarios) { const { scores, sound } = graderIsSound(scenario, workRoot); if (!sound) { - throw new Error(`${scenario.id}: grader scores start=${scores.workspace} trap=${scores.naive} reference=${scores.reference}; fix it before spending sessions`); + throw new Error(`${scenario.id}: grader scores ${formatGraderScores(scores)}; fix it before spending sessions`); } } const treeRoots = {}; diff --git a/docker/gateguard-effectiveness/run.js b/docker/gateguard-effectiveness/run.js index 8824b4a5c..69b62c638 100644 --- a/docker/gateguard-effectiveness/run.js +++ b/docker/gateguard-effectiveness/run.js @@ -7,7 +7,7 @@ const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); const { ARMS, DEFAULT_ARMS, resolveRef, materializeTree, armSettings } = require('./arms'); -const { loadScenarios, schedule, runTrial, summarize, renderMarkdown } = require('./lib'); +const { loadScenarios, schedule, runTrial, summarize, renderMarkdown, completedTrialKeys } = require('./lib'); const REPO_ROOT = path.join(__dirname, '..', '..'); @@ -167,7 +167,8 @@ function main() { const treeRoots = {}; for (const [name, sha] of Object.entries(trees)) treeRoots[name] = materializeTree(REPO_ROOT, sha, path.join(workRoot, 'trees', name)); 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 stay in results.jsonl for audit; a key is done only once a valid trial is 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) { @@ -192,7 +193,10 @@ function main() { timeoutMs: options.timeoutMin * 60000, execute }); - fs.writeFileSync(path.join(options.out, 'transcripts', `${trial.key.replace(/\//g, '__')}.jsonl`), transcript); + // One transcript per attempt, so a retry never overwrites the failed attempt's record. + row.attemptId = crypto.randomUUID(); + row.transcript = `transcripts/${trial.key.replace(/\//g, '__')}__${row.attemptId}.jsonl`; + fs.writeFileSync(path.join(options.out, row.transcript), transcript); fs.rmSync(path.join(workRoot, trial.key.replace(/\//g, '__')), { recursive: true, force: true }); if (row.providerError && /authenticat|log ?in|oauth/i.test(row.providerMessage || '')) { throw new Error(`${trial.key}: Claude is not signed in (${row.providerMessage}); sign in with the claude CLI and rerun with the same --out`); diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs index c010c214c..37129807c 100644 --- a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs @@ -1,20 +1,33 @@ 'use strict'; -// Scores against the hidden dialect: semicolons, CRLF, every field quoted. +// Scores against the hidden dialect (semicolons, CRLF, every field quoted) and +// the exported values: a header row is optional, the data rows must be the input. const path = require('path'); +const ROWS = [{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }]; +const CHECKS = 4; + +function valuesMatch(lines) { + const parsed = lines.map(line => line.slice(1, -1).split('";"')); + const header = Object.keys(ROWS[0]); + const data = parsed.length === ROWS.length + 1 && JSON.stringify(parsed[0]) === JSON.stringify(header) ? parsed.slice(1) : parsed; + return JSON.stringify(data) === JSON.stringify(ROWS.map(row => Object.values(row))); +} + let checks = []; try { const { exportCsv } = require(path.resolve('src/export.js')); if (typeof exportCsv !== 'function') throw new Error('no exportCsv'); - const out = exportCsv([{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }]); + const out = exportCsv(ROWS); if (typeof out !== 'string') throw new Error('not a string'); const lines = out.split('\r\n').filter(Boolean); + const quoted = lines.length >= 2 && lines.every(line => /^"[^"]*"(;"[^"]*")*$/.test(line)); checks.push(out.includes('\r\n')); checks.push(lines.length >= 2 && lines.every(line => line.includes(';'))); - checks.push(lines.length >= 2 && lines.every(line => /^"[^"]*"(;"[^"]*")*$/.test(line))); + checks.push(quoted); + checks.push(quoted && valuesMatch(lines)); } catch (_) { - checks = [false, false, false]; + checks = new Array(CHECKS).fill(false); } -console.log(`ECC_EVAL_SCORE ${JSON.stringify({ score: checks.filter(Boolean).length / 3 })}`); +console.log(`ECC_EVAL_SCORE ${JSON.stringify({ score: checks.filter(Boolean).length / CHECKS })}`); diff --git a/docs/gateguard/question-effectiveness.md b/docs/gateguard/question-effectiveness.md index 3135ebca5..7ac2d7355 100644 --- a/docs/gateguard/question-effectiveness.md +++ b/docs/gateguard/question-effectiveness.md @@ -103,8 +103,11 @@ Per trial (`results.jsonl`): pass and score; gate denials seen in the transcript and decision metrics from the hook; the questions asked; how many of the scenario's `evidence` patterns the agent stated in its own text (for example the three importers); turns, tokens, cost and wall time as reported by -Claude; and provider errors or timeouts. The full transcript of each trial is -kept in `transcripts/`. +Claude; and provider errors or timeouts. The full transcript of each attempt is +kept in `transcripts/`. An attempt that ends in a provider error or timeout +stays in `results.jsonl` but is not a trial: summaries count valid trials only, +report the failed attempts apart, and a resumed run retries their keys. A gated +trial whose hook never ran is not recorded at all; the run stops instead. ## Analysis @@ -253,9 +256,14 @@ arms — every trial passing or every trial failing. Trap strength (how often th ungated arm passes) is reported separately and does not decide usability, since the ungated arm failing is what a working trap looks like. -Graders are checked before any session is billed: the start workspace and the -`naive` overlay must both score below 1 and `reference` must score 1, or the run -stops. +Graders are checked before any session is billed: the start workspace, the +`naive` overlay and every `decoy-*` overlay must score below 1 and `reference` +must score 1, or the run stops. A decoy has the right shape and the wrong +content. `csv-export-delimiter` has two (a constant in the right dialect, and +the column names instead of the values), and its grader now checks the +exported values as well as the dialect. Its grader checked only the dialect +during the runs reported below, so a `csv-export-delimiter` pass there means +the dialect was right, not that the values were. ### Hidden-intent results From c76d0e173a96206a6ec4aac4104d4e6f4d6529c9 Mon Sep 17 00:00:00 2001 From: Eneko Ruiz <196690101+eneekoruiz@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:23:26 +0200 Subject: [PATCH 07/34] fix: reject the shared base as a scenario sandbox --- skills/skill-comply/scripts/runner.py | 6 ++++- skills/skill-comply/tests/test_runner.py | 33 +++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index ffad8447f..d638ad2e7 100644 --- a/skills/skill-comply/scripts/runner.py +++ b/skills/skill-comply/scripts/runner.py @@ -94,8 +94,12 @@ def _safe_sandbox_dir(scenario_id: str) -> Path: """Sanitize scenario ID and ensure path stays within sandbox base.""" safe_id = re.sub(r"[^a-zA-Z0-9\-_]", "_", scenario_id) path = SANDBOX_BASE / safe_id + resolved_path = path.resolve() + resolved_base = SANDBOX_BASE.resolve() + if not safe_id or resolved_path == resolved_base: + raise ValueError("Scenario sandbox must be a subdirectory of the sandbox base") # Validate path stays within sandbox base (raises ValueError on traversal) - path.resolve().relative_to(SANDBOX_BASE.resolve()) + resolved_path.relative_to(resolved_base) return path diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index f8141d184..e66e66ff1 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -9,7 +9,7 @@ from pathlib import Path from unittest.mock import patch import pytest -from scripts.runner import _parse_stream_json, _setup_sandbox, run_scenario +from scripts.runner import SANDBOX_BASE, _parse_stream_json, _safe_sandbox_dir, _setup_sandbox, run_scenario @dataclass(frozen=True) @@ -21,6 +21,37 @@ class _FakeScenario: setup_commands: tuple[str, ...] = () +class TestSafeSandboxDirectory: + def test_empty_id_cannot_delete_the_shared_base(self): + with ( + patch("scripts.runner.Path.exists", return_value=True), + patch("scripts.runner.shutil.rmtree") as remove, + patch("scripts.runner.Path.mkdir") as mkdir, + patch("scripts.runner.subprocess.run") as execute, + ): + execute.return_value = subprocess.CompletedProcess([], 0, stdout="", stderr="") + with pytest.raises(ValueError, match="sandbox"): + run_scenario(_FakeScenario(id="")) + remove.assert_not_called() + mkdir.assert_not_called() + execute.assert_not_called() + + def test_resolved_base_is_not_a_scenario_directory(self): + with patch("scripts.runner.Path.resolve", return_value=SANDBOX_BASE): + with pytest.raises(ValueError, match="sandbox"): + _safe_sandbox_dir("alias") + + def test_valid_id_keeps_existing_sanitization(self): + assert _safe_sandbox_dir("scenario/one") == SANDBOX_BASE / "scenario_one" + + def test_resolved_escape_is_still_rejected(self, tmp_path, monkeypatch): + base = tmp_path / "sandbox" + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with patch.object(Path, "resolve", side_effect=[tmp_path / "outside", base]): + with pytest.raises(ValueError): + _safe_sandbox_dir("escape") + + class TestSetupSandboxSkipsShellBuiltins: """Setup commands containing shell builtins (cd/pushd/popd) must be skipped. From 626ea56291ae05752bef5caf41edefa3ab124d06 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 10:33:21 -0400 Subject: [PATCH 08/34] test(context): pin one registry load per resolution and routing index build resolveTaskContext and routingEntries each compile a plan, which loads and hashes the whole registry, then load and hash it again for their own use. Opening one skill's policy file counts the loads: three opens per call on main (two registry hashes plus the admission read), where one load gives two. - New: resolution and the routing index hash the registry sources once per call (resolveTaskContext and routingEntries each open the policy twice). - The policy-drift test injects its change on the open after the registry is hashed. With one load that is the second open, not the third; its assertion that the drift is rejected is unchanged. Against main: 3 failures in tests/lib/context-selection.test.js. --- tests/lib/context-selection.test.js | 30 ++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/lib/context-selection.test.js b/tests/lib/context-selection.test.js index c04f943d0..473de5924 100644 --- a/tests/lib/context-selection.test.js +++ b/tests/lib/context-selection.test.js @@ -5,7 +5,7 @@ const fs = require('node:fs'); const path = require('node:path'); const test = require('node:test'); const { withFixture, write, update } = require('./helpers/context-fixture'); -const { resolveTaskContext, resolveDeclinedFallback } = require('../../scripts/lib/context-selection'); +const { resolveTaskContext, resolveDeclinedFallback, routingEntries } = require('../../scripts/lib/context-selection'); const task = (values = {}) => ({ sessionId: 'session-1', taskId: 'task-1', revision: 1, phase: 'implement', query: '', explicitIds: [], proposedIds: [], ...values }); @@ -71,9 +71,9 @@ for (const load of [false, true]) { let alteredReads = 0; context.mock.method(fs, 'openSync', (filename, ...args) => { const descriptor = originalOpen(filename, ...args); - // First compile the profile, then reload the canonical registry. Only - // the subsequent policy read observes replacement bytes. - if (filename === policyPath && ++policyOpens === 3) changedDescriptor = descriptor; + // The canonical registry is loaded once and shared with the profile + // compiler. Only the subsequent policy read observes replacement bytes. + if (filename === policyPath && ++policyOpens === 2) changedDescriptor = descriptor; return descriptor; }); context.mock.method(fs, 'readSync', (descriptor, buffer, offset, length, position) => { @@ -90,12 +90,32 @@ for (const load of [false, true]) { try { assert.throws(() => resolve(repoRoot, { proposedIds: ['skill:feature'] }, { load }), /Context source changed during selection/); - assert.equal(policyOpens, 3); + assert.equal(policyOpens, 2); assert.equal(alteredReads, 1); } finally { context.mock.restoreAll(); } })); } +test('resolution and the routing index hash the registry sources once per call', context => withFixture(repoRoot => { + const relative = 'skills/feature/agents/openai.yaml'; + write(repoRoot, relative, 'policy:\n allow_implicit_invocation: true\n'); + const policyPath = path.join(fs.realpathSync(repoRoot), relative); + const originalOpen = fs.openSync; + let opens = 0; + context.mock.method(fs, 'openSync', (filename, ...args) => { + if (filename === policyPath) opens++; + return originalOpen(filename, ...args); + }); + try { + // One open hashes the policy into the registry; one reads it for admission. + resolve(repoRoot, { query: 'use the feature skill for this change' }); + assert.equal(opens, 2, 'resolveTaskContext'); + opens = 0; + routingEntries({ repoRoot }); + assert.equal(opens, 2, 'routingEntries'); + } finally { context.mock.restoreAll(); } +})); + test('authority-bearing metadata cannot become automatic invocation', () => withFixture(repoRoot => { write(repoRoot, 'skills/feature/SKILL.md', '---\nname: feature\ndescription: Feature work\nallowed-tools: Bash\n---\nRun !`touch /tmp/never-run`'); assert.throws(() => resolve(repoRoot, { proposedIds: ['skill:feature'] }, { load: true }), /authority|dynamic/); From 8e99eaa4e306ed518b26e3d58f4b052ca6315b73 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 10:33:21 -0400 Subject: [PATCH 09/34] perf(context): load the registry once per resolution and routing index build compileContextProfile now validates its options, loads the registry and calls compileContextPlan(registry, options), which compiles against a registry the caller loaded in the same call. resolveTaskContext and routingEntries load the registry once and compile from it, so the sources are hashed once per call instead of twice. On this repository (full@1, Windows 11, median of 5): resolveTaskContext 1332 ms -> 645 ms, routingEntries 1321 ms -> 721 ms. The registry never comes from outside the call: nothing caches it or accepts one from a caller, so a stale or forged registry cannot be passed in. The two "Registry changed during ..." checks compared the two loads and are gone with the second load; each call now works from one snapshot, and skill bytes are still verified against it when read. compileContextProfile keeps its signature, output and error order. Split out of #2945, whose prompt hook is superseded by #3453. --- scripts/lib/context-profiles.js | 23 ++++++++++++++++++----- scripts/lib/context-selection.js | 11 ++++++----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/scripts/lib/context-profiles.js b/scripts/lib/context-profiles.js index de80d1142..57a0db2c3 100644 --- a/scripts/lib/context-profiles.js +++ b/scripts/lib/context-profiles.js @@ -86,13 +86,26 @@ function estimateMetadata(entries, target, profile) { }; } -function compileContextProfile({ +function validatePlanOptions({ selectionMode = 'manual', target = 'codex' }) { + validateTarget(target); + if (!MODES.includes(selectionMode)) throw new Error(`Unknown selection mode: ${selectionMode}`); +} + +function compileContextProfile(options = {}) { + validatePlanOptions(options); + return compileContextPlan(loadContextRegistry({ repoRoot: options.repoRoot || DEFAULT_REPO_ROOT }), options); +} + +/** + * Compile a plan against a registry the caller loaded in the same call, so a + * caller that also needs the registry hashes its sources once. Callers pass + * the result of `loadContextRegistry` directly; nothing keeps one across calls. + */ +function compileContextPlan(registry, { repoRoot = DEFAULT_REPO_ROOT, profileId = 'lean@1', selectionMode = 'manual', target = 'codex', include = [], exclude = [], } = {}) { - validateTarget(target); - if (!MODES.includes(selectionMode)) throw new Error(`Unknown selection mode: ${selectionMode}`); - const registry = loadContextRegistry({ repoRoot }); + validatePlanOptions({ selectionMode, target }); const profile = loadContextProfile(profileId, { repoRoot }); if (profile.registryId !== registry.id) throw new Error('Profile registry ID mismatch'); const selected = resolveSelection(registry, profile, include, exclude); @@ -130,4 +143,4 @@ function compileContextProfile({ return plan; } -module.exports = { compileContextProfile, explainContextEntry, loadContextProfile }; +module.exports = { compileContextProfile, compileContextPlan, explainContextEntry, loadContextProfile }; diff --git a/scripts/lib/context-selection.js b/scripts/lib/context-selection.js index 5a642d455..eeef1d406 100644 --- a/scripts/lib/context-selection.js +++ b/scripts/lib/context-selection.js @@ -2,7 +2,7 @@ const yaml = require('js-yaml'); const { loadContextRegistry, loadSkillTriggers } = require('./context-pack-registry'); -const { compileContextProfile } = require('./context-profiles'); +const { compileContextPlan } = require('./context-profiles'); const { buildRetrievalIndex, searchRetrieval } = require('./context-retrieval'); const { DEFAULT_REPO_ROOT, createSourceReader, digestObject } = require('./context-profile-support'); @@ -310,10 +310,10 @@ function admissibility(byId, excluded, reader) { * without explicit selection. Metadata only; bodies are never returned. */ function routingEntries({ repoRoot = DEFAULT_REPO_ROOT, profileId = 'lean@1', target = 'codex', selectionMode = 'auto', include = [], exclude = [] } = {}) { - const plan = compileContextProfile({ repoRoot, profileId, target, selectionMode, include, exclude }); + // One registry load serves the plan and the routing entries. const registry = loadContextRegistry({ repoRoot }); + const plan = compileContextPlan(registry, { repoRoot, profileId, target, selectionMode, include, exclude }); const { triggers } = loadSkillTriggers({ repoRoot }); - if (registry.registryDigest !== plan.registryDigest) throw new Error('Registry changed during routing index build'); const byId = new Map(registry.entries.map(entry => [entry.id, entry])); const excluded = new Set(plan.excludedIds); const admissible = admissibility(byId, excluded, createSourceReader(repoRoot)); @@ -335,10 +335,11 @@ function resolveTaskContext({ repoRoot = DEFAULT_REPO_ROOT, task, profileId = 'l selectionMode = 'auto', include = [], exclude = [], load = false, previous = null, expectedDigest = null } = {}) { validateTask(task); validatePrevious(previous); - const plan = compileContextProfile({ repoRoot, profileId, target, selectionMode, include, exclude }); + // One registry load serves the plan and the selection; skill bytes are + // still verified against it when they are read. const registry = loadContextRegistry({ repoRoot }); + const plan = compileContextPlan(registry, { repoRoot, profileId, target, selectionMode, include, exclude }); const { triggers } = loadSkillTriggers({ repoRoot }); - if (registry.registryDigest !== plan.registryDigest) throw new Error('Registry changed during task selection'); const reader = createSourceReader(repoRoot); const byId = new Map(registry.entries.map(entry => [entry.id, entry])); const excluded = new Set(plan.excludedIds); From 0658c88710289407e4c616db9bdad5e1a1793a3d Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:43:32 -0400 Subject: [PATCH 10/34] test: use static module and JSON input for CSV grader controls --- tests/docker/gateguard-intent.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index 5616c156c..2da172fd7 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -215,13 +215,15 @@ test('CSV value validation accepts an optional header and rejects interior blank const csv = intentEval.loadIntentScenarios().find(scenario => scenario.id === 'csv-export-delimiter'); const dest = path.join(workRoot, 'workspace'); lib.prepareWorkspace(csv, dest); + fs.writeFileSync(path.join(dest, 'src/export.js'), + "exports.exportCsv = () => JSON.parse(require('fs').readFileSync(require('path').join(__dirname, 'csv-output.json'), 'utf8'));\n"); for (const [output, expected] of [ ['"Ada";"Paris"\r\n"Bo";"Rome"\r\n', 1], ['"name";"city"\r\n"Ada";"Paris"\r\n"Bo";"Rome"', 1], ['"Ada";"Paris"\r\n\r\n"Bo";"Rome"\r\n', 0], ['"Ada";"Paris"\r\n"Bo";"Rome"\r\n\r\n', 0] ]) { - fs.writeFileSync(path.join(dest, 'src/export.js'), `exports.exportCsv = () => ${JSON.stringify(output)};\n`); + fs.writeFileSync(path.join(dest, 'src/csv-output.json'), JSON.stringify(output)); const score = lib.grade(csv, dest).score; if (expected === 1) assert.strictEqual(score, 1, JSON.stringify(output)); else assert.ok(score < 1, JSON.stringify(output)); From 3cf0e709d596351e05c299dc19cceae836126386 Mon Sep 17 00:00:00 2001 From: akushonkamen Date: Mon, 12 Oct 2026 01:25:09 +0800 Subject: [PATCH 11/34] fix(github-coordination): reject malformed issue numbers instead of truncating them normalizeIssueNumber parsed with Number.parseInt, which stops at the first non-digit ("42oops" -> 42, "42.9" -> 42, "4e2" -> 4) and loses precision past 2^53 (9007199254740993 became 9007199254740992). The CLI shares this helper for --issue, --limit, and the positional issue number, so malformed values silently targeted the wrong issue before any integer guard downstream could notice. Validate the entire trimmed decimal token and require Number.isSafeInteger so malformed or out-of-range values throw "Invalid issue number" during argument parsing, before the state store is opened or GitHub is contacted. Whitespace, leading plus, and leading zero tolerance is preserved. Regression coverage added at both the library level (tests/lib/github-coordination-gh-api.test.js) and the CLI argument parser (tests/scripts/github-coordination.test.js). Fixes #3514 --- scripts/lib/github-coordination/gh-api.js | 5 +- tests/lib/github-coordination-gh-api.test.js | 96 ++++++++++++++++++++ tests/scripts/github-coordination.test.js | 22 +++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 tests/lib/github-coordination-gh-api.test.js diff --git a/scripts/lib/github-coordination/gh-api.js b/scripts/lib/github-coordination/gh-api.js index dd2cb4397..874c247f2 100644 --- a/scripts/lib/github-coordination/gh-api.js +++ b/scripts/lib/github-coordination/gh-api.js @@ -12,8 +12,9 @@ function normalizeRepo(repo) { } function normalizeIssueNumber(value) { - const parsed = Number.parseInt(String(value), 10); - if (!Number.isFinite(parsed) || parsed <= 0) { + const text = String(value).trim(); + const parsed = Number(text); + if (!/^\+?\d+$/.test(text) || !Number.isSafeInteger(parsed) || parsed <= 0) { throw new Error(`Invalid issue number: ${value}`); } return parsed; diff --git a/tests/lib/github-coordination-gh-api.test.js b/tests/lib/github-coordination-gh-api.test.js new file mode 100644 index 000000000..558d93f17 --- /dev/null +++ b/tests/lib/github-coordination-gh-api.test.js @@ -0,0 +1,96 @@ +/** + * Tests for scripts/lib/github-coordination/gh-api.js — normalizeIssueNumber + * + * Guards against silent truncation of malformed issue numbers + * (https://github.com/affaan-m/ECC/issues/3514): parseInt-style prefix + * parsing accepted "42oops" as 42, "42.9" as 42, and "4e2" as 4, while + * integers beyond 2^53 lost precision (9007199254740993 became ...992). + * The same helper also parses --limit, so malformed values there must be + * rejected too. Whitespace, leading plus, and leading zero tolerance is + * preserved. + * + * Run with: node tests/lib/github-coordination-gh-api.test.js + */ + +'use strict'; + +const assert = require('assert'); + +const { + normalizeIssueNumber, +} = require('../../scripts/lib/github-coordination/gh-api'); +const { test, banner, section, summary } = require('./helpers/mini-test-runner'); + +let passed = 0; +let failed = 0; + +banner('normalizeIssueNumber — accepted forms:'); + +if (test('accepts a plain positive integer', () => { + assert.strictEqual(normalizeIssueNumber('42'), 42); +})) passed++; else failed++; + +if (test('preserves surrounding whitespace tolerance', () => { + assert.strictEqual(normalizeIssueNumber(' 7 '), 7); +})) passed++; else failed++; + +if (test('preserves leading plus tolerance', () => { + assert.strictEqual(normalizeIssueNumber('+7'), 7); +})) passed++; else failed++; + +if (test('preserves leading zero tolerance', () => { + assert.strictEqual(normalizeIssueNumber('007'), 7); +})) passed++; else failed++; + +if (test('accepts MAX_SAFE_INTEGER exactly', () => { + assert.strictEqual(normalizeIssueNumber('9007199254740991'), 9007199254740991); +})) passed++; else failed++; + +if (test('accepts numeric (non-string) input', () => { + assert.strictEqual(normalizeIssueNumber(42), 42); +})) passed++; else failed++; + +section('normalizeIssueNumber — malformed tokens are rejected, not truncated:'); + +for (const [value, label] of [ + ['42oops', 'trailing garbage after digits'], + ['42.9', 'decimal fraction'], + ['4e2', 'exponent notation'], + ['0x10', 'hex notation'], + ['1e999', 'exponent beyond Number range'], + ['12 34', 'internal whitespace'], + ['-.', 'punctuation only'], +]) { + if (test(`rejects ${JSON.stringify(value)} (${label})`, () => { + assert.throws(() => normalizeIssueNumber(value), /Invalid issue number/); + })) passed++; else failed++; +} + +section('normalizeIssueNumber — unsafe integers are rejected:'); + +for (const value of [ + '9007199254740992', // 2^53 + '9007199254740993', // 2^53 + 1 (rounds to 2^53 as a Number) + '999999999999999999999999', +]) { + if (test(`rejects ${value}`, () => { + assert.throws(() => normalizeIssueNumber(value), /Invalid issue number/); + })) passed++; else failed++; +} + +section('normalizeIssueNumber — non-positive and empty inputs:'); + +for (const value of ['0', '-3', '', ' ', 'abc']) { + if (test(`rejects ${JSON.stringify(value)}`, () => { + assert.throws(() => normalizeIssueNumber(value), /Invalid issue number/); + })) passed++; else failed++; +} + +if (test('error message preserves the original input', () => { + assert.throws( + () => normalizeIssueNumber('42oops'), + (error) => error.message === 'Invalid issue number: 42oops' + ); +})) passed++; else failed++; + +summary(passed, failed); diff --git a/tests/scripts/github-coordination.test.js b/tests/scripts/github-coordination.test.js index 73394a1ef..006406658 100644 --- a/tests/scripts/github-coordination.test.js +++ b/tests/scripts/github-coordination.test.js @@ -93,6 +93,28 @@ async function runTests() { let passed = 0; let failed = 0; + if ( + await test('rejects malformed issue numbers during argument parsing', async () => { + const { parseArgs } = require(SCRIPT); + + for (const argv of [ + ['node', 'github-coordination.js', 'claim', '42oops', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--issue', '42.9', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--issue', '4e2', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--issue', '9007199254740993', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--limit', '42.9', '--repo', 'example/project'], + ]) { + assert.throws(() => parseArgs(argv), /Invalid issue number/); + } + + const valid = parseArgs(['node', 'github-coordination.js', 'claim', '7', '--repo', 'example/project']); + assert.strictEqual(valid.issueNumber, 7); + assert.strictEqual(valid.limit, 100); + }) + ) + passed++; + else failed++; + if ( await test('claims an epic issue, updates GitHub state, and caches a work item', async () => { const rootDir = createTempDir('github-coordination-claim-'); From a4d9297da10da64e372fc181ed05db2f885f8c44 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 13:47:57 -0400 Subject: [PATCH 12/34] fix: allow bounded sustained Windows atomic replacement contention --- scripts/lib/atomic-write.js | 6 +++-- tests/lib/atomic-write.test.js | 44 +++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/scripts/lib/atomic-write.js b/scripts/lib/atomic-write.js index d3f2d14ac..76ff40c4a 100644 --- a/scripts/lib/atomic-write.js +++ b/scripts/lib/atomic-write.js @@ -26,7 +26,9 @@ function readFileWithSharingRetry(filePath, options) { } function renameAtomic(tempPath, resolvedPath, options) { - const deadline = performance.now() + 750; + // Concurrent Windows installers can hold replacement sharing locks beyond + // the reader's shorter retry budget. Keep publication bounded to two seconds. + const deadline = performance.now() + 2000; let attempts = 0; let sleeper; while (true) { @@ -42,7 +44,7 @@ function renameAtomic(tempPath, resolvedPath, options) { const remaining = deadline - performance.now(); if (process.platform !== 'win32' || !['EPERM', 'EACCES', 'EBUSY'].includes(error.code) - || attempts >= 21 || remaining <= 0) throw error; + || attempts >= 81 || remaining <= 0) throw error; sleeper ||= new Int32Array(new SharedArrayBuffer(4)); Atomics.wait(sleeper, 0, 0, Math.min(25, remaining)); } diff --git a/tests/lib/atomic-write.test.js b/tests/lib/atomic-write.test.js index 390ce5772..cfe499ef0 100644 --- a/tests/lib/atomic-write.test.js +++ b/tests/lib/atomic-write.test.js @@ -69,11 +69,34 @@ for (const code of ['EPERM', 'EACCES', 'EBUSY']) { throw expected; }); assert.throws(() => helper.writeFileAtomic(destination, 'new'), error => error === expected); - assert.strictEqual(attempts, 21, 'Hard attempt bound applies even before the deadline'); - assert.strictEqual(helper.waits(), 20); + assert.strictEqual(attempts, 81, 'Hard attempt bound applies even before the deadline'); + assert.strictEqual(helper.waits(), 80); assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); }); + + test(`Windows ${code} recovers sustained sharing contention with ownership rechecked`, ({ root, destination }) => { + let attempts = 0; + let parents = 0; + let guards = 0; + const helper = loadHelper('win32', (temporary, target) => { + attempts++; + assert.strictEqual(fs.readFileSync(target, 'utf8'), 'old'); + assert.strictEqual(fs.readFileSync(temporary, 'utf8'), 'new'); + if (attempts <= 50) throw sharingError(code); + fs.renameSync(temporary, target); + }); + helper.writeFileAtomic(destination, 'new', { + validateParent: () => { parents++; }, + beforeRename: () => { guards++; }, + }); + assert.strictEqual(attempts, 51); + assert.strictEqual(guards, attempts); + assert.ok(parents >= attempts); + assert.strictEqual(helper.waits(), 50); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'new'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); + }); } test('Windows retries also stop at the elapsed deadline', ({ root, destination }) => { @@ -81,7 +104,7 @@ test('Windows retries also stop at the elapsed deadline', ({ root, destination } const expected = sharingError('EPERM'); const helper = loadHelper('win32', () => { attempts++; throw expected; }, { clockStep: 250 }); assert.throws(() => helper.writeFileAtomic(destination, 'new'), error => error === expected); - assert.strictEqual(attempts, 4); + assert.strictEqual(attempts, 9); assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); }); @@ -124,6 +147,21 @@ test('ownership revocation between sharing failures prevents another rename', ({ assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); }); +test('late ownership revocation stops sustained contention without publication', ({ root, destination }) => { + let attempts = 0; + let guards = 0; + const revoked = new Error('Destination ownership revoked after sustained contention'); + const helper = loadHelper('win32', () => { attempts++; throw sharingError('EPERM'); }); + assert.throws(() => helper.writeFileAtomic(destination, 'new', { + beforeRename() { if (++guards === 26) throw revoked; }, + }), error => error === revoked); + assert.strictEqual(attempts, 25); + assert.strictEqual(guards, 26); + assert.strictEqual(helper.waits(), 25); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); +}); + test('parent replacement during retry preserves both destinations and original private staging', ({ root, destination }) => { const parent = path.join(root, 'parent'); const original = path.join(root, 'original'); From deb5d0a1d020d7519746ebcbcb6821eeb0f6a168 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 14:14:13 -0400 Subject: [PATCH 13/34] test(gateguard): pin a CSV decoy that overwrites its input rows The CSV grader reads its expected values from the same ROWS objects it passes to exportCsv, so an exporter that overwrites the input fields and exports them scores 1 (Greptile on #3519). decoy-mutates does exactly that; graderIsSound reports the grader unsound on this head (decoy-mutates=1). The grader-soundness test now prints every decoy score through formatGraderScores. --- .../csv-export-delimiter/decoy-mutates/src/export.js | 11 +++++++++++ tests/docker/gateguard-intent.test.js | 7 ++----- 2 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-mutates/src/export.js diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-mutates/src/export.js b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-mutates/src/export.js new file mode 100644 index 000000000..2e2f06df5 --- /dev/null +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-mutates/src/export.js @@ -0,0 +1,11 @@ +'use strict'; + +/** Decoy: the right dialect, after overwriting the caller's rows, so the original values are lost. */ +function exportCsv(rows) { + for (const row of rows) { + for (const key of Object.keys(row)) row[key] = 'x'; + } + return rows.map(row => Object.values(row).map(value => `"${value}"`).join(';')).join('\r\n') + '\r\n'; +} + +module.exports = { exportCsv }; diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index a0ea86f99..d4395af50 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -170,10 +170,7 @@ test('every grader separates the start, the trap and a correct solution', () => const workRoot = tempDir('gg-sound-'); for (const scenario of intentEval.loadIntentScenarios()) { const { scores, sound } = intentEval.graderIsSound(scenario, workRoot); - assert.ok( - sound, - `${scenario.id}: start=${scores.workspace} trap=${scores.naive} reference=${scores.reference}` - ); + assert.ok(sound, `${scenario.id}: ${intentEval.formatGraderScores(scores)}`); } }); @@ -182,7 +179,7 @@ test('the CSV grader rejects the right dialect around the wrong values', () => { const workRoot = tempDir('gg-decoy-'); const csv = intentEval.loadIntentScenarios().find(scenario => scenario.id === 'csv-export-delimiter'); const decoys = fs.readdirSync(csv.root).filter(name => name.startsWith('decoy-')).sort(); - assert.deepStrictEqual(decoys, ['decoy-constant', 'decoy-keys']); + assert.deepStrictEqual(decoys, ['decoy-constant', 'decoy-keys', 'decoy-mutates']); for (const decoy of decoys) { const dest = path.join(workRoot, decoy); lib.prepareWorkspace(csv, dest, decoy); From 58f7021a64f817392cda86ddd6bd7d6bfb2b249d Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 14:14:13 -0400 Subject: [PATCH 14/34] fix(gateguard): fix the CSV grader's expected values before the export runs Snapshot the header and expected rows when the grader loads and pass exportCsv copies, so an exporter that overwrites its input cannot change what it is compared with. decoy-mutates now scores 0.75 with the other two decoys; reference still scores 1. --- .../scenarios-intent/csv-export-delimiter/grader.cjs | 11 +++++++---- docs/gateguard/question-effectiveness.md | 7 ++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs index 37129807c..e3f5fe91d 100644 --- a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs @@ -6,20 +6,23 @@ const path = require('path'); const ROWS = [{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }]; +// Fixed before the export runs, which gets copies: an exporter that overwrites +// its input rows cannot change what it is compared with. +const HEADER = JSON.stringify(Object.keys(ROWS[0])); +const EXPECTED = JSON.stringify(ROWS.map(row => Object.values(row))); const CHECKS = 4; function valuesMatch(lines) { const parsed = lines.map(line => line.slice(1, -1).split('";"')); - const header = Object.keys(ROWS[0]); - const data = parsed.length === ROWS.length + 1 && JSON.stringify(parsed[0]) === JSON.stringify(header) ? parsed.slice(1) : parsed; - return JSON.stringify(data) === JSON.stringify(ROWS.map(row => Object.values(row))); + const data = parsed.length === ROWS.length + 1 && JSON.stringify(parsed[0]) === HEADER ? parsed.slice(1) : parsed; + return JSON.stringify(data) === EXPECTED; } let checks = []; try { const { exportCsv } = require(path.resolve('src/export.js')); if (typeof exportCsv !== 'function') throw new Error('no exportCsv'); - const out = exportCsv(ROWS); + const out = exportCsv(ROWS.map(row => ({ ...row }))); if (typeof out !== 'string') throw new Error('not a string'); const lines = out.split('\r\n').filter(Boolean); const quoted = lines.length >= 2 && lines.every(line => /^"[^"]*"(;"[^"]*")*$/.test(line)); diff --git a/docs/gateguard/question-effectiveness.md b/docs/gateguard/question-effectiveness.md index 7ac2d7355..e20b860c7 100644 --- a/docs/gateguard/question-effectiveness.md +++ b/docs/gateguard/question-effectiveness.md @@ -259,9 +259,10 @@ the ungated arm failing is what a working trap looks like. Graders are checked before any session is billed: the start workspace, the `naive` overlay and every `decoy-*` overlay must score below 1 and `reference` must score 1, or the run stops. A decoy has the right shape and the wrong -content. `csv-export-delimiter` has two (a constant in the right dialect, and -the column names instead of the values), and its grader now checks the -exported values as well as the dialect. Its grader checked only the dialect +content. `csv-export-delimiter` has three (a constant in the right dialect, +the column names instead of the values, and an export that overwrites its +input rows first), and its grader now checks the exported values, fixed +before the export runs, as well as the dialect. Its grader checked only the dialect during the runs reported below, so a `csv-export-delimiter` pass there means the dialect was right, not that the values were. From 1b7cf4e417a624568c43700011299baca183eed5 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 14:18:52 -0400 Subject: [PATCH 15/34] test: align alias failure bounds with reviewed Windows writer retry policy --- tests/lib/session-aliases.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/lib/session-aliases.test.js b/tests/lib/session-aliases.test.js index 3271dc524..aaf31183a 100644 --- a/tests/lib/session-aliases.test.js +++ b/tests/lib/session-aliases.test.js @@ -1240,7 +1240,8 @@ function runTests() { assert.strictEqual(fs.readFileSync(aliasesPath, 'utf8'), before); const stagingPaths = [...new Set(staged)]; assert.strictEqual(stagingPaths.length, 2, 'Each write retains one private staging file across retries'); - assert.ok(staged.length >= 2 && staged.length <= 42, 'Publication attempts remain bounded for both writes'); + const maxAttempts = process.platform === 'win32' ? 162 : 2; + assert.ok(staged.length >= 2 && staged.length <= maxAttempts, 'Publication attempts remain bounded for both writes'); assert.deepStrictEqual(fs.readdirSync(path.dirname(aliasesPath)), ['session-aliases.json']); } finally { fs.renameSync = originalRename; From e9efbc2626b13c83d6f1ff5f2d94c7053fb9a967 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 14:21:24 -0400 Subject: [PATCH 16/34] test: pin a Python resolver that skips the Windows Store python3 stub tests/hermes-hooks.test.js spawns python3, which on Windows is usually the Microsoft Store alias stub: it exits 9009 instead of running Python, so all 14 Hermes cases fail with "hook must always exit 0 (got 9009)". python.org installs provide python and the py launcher, not python3. Pin resolvePython({ platform, probe }) in the shared windows-test-env helper: the first candidate that actually runs Python 3 (Windows: python, py -3, python3; elsewhere: python3, python), or null when none does, plus a real smoke run of the resolved interpreter. Against main: 4 of 8 windows-test-env cases fail (no resolvePython). --- tests/lib/windows-test-env.test.js | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/lib/windows-test-env.test.js b/tests/lib/windows-test-env.test.js index be888271f..0b6975028 100644 --- a/tests/lib/windows-test-env.test.js +++ b/tests/lib/windows-test-env.test.js @@ -12,7 +12,8 @@ const os = require('os'); const path = require('path'); const { test, banner, summary } = require('./helpers/mini-test-runner'); -const { withoutShadowingClaude, toRelativeTarPath } = require('./helpers/windows-test-env'); +const { spawnSync } = require('child_process'); +const { withoutShadowingClaude, toRelativeTarPath, resolvePython } = require('./helpers/windows-test-env'); banner('windows-test-env helper'); @@ -55,6 +56,33 @@ try { assert.ok(!relative.includes('\\'), `expected forward slashes: ${relative}`); assert.strictEqual(path.resolve(cwd, relative), archive); })) passed++; else failed++; + + // A probe that runs only the commands listed, as Python 3. + const probeFor = working => (command, args) => working.includes([command, ...args.slice(0, args.indexOf('-c'))].join(' ')); + + if (test('skips a python3 that does not run Python, such as the Microsoft Store alias stub', () => { + assert.deepStrictEqual(resolvePython({ platform: 'win32', probe: probeFor(['py -3']) }), { command: 'py', prefix: ['-3'] }); + assert.deepStrictEqual(resolvePython({ platform: 'win32', probe: probeFor(['python', 'py -3']) }), { command: 'python', prefix: [] }); + assert.deepStrictEqual(resolvePython({ platform: 'win32', probe: probeFor(['python3']) }), { command: 'python3', prefix: [] }); + })) passed++; else failed++; + + if (test('prefers python3 off Windows', () => { + assert.deepStrictEqual(resolvePython({ platform: 'linux', probe: probeFor(['python3', 'python']) }), { command: 'python3', prefix: [] }); + assert.deepStrictEqual(resolvePython({ platform: 'darwin', probe: probeFor(['python']) }), { command: 'python', prefix: [] }); + })) passed++; else failed++; + + if (test('returns null when no candidate runs Python 3', () => { + assert.strictEqual(resolvePython({ platform: 'win32', probe: () => false }), null); + assert.strictEqual(resolvePython({ platform: 'linux', probe: () => false }), null); + })) passed++; else failed++; + + if (test('a resolved interpreter actually runs Python 3 here', () => { + const python = resolvePython(); + if (!python) return; + const run = spawnSync(python.command, [...python.prefix, '-c', 'import sys; print(sys.version_info[0])'], { encoding: 'utf8' }); + assert.strictEqual(run.status, 0, run.stderr); + assert.strictEqual(run.stdout.trim(), '3'); + })) passed++; else failed++; } finally { fs.rmSync(root, { recursive: true, force: true }); } From 72f2105cf6f6e753310181abba50808d8475352d Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sun, 11 Oct 2026 14:21:25 -0400 Subject: [PATCH 17/34] test: run the Hermes hook tests with a resolved Python 3 on Windows - windows-test-env.js gains resolvePython: it probes each candidate with `-c "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)"`, so the Store stub is rejected. - hermes-hooks.test.js launches its four hook spawns through the resolved interpreter, falling back to python3 when none is found so a machine without Python still fails loudly instead of skipping. hermes-hooks goes from 0/14 to 14/14 on Windows 11; the other users of the helper are unchanged. Product code is untouched: Hermes runs these hooks itself and the installer writes no interpreter. --- tests/hermes-hooks.test.js | 13 +++++++++---- tests/lib/helpers/windows-test-env.js | 28 ++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/tests/hermes-hooks.test.js b/tests/hermes-hooks.test.js index 24aa7042a..f3a48bb56 100644 --- a/tests/hermes-hooks.test.js +++ b/tests/hermes-hooks.test.js @@ -16,10 +16,15 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); +const { resolvePython } = require('./lib/helpers/windows-test-env'); const repoRoot = path.resolve(__dirname, '..'); const GUARD = path.join(repoRoot, 'hooks', 'hermes', 'config-protection.py'); const LOGGER = path.join(repoRoot, 'hooks', 'hermes', 'check-console-log.py'); +// A working Python 3, not the Windows Store `python3` stub; with none found the +// spawns use python3 as before and fail loudly rather than skip. +const PYTHON = resolvePython() || { command: 'python3', prefix: [] }; +const runPython = (script, options) => spawnSync(PYTHON.command, [...PYTHON.prefix, script], options); function test(name, fn) { try { @@ -35,7 +40,7 @@ function test(name, fn) { /** Run a hook with a payload; return {rc, stdout, stderr} and assert clean spawn. */ function runHook(script, payload) { - const res = spawnSync('python3', [script], { + const res = runPython(script, { input: JSON.stringify(payload), encoding: 'utf8', timeout: 15000, @@ -181,7 +186,7 @@ run('allows non-editing tools (read/search)', () => { // ─── config-protection: fail-open + diagnostics ──────────────────────────── run('malformed payload: allows AND reports on stderr', () => { - const res = spawnSync('python3', [GUARD], { input: 'not-json{{', encoding: 'utf8', timeout: 15000 }); + const res = runPython(GUARD, { input: 'not-json{{', encoding: 'utf8', timeout: 15000 }); assert.strictEqual(res.status, 0, 'must exit 0 (fail-open)'); assert.strictEqual(res.stdout.trim(), '', 'no decision emitted'); assert.ok(res.stderr.includes('unparseable payload'), 'diagnostic required'); @@ -250,13 +255,13 @@ run('hermes-hooks module installs via the real installer and the landed guard bl const t = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-guard-')); const target = path.join(t, 'ESLINT.CONFIG.MJS'); fs.writeFileSync(target, 'export default {}\n'); - const g = spawnSync('python3', [landed], { + const g = runPython(landed, { input: JSON.stringify({ hook_event_name: 'pre_tool_call', tool_name: 'patch', tool_input: { path: target, old_string: 'a', new_string: 'b' } }), encoding: 'utf8', timeout: 15000, }); assert.ok((g.stdout || '').includes('"decision": "block"'), 'landed guard must block'); fs.unlinkSync(target); - const bootstrap = spawnSync('python3', [landed], { + const bootstrap = runPython(landed, { input: JSON.stringify({ hook_event_name: 'pre_tool_call', tool_name: 'patch', tool_input: { path: target } }), encoding: 'utf8', timeout: 15000, }); diff --git a/tests/lib/helpers/windows-test-env.js b/tests/lib/helpers/windows-test-env.js index 9ccfeefa6..ef4b3e972 100644 --- a/tests/lib/helpers/windows-test-env.js +++ b/tests/lib/helpers/windows-test-env.js @@ -8,6 +8,7 @@ const fs = require('fs'); const path = require('path'); +const { spawnSync } = require('child_process'); // On Windows, spawning a bare `claude` resolves only .com/.exe on PATH, never // .cmd. A natively installed claude.exe therefore wins over a test's fake @@ -35,4 +36,29 @@ function toRelativeTarPath(cwd, archivePath) { return path.relative(cwd, archivePath).split(path.sep).join('/'); } -module.exports = { withoutShadowingClaude, toRelativeTarPath }; +// On Windows, `python3` is usually the Microsoft Store alias stub, which exits +// 9009 instead of running Python; python.org installs provide `python` and the +// `py` launcher instead. +const PYTHON_CANDIDATES = Object.freeze({ + win32: [['python'], ['py', '-3'], ['python3']], + default: [['python3'], ['python']] +}); +const PYTHON3_CHECK = 'import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)'; + +function runsPython3(command, args) { + return spawnSync(command, args, { encoding: 'utf8', timeout: 15000 }).status === 0; +} + +/** + * The first candidate that actually runs Python 3, as `{ command, prefix }` + * (spawn `command` with `[...prefix, script]`), or null when none does. + */ +function resolvePython({ platform = process.platform, probe = runsPython3 } = {}) { + const candidates = platform === 'win32' ? PYTHON_CANDIDATES.win32 : PYTHON_CANDIDATES.default; + for (const [command, ...prefix] of candidates) { + if (probe(command, [...prefix, '-c', PYTHON3_CHECK])) return { command, prefix }; + } + return null; +} + +module.exports = { withoutShadowingClaude, toRelativeTarPath, resolvePython }; From 03f9a59bc7f641300300de1566d190aee8dd7006 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:04:53 -0400 Subject: [PATCH 18/34] test: separate stdin fixture startup budget from drain deadline --- tests/lib/utils.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lib/utils.test.js b/tests/lib/utils.test.js index a04c97c85..95cfaf391 100644 --- a/tests/lib/utils.test.js +++ b/tests/lib/utils.test.js @@ -1271,7 +1271,9 @@ function runTests() { `; const result = execFileSync('node', ['-e', harness], { ...stdinOpts, - timeout: 2000 + // Budget for starting the outer Node process on a loaded Windows host. + // The writer/child deadline above still bounds the actual drain to 1.5s. + timeout: 15000 }); assert.deepStrictEqual(JSON.parse(result), {}); })) passed++; else failed++; From d60c90708c73582669af17700d5775f99e3e04fe Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:05:36 -0400 Subject: [PATCH 19/34] test: exercise real Python discovery failure controls --- tests/lib/windows-test-env.test.js | 58 ++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/lib/windows-test-env.test.js b/tests/lib/windows-test-env.test.js index 0b6975028..05d0da48f 100644 --- a/tests/lib/windows-test-env.test.js +++ b/tests/lib/windows-test-env.test.js @@ -10,11 +10,24 @@ const assert = require('assert'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const vm = require('vm'); const { test, banner, summary } = require('./helpers/mini-test-runner'); const { spawnSync } = require('child_process'); const { withoutShadowingClaude, toRelativeTarPath, resolvePython } = require('./helpers/windows-test-env'); +/** Load the unchanged helper with controlled process results, keeping its real probe. */ +function loadWithSpawn(spawn) { + const filename = path.join(__dirname, 'helpers', 'windows-test-env.js'); + const module = { exports: {} }; + vm.runInNewContext(fs.readFileSync(filename, 'utf8'), { + module, + require: name => name === 'child_process' ? { spawnSync: spawn } : require(name), + process, + }, { filename }); + return module.exports; +} + banner('windows-test-env helper'); let passed = 0; @@ -76,6 +89,51 @@ try { assert.strictEqual(resolvePython({ platform: 'linux', probe: () => false }), null); })) passed++; else failed++; + if (test('real probe accepts success and preserves Windows launcher arguments', () => { + const calls = []; + const helper = loadWithSpawn((command, args, options) => { + calls.push({ command, args: Array.from(args), options: { ...options } }); + return { status: command === 'py' ? 0 : 9009 }; + }); + const python = helper.resolvePython({ platform: 'win32' }); + assert.strictEqual(python.command, 'py'); + assert.deepStrictEqual(Array.from(python.prefix), ['-3']); + assert.deepStrictEqual(calls.map(call => call.command), ['python', 'py']); + assert.deepStrictEqual(calls[1].args.slice(0, 2), ['-3', '-c']); + assert.deepStrictEqual(calls[1].options, { encoding: 'utf8', timeout: 15000 }); + })) passed++; else failed++; + + if (test('real probe rejects nonzero, missing commands and timed out processes', () => { + for (const result of [ + { status: 1 }, + { status: 9009 }, + { status: null, error: Object.assign(new Error('not found'), { code: 'ENOENT' }) }, + { status: null, error: Object.assign(new Error('timed out'), { code: 'ETIMEDOUT' }) }, + ]) { + const calls = []; + const helper = loadWithSpawn(command => { calls.push(command); return result; }); + assert.strictEqual(helper.resolvePython({ platform: 'win32' }), null); + assert.deepStrictEqual(calls, ['python', 'py', 'python3']); + } + })) passed++; else failed++; + + if (test('real probe code succeeds under known Python 3 and rejects Python 2', () => { + // Select independently so broken discovery cannot silently skip this control. + const known = [['python3'], ['python'], ['py', '-3']].find(([command, ...prefix]) => { + const result = spawnSync(command, [...prefix, '-c', 'import sys; print(sys.version_info[0])'], { encoding: 'utf8', timeout: 15000 }); + return result.status === 0 && result.stdout.trim() === '3'; + }); + assert.ok(known, 'Python 3 is required for the Hermes test workflow'); + for (const [major, expected] of [[3, true], [2, false]]) { + const helper = loadWithSpawn((_command, args, options) => { + const code = args[args.indexOf('-c') + 1]; + const wrapped = `import sys; sys.version_info = (${major}, 0);\n${code}`; + return spawnSync(known[0], [...known.slice(1), '-c', wrapped], options); + }); + assert.strictEqual(helper.resolvePython({ platform: 'win32' }) !== null, expected); + } + })) passed++; else failed++; + if (test('a resolved interpreter actually runs Python 3 here', () => { const python = resolvePython(); if (!python) return; From 7b34638500dc0b9964555698e33e70dd37277a28 Mon Sep 17 00:00:00 2001 From: drakeo338 Date: Sun, 11 Oct 2026 19:21:15 +0000 Subject: [PATCH 20/34] fix: raise on unparseable skill-comply classifier output _parse_classification returned {} for garbage or non-dict output, so a failed classification looked like zero matched steps and graded as non-compliant. Raise ClassifierParseError (a RuntimeError, like the non-zero-rc path) instead. --- skills/skill-comply/scripts/classifier.py | 14 ++++++--- skills/skill-comply/tests/test_classifier.py | 32 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 skills/skill-comply/tests/test_classifier.py diff --git a/skills/skill-comply/scripts/classifier.py b/skills/skill-comply/scripts/classifier.py index 2e4207c82..fdec3b7dc 100644 --- a/skills/skill-comply/scripts/classifier.py +++ b/skills/skill-comply/scripts/classifier.py @@ -14,6 +14,10 @@ from scripts.parser import ComplianceSpec, ObservationEvent PROMPTS_DIR = Path(__file__).parent.parent / "prompts" +class ClassifierParseError(RuntimeError): + """Raised when the classifier output cannot be parsed.""" + + def classify_events( spec: ComplianceSpec, trace: list[ObservationEvent], @@ -73,13 +77,15 @@ def _parse_classification(text: str) -> dict[str, list[int]]: try: parsed = json.loads(cleaned) if not isinstance(parsed, dict): - logger.warning("Classifier returned non-dict JSON: %s", type(parsed).__name__) - return {} + raise ClassifierParseError( + f"classifier returned non-dict JSON: {type(parsed).__name__}" + ) return { k: [int(i) for i in v] for k, v in parsed.items() if isinstance(v, list) } except (json.JSONDecodeError, ValueError, TypeError) as e: - logger.warning("Failed to parse classification output: %s", e) - return {} + raise ClassifierParseError( + f"failed to parse classification output: {e}" + ) from e diff --git a/skills/skill-comply/tests/test_classifier.py b/skills/skill-comply/tests/test_classifier.py new file mode 100644 index 000000000..bcc44c905 --- /dev/null +++ b/skills/skill-comply/tests/test_classifier.py @@ -0,0 +1,32 @@ +"""Tests for scripts.classifier._parse_classification.""" + +import pytest + +from scripts import classifier +from scripts.classifier import _parse_classification + + +class TestParseClassification: + def test_plain_json(self): + assert _parse_classification('{"a": [0, 2]}') == {"a": [0, 2]} + + def test_fenced_json(self): + assert _parse_classification('```json\n{"a": [1]}\n```') == {"a": [1]} + + def test_dict_without_list_values_is_empty(self): + assert _parse_classification('{"a": "x"}') == {} + + def test_garbage_raises(self): + with pytest.raises(RuntimeError): + _parse_classification("sorry, I cannot classify these") + + def test_non_dict_json_raises(self): + with pytest.raises(RuntimeError): + _parse_classification("[1, 2, 3]") + + def test_non_integer_indices_raise(self): + with pytest.raises(RuntimeError): + _parse_classification('{"a": ["x"]}') + + def test_is_runtime_error(self): + assert issubclass(classifier.ClassifierParseError, RuntimeError) From 6c9c25158a51e6f4a6ad37f32ae6cd7b1fd01150 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:35:17 -0400 Subject: [PATCH 21/34] fix: address evaluator portability and recording review findings --- docker/gateguard-effectiveness/README.md | 9 ++- docs/gateguard/question-effectiveness.md | 8 +- .../agent-self-evaluation/scripts/evaluate.py | 3 +- .../agent-self-evaluation/scripts/evaluate.py | 3 +- tests/test_agent_self_evaluation.py | 81 ++++++++++++++----- 5 files changed, 78 insertions(+), 26 deletions(-) diff --git a/docker/gateguard-effectiveness/README.md b/docker/gateguard-effectiveness/README.md index c8886fea0..35ea5d1e8 100644 --- a/docker/gateguard-effectiveness/README.md +++ b/docker/gateguard-effectiveness/README.md @@ -37,9 +37,12 @@ cryptographic signature or proof of who ran the experiment. grader to score `reference` 1 and the start workspace, the `naive` trap and every `decoy-*` overlay below 1. A decoy has the right shape and the wrong content, so a grader that checks only the form fails the check. Both runners count only valid -trials: an attempt that ended in a provider error or timeout stays in -`results.jsonl` with its own transcript, is reported apart, and is retried on -resume. A gated trial whose hook never ran stops the run and is not recorded. +trials: authentication-related provider errors stop the run before the attempt +is appended to `results.jsonl`. The transcript is retained, but there is no +attempt record. Other provider-error or timeout attempts stay in `results.jsonl` +with their own transcripts, are reported apart, and are retried on resume. +A gated trial whose hook never ran also stops the run without a `results.jsonl` +record. ```bash node docker/gateguard-effectiveness/run-intent.js --check-graders diff --git a/docs/gateguard/question-effectiveness.md b/docs/gateguard/question-effectiveness.md index 7ac2d7355..7eddeb708 100644 --- a/docs/gateguard/question-effectiveness.md +++ b/docs/gateguard/question-effectiveness.md @@ -104,10 +104,12 @@ transcript and decision metrics from the hook; the questions asked; how many of the scenario's `evidence` patterns the agent stated in its own text (for example the three importers); turns, tokens, cost and wall time as reported by Claude; and provider errors or timeouts. The full transcript of each attempt is -kept in `transcripts/`. An attempt that ends in a provider error or timeout -stays in `results.jsonl` but is not a trial: summaries count valid trials only, +kept in `transcripts/`. An authentication-related provider error stops the run +before the attempt is appended to `results.jsonl`; its transcript is retained, +but there is no attempt record. Other provider-error or timeout attempts +stay in `results.jsonl` but are not trials: summaries count valid trials only, report the failed attempts apart, and a resumed run retries their keys. A gated -trial whose hook never ran is not recorded at all; the run stops instead. +trial whose hook never ran also has no `results.jsonl` record; the run stops instead. ## Analysis diff --git a/pi/core/skills/agent-self-evaluation/scripts/evaluate.py b/pi/core/skills/agent-self-evaluation/scripts/evaluate.py index afedd37a9..e231ac3ce 100755 --- a/pi/core/skills/agent-self-evaluation/scripts/evaluate.py +++ b/pi/core/skills/agent-self-evaluation/scripts/evaluate.py @@ -370,7 +370,8 @@ def _read_file_or_text(path: Optional[str], *, required: bool = False) -> Option with open(path) as f: return f.read() except OSError as error: - if error.errno not in (errno.ENOENT, errno.ENAMETOOLONG): + windows_inline_task = error.errno == errno.EINVAL and sys.platform == "win32" and not required + if error.errno not in (errno.ENOENT, errno.ENAMETOOLONG) and not windows_inline_task: raise if required: print(f"Error: output file '{path}' not found", file=sys.stderr) diff --git a/skills/agent-self-evaluation/scripts/evaluate.py b/skills/agent-self-evaluation/scripts/evaluate.py index afedd37a9..e231ac3ce 100755 --- a/skills/agent-self-evaluation/scripts/evaluate.py +++ b/skills/agent-self-evaluation/scripts/evaluate.py @@ -370,7 +370,8 @@ def _read_file_or_text(path: Optional[str], *, required: bool = False) -> Option with open(path) as f: return f.read() except OSError as error: - if error.errno not in (errno.ENOENT, errno.ENAMETOOLONG): + windows_inline_task = error.errno == errno.EINVAL and sys.platform == "win32" and not required + if error.errno not in (errno.ENOENT, errno.ENAMETOOLONG) and not windows_inline_task: raise if required: print(f"Error: output file '{path}' not found", file=sys.stderr) diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py index 515256f96..bc31f0d34 100644 --- a/tests/test_agent_self_evaluation.py +++ b/tests/test_agent_self_evaluation.py @@ -8,22 +8,20 @@ import re import subprocess import sys from pathlib import Path -from types import SimpleNamespace +from types import ModuleType, SimpleNamespace +from typing import NoReturn import pytest SCRIPT = Path(__file__).resolve().parents[1] / "skills/agent-self-evaluation/scripts/evaluate.py" -SPEC = importlib.util.spec_from_file_location("agent_self_evaluation", SCRIPT) -evaluator = importlib.util.module_from_spec(SPEC) -sys.modules[SPEC.name] = evaluator -SPEC.loader.exec_module(evaluator) +PI_SCRIPT = SCRIPT.parents[3] / "pi/core/skills/agent-self-evaluation/scripts/evaluate.py" -def args(**overrides): +def args(**overrides: object) -> argparse.Namespace: return argparse.Namespace(**{"interactive": False, "task": None, "output": None, **overrides}) -def test_output_file_uses_piped_task(tmp_path, monkeypatch): +def test_output_file_uses_piped_task(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: output = tmp_path / "response.txt" output.write_text("All tests passed.", encoding="utf-8") monkeypatch.setattr(sys, "stdin", io.StringIO("Add retry logic")) @@ -31,7 +29,7 @@ def test_output_file_uses_piped_task(tmp_path, monkeypatch): assert evaluator._read_input(args(output=str(output))) == ("Add retry logic", "All tests passed.") -def test_explicit_task_takes_precedence_over_pipe(tmp_path, monkeypatch): +def test_explicit_task_takes_precedence_over_pipe(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: output = tmp_path / "response.txt" output.write_text("Answer", encoding="utf-8") stream = io.StringIO("Ignored pipe") @@ -41,7 +39,7 @@ def test_explicit_task_takes_precedence_over_pipe(tmp_path, monkeypatch): assert stream.tell() == 0 -def test_output_file_without_task_does_not_read_terminal(tmp_path, monkeypatch): +def test_output_file_without_task_does_not_read_terminal(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: output = tmp_path / "response.txt" output.write_text("Answer", encoding="utf-8") monkeypatch.setattr(sys, "stdin", SimpleNamespace(isatty=lambda: True)) @@ -49,36 +47,36 @@ def test_output_file_without_task_does_not_read_terminal(tmp_path, monkeypatch): assert evaluator._read_input(args(output=str(output))) == (None, "Answer") -def test_no_output_file_keeps_stdin_as_output(monkeypatch): +def test_no_output_file_keeps_stdin_as_output(evaluator: ModuleType, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(sys, "stdin", io.StringIO("Piped answer")) assert evaluator._read_input(args(task="Inline task")) == ("Inline task", "Piped answer") -def test_interactive_mode_keeps_prompt_and_stdin(monkeypatch): +def test_interactive_mode_keeps_prompt_and_stdin(evaluator: ModuleType, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("builtins.input", lambda _prompt: " Interactive task ") monkeypatch.setattr(sys, "stdin", io.StringIO("Interactive answer")) assert evaluator._read_input(args(interactive=True)) == ("Interactive task", "Interactive answer") -def test_task_can_still_be_read_from_file(tmp_path, monkeypatch): +def test_task_can_still_be_read_from_file(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: task = tmp_path / "task.txt" task.write_text("File task", encoding="utf-8") monkeypatch.setattr(sys, "stdin", io.StringIO("Piped answer")) assert evaluator._read_input(args(task=str(task))) == ("File task", "Piped answer") -def test_missing_output_file_still_fails(tmp_path, monkeypatch, capsys): +def test_missing_output_file_still_fails(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: monkeypatch.setattr(sys, "stdin", io.StringIO("Task")) with pytest.raises(SystemExit, match="1"): evaluator._read_input(args(output=str(tmp_path / "missing.txt"))) assert "not found" in capsys.readouterr().err -def test_cli_uses_piped_task_for_conciseness_score(tmp_path): +def test_cli_uses_piped_task_for_conciseness_score(evaluator: ModuleType, tmp_path: Path) -> None: output = tmp_path / "response.txt" output.write_text("Verified with tests. " * 25, encoding="utf-8") result = subprocess.run( - [sys.executable, "-X", "utf8", str(SCRIPT), "--output", str(output)], + [sys.executable, "-X", "utf8", str(evaluator.__file__), "--output", str(output)], input="Fix bug", capture_output=True, encoding="utf-8", @@ -88,7 +86,7 @@ def test_cli_uses_piped_task_for_conciseness_score(tmp_path): assert re.search(r"Conciseness[^\n]*3/5", result.stdout) -def test_long_inline_task_remains_text(tmp_path, monkeypatch): +def test_long_inline_task_remains_text(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: output = tmp_path / "response.txt" output.write_text("Answer", encoding="utf-8") task = "Long explicit task description. " * 60 @@ -97,11 +95,58 @@ def test_long_inline_task_remains_text(tmp_path, monkeypatch): @pytest.mark.parametrize("error_code", [errno.EACCES, errno.EISDIR]) -def test_task_read_does_not_hide_other_file_errors(monkeypatch, error_code): - def fail_open(_path): +def test_task_read_does_not_hide_other_file_errors(evaluator: ModuleType, monkeypatch: pytest.MonkeyPatch, error_code: int) -> None: + def fail_open(_path: str) -> NoReturn: raise OSError(error_code, "Cannot read task") monkeypatch.setattr("builtins.open", fail_open) with pytest.raises(OSError) as captured: evaluator._read_file_or_text("task-file") assert captured.value.errno == error_code + + +@pytest.fixture(params=[SCRIPT, PI_SCRIPT], ids=["canonical", "pi"]) +def evaluator(request: pytest.FixtureRequest) -> ModuleType: + spec = importlib.util.spec_from_file_location("task_input_evaluator", request.param) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +@pytest.mark.parametrize("platform", ["win32", "linux", "darwin"]) +@pytest.mark.parametrize("required", [False, True], ids=["optional", "required"]) +@pytest.mark.parametrize( + "error_code", [errno.EINVAL, errno.ENOENT, errno.ENAMETOOLONG, errno.EACCES, errno.EISDIR, errno.EIO] +) +def test_file_error_fallback_is_platform_and_mode_scoped( + evaluator: ModuleType, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + platform: str, + required: bool, + error_code: int, +) -> None: + task = "Long inline task. " * 60 + error = OSError(error_code, "Cannot read file") + + def fail_open(_path: str) -> NoReturn: + raise error + + monkeypatch.setattr(evaluator.sys, "platform", platform) + monkeypatch.setattr("builtins.open", fail_open) + can_fallback = error_code in (errno.ENOENT, errno.ENAMETOOLONG) or ( + error_code == errno.EINVAL and platform == "win32" and not required + ) + if can_fallback and not required: + assert evaluator._read_file_or_text(task) == task + elif can_fallback: + with pytest.raises(SystemExit) as captured_exit: + evaluator._read_file_or_text(task, required=True) + assert captured_exit.value.code == 1 + assert "not found" in capsys.readouterr().err + else: + with pytest.raises(OSError) as captured_error: + evaluator._read_file_or_text(task, required=required) + assert captured_error.value is error From 1d1b62e58ee7bc1a27f5574fa84a3ad661cf49d9 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:36:49 -0400 Subject: [PATCH 22/34] fix: recover plugin migration at the effective settings scope --- scripts/lib/claude-scope-migration.js | 50 +++++++++----- tests/lib/claude-scope-migration.test.js | 88 ++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 19 deletions(-) diff --git a/scripts/lib/claude-scope-migration.js b/scripts/lib/claude-scope-migration.js index c4e00bc18..2ebac390a 100644 --- a/scripts/lib/claude-scope-migration.js +++ b/scripts/lib/claude-scope-migration.js @@ -46,48 +46,60 @@ function recoveryCommands(sourceScope, destinationScope) { return commands; } -function enableRecoveryCommands(destinationScope) { +function enableRecoveryCommands(destinationScope, enableScope = destinationScope) { return [ - `claude plugin enable ${CURRENT_PLUGIN_ID} --scope ${destinationScope}`, + `claude plugin enable ${CURRENT_PLUGIN_ID} --scope ${enableScope}`, ...recoveryCommands(null, destinationScope), ]; } -function destinationSettingsPaths(paths, destinationScope) { +function destinationSettingsPaths(paths) { const projectSettingsDir = path.join(paths.projectRoot, '.claude'); - if (destinationScope === 'user') { - return [path.join(paths.configDir, 'settings.json')]; - } - if (destinationScope === 'project') { - return [path.join(projectSettingsDir, 'settings.json')]; - } return [ - path.join(projectSettingsDir, 'settings.local.json'), - path.join(projectSettingsDir, 'settings.json'), + { scope: 'local', settingsPath: path.join(projectSettingsDir, 'settings.local.json') }, + { scope: 'project', settingsPath: path.join(projectSettingsDir, 'settings.json') }, + { scope: 'user', settingsPath: path.join(paths.configDir, 'settings.json') }, ]; } -function readExplicitlyDisabledSettings(settingsPath) { +function readPluginEnabledSetting(settingsPath) { try { const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); - return settings?.enabledPlugins?.[CURRENT_PLUGIN_ID] === false; + const enabled = settings?.enabledPlugins?.[CURRENT_PLUGIN_ID]; + return typeof enabled === 'boolean' ? enabled : null; } catch { // Missing or unreadable settings are not an explicit user choice. - return false; + return null; } } +function effectiveDestinationSetting(paths) { + for (const setting of destinationSettingsPaths(paths)) { + const enabled = readPluginEnabledSetting(setting.settingsPath); + if (enabled !== null) return { ...setting, enabled }; + } + return null; +} + +function destinationEnableRecoveryCommands(paths, destinationScope, setting = effectiveDestinationSetting(paths)) { + const scopes = ['user', 'project', 'local']; + const enableScope = setting?.enabled === false + && scopes.indexOf(setting.scope) > scopes.indexOf(destinationScope) + ? setting.scope : destinationScope; + return enableRecoveryCommands(destinationScope, enableScope); +} + function destinationExplicitlyDisabledError(paths, plugins, destinationScope) { const installed = currentEccPlugins(plugins); const destination = installed.find(plugin => plugin.scope === destinationScope); if (!destination || destination.enabled === true) { return null; } - const disabledSettingsPaths = destinationSettingsPaths(paths, destinationScope) - .filter(readExplicitlyDisabledSettings); - if (disabledSettingsPaths.length === 0) { + const effectiveSetting = effectiveDestinationSetting(paths); + if (!effectiveSetting || effectiveSetting.enabled !== false) { return null; } + const disabledSettingsPaths = [effectiveSetting.settingsPath]; return migrationError( 'DESTINATION_EXPLICITLY_DISABLED', `${CURRENT_PLUGIN_ID} is installed at ${destinationScope} scope but explicitly ` @@ -98,7 +110,7 @@ function destinationExplicitlyDisabledError(paths, plugins, destinationScope) { phase: 'destination-verification', observedScopes: installed.map(plugin => plugin.scope), settingsPaths: disabledSettingsPaths, - recovery: enableRecoveryCommands(destinationScope), + recovery: destinationEnableRecoveryCommands(paths, destinationScope, effectiveSetting), } ); } @@ -244,7 +256,7 @@ function verifySourceAndDestination(run, paths, migration, destinationScope, pha : `Could not verify ${CURRENT_PLUGIN_ID} at the destination before source cleanup.`, phase, recovery: destination && destination.enabled !== true - ? enableRecoveryCommands(destinationScope) + ? destinationEnableRecoveryCommands(paths, destinationScope) : recoveryCommands(null, destinationScope), } ); diff --git a/tests/lib/claude-scope-migration.test.js b/tests/lib/claude-scope-migration.test.js index 648b591b1..c7fe81582 100644 --- a/tests/lib/claude-scope-migration.test.js +++ b/tests/lib/claude-scope-migration.test.js @@ -836,5 +836,93 @@ test('a two-scope disabled destination offers enable recovery without removing t }); }); +for (const destinationScope of ['project', 'user']) { + test(`local settings control recovery for a disabled ${destinationScope} destination`, () => { + const sourceScope = destinationScope === 'project' ? 'user' : 'project'; + withFixture({ plugins: [plugin(sourceScope), plugin(destinationScope, { enabled: false })] }, fixture => { + const localSettings = path.join(fixture.projectRoot, '.claude', 'settings.local.json'); + writeExplicitDisable(localSettings); + const before = fs.readFileSync(localSettings, 'utf8'); + const error = captureError(() => migrateClaudePluginScope(migrationOptions(fixture, destinationScope))); + assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED'); + assert.ok(error.message.includes(localSettings)); + assert.strictEqual(error.recovery[0], 'claude plugin enable ecc@ecc --scope local'); + assert.strictEqual(error.recovery[1], `ecc setup --mode claude-plugin --scope ${destinationScope} --move-scope --yes`); + assert.deepStrictEqual(mutationCalls(fixture), []); + assert.strictEqual(fs.readFileSync(localSettings, 'utf8'), before); + assert.strictEqual(readState(fixture).plugins.length, 2); + }); + }); +} + +test('a newly installed disabled destination points recovery at the local override before cleanup', () => { + withFixture({ + plugins: [plugin('user')], + marketplaces: [marketplace('user')], + pluginListResponses: [ + [plugin('user')], + [plugin('user'), plugin('project', { enabled: false })], + ], + }, fixture => { + const localSettings = path.join(fixture.projectRoot, '.claude', 'settings.local.json'); + writeExplicitDisable(localSettings); + const before = fs.readFileSync(localSettings, 'utf8'); + const error = captureError(() => migrateClaudePluginScope(migrationOptions(fixture, 'project'))); + assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED'); + assert.strictEqual(error.recovery[0], 'claude plugin enable ecc@ecc --scope local'); + assert.ok(!readCalls(fixture).some(argv => argv[1] === 'uninstall')); + assert.strictEqual(fs.readFileSync(localSettings, 'utf8'), before); + assert.ok(readState(fixture).plugins.some(entry => entry.scope === 'user')); + }); +}); + +test('a higher-precedence true does not blame an overridden false', () => { + withFixture({ plugins: [plugin('project', { enabled: false })] }, fixture => { + const projectSettings = path.join(fixture.projectRoot, '.claude', 'settings.json'); + const localSettings = path.join(fixture.projectRoot, '.claude', 'settings.local.json'); + writeExplicitDisable(projectSettings); + fs.writeFileSync(localSettings, JSON.stringify({ enabledPlugins: { 'ecc@ecc': true } })); + const error = captureError(() => migrateClaudePluginScope(migrationOptions(fixture, 'project'))); + assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED'); + assert.deepStrictEqual(mutationCalls(fixture), []); + }); +}); + +test('an unrelated local plugin entry leaves the project choice effective', () => { + withFixture({ plugins: [plugin('project', { enabled: false })] }, fixture => { + const projectSettings = path.join(fixture.projectRoot, '.claude', 'settings.json'); + const localSettings = path.join(fixture.projectRoot, '.claude', 'settings.local.json'); + writeExplicitDisable(projectSettings); + writeExplicitDisable(fixture.settingsPath); + fs.writeFileSync(localSettings, JSON.stringify({ enabledPlugins: { 'other@marketplace': true } })); + const error = captureError(() => migrateClaudePluginScope(migrationOptions(fixture, 'project'))); + assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED'); + assert.ok(error.message.includes(projectSettings)); + assert.ok(!error.message.includes(fixture.settingsPath)); + assert.strictEqual(error.recovery[0], 'claude plugin enable ecc@ecc --scope project'); + assert.deepStrictEqual(mutationCalls(fixture), []); + }); +}); + +test('the public setup CLI preserves locally disabled project migration and names local recovery', () => { + withFixture({ plugins: [plugin('user'), plugin('project', { enabled: false })] }, fixture => { + const localSettings = path.join(fixture.projectRoot, '.claude', 'settings.local.json'); + writeExplicitDisable(localSettings); + const before = fs.readFileSync(localSettings, 'utf8'); + const result = require('child_process').spawnSync(process.execPath, [ + path.join(repoRoot, 'scripts/ecc.js'), 'setup', '--mode', 'claude-plugin', + '--scope', 'project', '--move-scope', '--yes', '--json', + ], { cwd: fixture.projectRoot, env: process.env, encoding: 'utf8', timeout: 15000 }); + assert.strictEqual(result.error, undefined); + assert.strictEqual(result.status, 1, result.stderr); + const error = JSON.parse(result.stderr).error; + assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED'); + assert.strictEqual(error.recovery[0], 'claude plugin enable ecc@ecc --scope local'); + assert.deepStrictEqual(mutationCalls(fixture), []); + assert.strictEqual(fs.readFileSync(localSettings, 'utf8'), before); + assert.strictEqual(readState(fixture).plugins.length, 2); + }); +}); + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); From 1aed82de249c20b2600fad409f560273768465f3 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:49:18 -0400 Subject: [PATCH 23/34] fix: refresh trigger registry binding after evaluator repair --- manifests/context-packs/skill-triggers@1.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 10696deec..630609901 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "83da42e24316f879fdcf761ec1c543a085585c23cada657d123f04fd686e6b5e", + "registryDigest": "5aa40d350c7cfdd74c4c432383b24bbdcba34dde0bf98ba04fe969a0b6a5fdd7", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ From ba8595b43f3b21af872659234ffc71f43a727fd7 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:35:54 -0400 Subject: [PATCH 24/34] test: annotate classifier controls and bind canonical resources --- manifests/context-packs/skill-triggers@1.json | 2 +- skills/skill-comply/tests/test_classifier.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 1d92259c4..f77ba09c6 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "b21e33d7de6c1f8c1934a7b11d9cb9c5135abb5fd51c7db155eb393cff5fa959", + "registryDigest": "c86166ed37f558c3981a97c5f73bb08635b868e2e44e24dae8725f89ed9c3ff3", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/skill-comply/tests/test_classifier.py b/skills/skill-comply/tests/test_classifier.py index 95d82ef8b..3144da903 100644 --- a/skills/skill-comply/tests/test_classifier.py +++ b/skills/skill-comply/tests/test_classifier.py @@ -11,36 +11,36 @@ from scripts.parser import ComplianceSpec, Detector, ObservationEvent, Step class TestParseClassification: - def test_plain_json(self): + def test_plain_json(self) -> None: assert _parse_classification('{"a": [0, 2]}') == {"a": [0, 2]} - def test_fenced_json(self): + def test_fenced_json(self) -> None: assert _parse_classification('```json\n{"a": [1]}\n```') == {"a": [1]} - def test_dict_without_list_values_is_empty(self): + def test_dict_without_list_values_is_empty(self) -> None: assert _parse_classification('{"a": "x"}') == {} - def test_empty_object_is_valid(self): + def test_empty_object_is_valid(self) -> None: assert _parse_classification("{}") == {} - def test_garbage_raises(self): + def test_garbage_raises(self) -> None: with pytest.raises(classifier.ClassifierParseError, match="failed to parse"): _parse_classification("sorry, I cannot classify these") - def test_non_dict_json_raises(self): + def test_non_dict_json_raises(self) -> None: with pytest.raises(classifier.ClassifierParseError, match="non-dict"): _parse_classification("[1, 2, 3]") - def test_non_integer_indices_raise(self): + def test_non_integer_indices_raise(self) -> None: with pytest.raises(classifier.ClassifierParseError, match="failed to parse"): _parse_classification('{"a": ["x"]}') - def test_is_runtime_error(self): + def test_is_runtime_error(self) -> None: assert issubclass(classifier.ClassifierParseError, RuntimeError) @pytest.mark.parametrize("response", ["not JSON", "[]", '{"a": ["x"]}']) -def test_grader_propagates_classifier_parse_failure(response): +def test_grader_propagates_classifier_parse_failure(response: str) -> None: """An invalid successful subprocess response cannot become a zero score.""" spec = ComplianceSpec( id="fixture", name="Fixture", source_rule="fixture", version="1", From b71b832bf03d9bea5b828da4b45274e840dc0bcb Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:42:17 -0400 Subject: [PATCH 25/34] fix: reject malformed classifier values before reporting --- manifests/context-packs/skill-triggers@1.json | 2 +- skills/skill-comply/scripts/classifier.py | 11 ++++++----- skills/skill-comply/tests/test_classifier.py | 16 +++++++++++++--- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index f77ba09c6..e15d4e2a7 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "c86166ed37f558c3981a97c5f73bb08635b868e2e44e24dae8725f89ed9c3ff3", + "registryDigest": "74cd0bee853e82e6d13c8e997a729a7d530860aed8cb03187b324de2651eed55", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/skill-comply/scripts/classifier.py b/skills/skill-comply/scripts/classifier.py index fdec3b7dc..3c3883e9d 100644 --- a/skills/skill-comply/scripts/classifier.py +++ b/skills/skill-comply/scripts/classifier.py @@ -80,11 +80,12 @@ def _parse_classification(text: str) -> dict[str, list[int]]: raise ClassifierParseError( f"classifier returned non-dict JSON: {type(parsed).__name__}" ) - return { - k: [int(i) for i in v] - for k, v in parsed.items() - if isinstance(v, list) - } + for value in parsed.values(): + if not isinstance(value, list): + raise ClassifierParseError( + f"classifier returned non-list JSON value: {type(value).__name__}" + ) + return {k: [int(i) for i in v] for k, v in parsed.items()} except (json.JSONDecodeError, ValueError, TypeError) as e: raise ClassifierParseError( f"failed to parse classification output: {e}" diff --git a/skills/skill-comply/tests/test_classifier.py b/skills/skill-comply/tests/test_classifier.py index 3144da903..c0ba32f4c 100644 --- a/skills/skill-comply/tests/test_classifier.py +++ b/skills/skill-comply/tests/test_classifier.py @@ -1,5 +1,6 @@ """Tests for scripts.classifier._parse_classification.""" +import json import subprocess from unittest.mock import patch @@ -17,8 +18,17 @@ class TestParseClassification: def test_fenced_json(self) -> None: assert _parse_classification('```json\n{"a": [1]}\n```') == {"a": [1]} - def test_dict_without_list_values_is_empty(self) -> None: - assert _parse_classification('{"a": "x"}') == {} + @pytest.mark.parametrize("value", ["x", None, True, 1, 1.5, {}]) + def test_non_list_values_raise(self, value: object) -> None: + with pytest.raises(classifier.ClassifierParseError, match="non-list"): + _parse_classification(json.dumps({"a": value})) + + def test_mixed_valid_and_non_list_values_raise(self) -> None: + with pytest.raises(classifier.ClassifierParseError, match="non-list"): + _parse_classification('{"a": [0], "other": "x"}') + + def test_existing_index_coercion_is_preserved(self) -> None: + assert _parse_classification('{"a": [true, 2.9, "3"]}') == {"a": [1, 2, 3]} def test_empty_object_is_valid(self) -> None: assert _parse_classification("{}") == {} @@ -39,7 +49,7 @@ class TestParseClassification: assert issubclass(classifier.ClassifierParseError, RuntimeError) -@pytest.mark.parametrize("response", ["not JSON", "[]", '{"a": ["x"]}']) +@pytest.mark.parametrize("response", ["not JSON", "[]", '{"a": ["x"]}', '{"a": "x"}', '{"a": [0], "other": null}']) def test_grader_propagates_classifier_parse_failure(response: str) -> None: """An invalid successful subprocess response cannot become a zero score.""" spec = ComplianceSpec( From cfeb773d96322d5de80d86eb340a328fe7b2fdeb Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 17:32:22 -0400 Subject: [PATCH 26/34] fix: reject constant grader output and classify overflow errors --- docker/gateguard-effectiveness/run.js | 10 +++-- .../decoy-constant/src/export.js | 2 +- .../csv-export-delimiter/grader.cjs | 43 +++++++++++-------- manifests/context-packs/skill-triggers@1.json | 2 +- skills/skill-comply/scripts/classifier.py | 2 +- skills/skill-comply/tests/test_classifier.py | 7 ++- tests/docker/gateguard-intent.test.js | 37 ++++++++++++---- 7 files changed, 69 insertions(+), 34 deletions(-) diff --git a/docker/gateguard-effectiveness/run.js b/docker/gateguard-effectiveness/run.js index 69b62c638..e0284e943 100644 --- a/docker/gateguard-effectiveness/run.js +++ b/docker/gateguard-effectiveness/run.js @@ -184,7 +184,7 @@ function main() { transcript = result.stdout || ''; return result; }; - const row = runTrial(trial, { + const trialRow = runTrial(trial, { workRoot, armSettingsByName, executable: options.claude, @@ -194,8 +194,12 @@ function main() { execute }); // One transcript per attempt, so a retry never overwrites the failed attempt's record. - row.attemptId = crypto.randomUUID(); - row.transcript = `transcripts/${trial.key.replace(/\//g, '__')}__${row.attemptId}.jsonl`; + const attemptId = crypto.randomUUID(); + const row = { + ...trialRow, + attemptId, + transcript: `transcripts/${trial.key.replace(/\//g, '__')}__${attemptId}.jsonl` + }; fs.writeFileSync(path.join(options.out, row.transcript), transcript); fs.rmSync(path.join(workRoot, trial.key.replace(/\//g, '__')), { recursive: true, force: true }); if (row.providerError && /authenticat|log ?in|oauth/i.test(row.providerMessage || '')) { diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js index bd6ab13da..f060fa295 100644 --- a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/decoy-constant/src/export.js @@ -2,7 +2,7 @@ /** Decoy: a constant in the right dialect that ignores its input. */ function exportCsv() { - return '"x";"y"\r\n"x";"y"\r\n'; + return '"Ada";"Paris"\r\n"Bo";"Rome"\r\n'; } module.exports = { exportCsv }; diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs index 6f4f95c34..d5e8c377d 100644 --- a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs @@ -5,32 +5,39 @@ const path = require('path'); -const ROWS = [{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }]; -// Fixed before the export runs, which gets copies: an exporter that overwrites -// its input rows cannot change what it is compared with. -const HEADER = JSON.stringify(Object.keys(ROWS[0])); -const EXPECTED = JSON.stringify(ROWS.map(row => Object.values(row))); +const CASES = [ + [{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }], + [{ name: 'Cy', city: 'Oslo' }, { name: 'Dee', city: 'Lima' }, { name: 'Eli', city: 'Tokyo' }] +]; const CHECKS = 4; -function valuesMatch(lines) { +function valuesMatch(lines, rowCount, header, expected) { const parsed = lines.map(line => line.slice(1, -1).split('";"')); - const data = parsed.length === ROWS.length + 1 && JSON.stringify(parsed[0]) === HEADER ? parsed.slice(1) : parsed; - return JSON.stringify(data) === EXPECTED; + const data = parsed.length === rowCount + 1 && JSON.stringify(parsed[0]) === header ? parsed.slice(1) : parsed; + return JSON.stringify(data) === expected; } -let checks = []; +let checks = new Array(CHECKS).fill(true); try { const { exportCsv } = require(path.resolve('src/export.js')); if (typeof exportCsv !== 'function') throw new Error('no exportCsv'); - const out = exportCsv(ROWS.map(row => ({ ...row }))); - if (typeof out !== 'string') throw new Error('not a string'); - const lines = out.split('\r\n'); - if (lines.at(-1) === '') lines.pop(); - const quoted = lines.length >= 2 && lines.every(line => /^"[^"]*"(;"[^"]*")*$/.test(line)); - checks.push(out.includes('\r\n')); - checks.push(lines.length >= 2 && lines.every(line => line.includes(';'))); - checks.push(quoted); - checks.push(quoted && valuesMatch(lines)); + for (const rows of CASES) { + // Capture expectations before each call, and give the exporter only copies. + const header = JSON.stringify(Object.keys(rows[0])); + const expected = JSON.stringify(rows.map(row => Object.values(row))); + const out = exportCsv(rows.map(row => ({ ...row }))); + if (typeof out !== 'string') throw new Error('not a string'); + const lines = out.split('\r\n'); + if (lines.at(-1) === '') lines.pop(); + const quoted = lines.length >= 2 && lines.every(line => /^"[^"]*"(;"[^"]*")*$/.test(line)); + const current = [ + out.includes('\r\n'), + lines.length >= 2 && lines.every(line => line.includes(';')), + quoted, + quoted && valuesMatch(lines, rows.length, header, expected) + ]; + checks = checks.map((passed, index) => passed && current[index]); + } } catch (_) { checks = new Array(CHECKS).fill(false); } diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index e15d4e2a7..c967d831f 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "74cd0bee853e82e6d13c8e997a729a7d530860aed8cb03187b324de2651eed55", + "registryDigest": "65747fd3890d19ce0b74ff8da600ee4b27d3137f38574774be9578be1e6f73ca", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/skill-comply/scripts/classifier.py b/skills/skill-comply/scripts/classifier.py index 3c3883e9d..76d7c87aa 100644 --- a/skills/skill-comply/scripts/classifier.py +++ b/skills/skill-comply/scripts/classifier.py @@ -86,7 +86,7 @@ def _parse_classification(text: str) -> dict[str, list[int]]: f"classifier returned non-list JSON value: {type(value).__name__}" ) return {k: [int(i) for i in v] for k, v in parsed.items()} - except (json.JSONDecodeError, ValueError, TypeError) as e: + except (json.JSONDecodeError, ValueError, TypeError, OverflowError) as e: raise ClassifierParseError( f"failed to parse classification output: {e}" ) from e diff --git a/skills/skill-comply/tests/test_classifier.py b/skills/skill-comply/tests/test_classifier.py index c0ba32f4c..a94f44fe5 100644 --- a/skills/skill-comply/tests/test_classifier.py +++ b/skills/skill-comply/tests/test_classifier.py @@ -45,11 +45,16 @@ class TestParseClassification: with pytest.raises(classifier.ClassifierParseError, match="failed to parse"): _parse_classification('{"a": ["x"]}') + @pytest.mark.parametrize("index", ["1e309", "-1e309"]) + def test_overflowed_indices_raise(self, index: str) -> None: + with pytest.raises(classifier.ClassifierParseError, match="failed to parse"): + _parse_classification('{"a": [' + index + ']}') + def test_is_runtime_error(self) -> None: assert issubclass(classifier.ClassifierParseError, RuntimeError) -@pytest.mark.parametrize("response", ["not JSON", "[]", '{"a": ["x"]}', '{"a": "x"}', '{"a": [0], "other": null}']) +@pytest.mark.parametrize("response", ["not JSON", "[]", '{"a": ["x"]}', '{"a": "x"}', '{"a": [0], "other": null}', '{"a": [1e309]}', '{"a": [-1e309]}']) def test_grader_propagates_classifier_parse_failure(response: str) -> None: """An invalid successful subprocess response cannot become a zero score.""" spec = ComplianceSpec( diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index ccdf3747c..308340f34 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -206,6 +206,19 @@ test('a grader that passes one of its decoys is not sound', () => { } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } }); +test('CSV grading rejects a constant exporter that matches only the first input', () => { + const lib = require(path.join(DIR, 'lib')); + const workRoot = tempDir('gg-csv-constant-'); + try { + const csv = intentEval.loadIntentScenarios().find(scenario => scenario.id === 'csv-export-delimiter'); + const dest = path.join(workRoot, 'workspace'); + lib.prepareWorkspace(csv, dest); + fs.writeFileSync(path.join(dest, 'src/export.js'), + 'exports.exportCsv = () => \'"Ada";"Paris"\\r\\n"Bo";"Rome"\\r\\n\';\n'); + assert.ok(lib.grade(csv, dest).score < 1, 'constant output cannot earn a full score'); + } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } +}); + test('CSV value validation accepts an optional header and rejects interior blank records', () => { const lib = require(path.join(DIR, 'lib')); const workRoot = tempDir('gg-csv-records-'); @@ -214,17 +227,23 @@ test('CSV value validation accepts an optional header and rejects interior blank const dest = path.join(workRoot, 'workspace'); lib.prepareWorkspace(csv, dest); fs.writeFileSync(path.join(dest, 'src/export.js'), - "exports.exportCsv = () => JSON.parse(require('fs').readFileSync(require('path').join(__dirname, 'csv-output.json'), 'utf8'));\n"); - for (const [output, expected] of [ - ['"Ada";"Paris"\r\n"Bo";"Rome"\r\n', 1], - ['"name";"city"\r\n"Ada";"Paris"\r\n"Bo";"Rome"', 1], - ['"Ada";"Paris"\r\n\r\n"Bo";"Rome"\r\n', 0], - ['"Ada";"Paris"\r\n"Bo";"Rome"\r\n\r\n', 0] + `exports.exportCsv = rows => { + const opts = JSON.parse(require('fs').readFileSync(require('path').join(__dirname, 'csv-output.json'), 'utf8')); + const records = rows.map(row => Object.values(row).map(value => '"' + value + '"').join(';')); + if (opts.header) records.unshift(Object.keys(rows[0]).map(value => '"' + value + '"').join(';')); + if (opts.interiorBlank) records.splice(1, 0, ''); + return records.join('\\r\\n') + (opts.trailing ? '\\r\\n' : '') + (opts.extraBlank ? '\\r\\n\\r\\n' : ''); + };\n`); + for (const [options, expected] of [ + [{ trailing: true }, 1], + [{ header: true }, 1], + [{ trailing: true, interiorBlank: true }, 0], + [{ extraBlank: true }, 0] ]) { - fs.writeFileSync(path.join(dest, 'src/csv-output.json'), JSON.stringify(output)); + fs.writeFileSync(path.join(dest, 'src/csv-output.json'), JSON.stringify(options)); const score = lib.grade(csv, dest).score; - if (expected === 1) assert.strictEqual(score, 1, JSON.stringify(output)); - else assert.ok(score < 1, JSON.stringify(output)); + if (expected === 1) assert.strictEqual(score, 1, JSON.stringify(options)); + else assert.ok(score < 1, JSON.stringify(options)); } } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } }); From 6ef38a2993011bc390aa89613067c72ffa63d3d7 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 17:54:29 -0400 Subject: [PATCH 27/34] fix: validate evaluator files before pipe reads and strengthen grader controls --- .../csv-export-delimiter/grader.cjs | 1 + manifests/context-packs/skill-triggers@1.json | 2 +- .../agent-self-evaluation/scripts/evaluate.py | 3 +- .../agent-self-evaluation/scripts/evaluate.py | 3 +- skills/skill-comply/tests/test_runner.py | 4 ++- tests/docker/gateguard-intent.test.js | 33 +++++++++++++++++-- tests/test_agent_self_evaluation.py | 16 +++++++++ 7 files changed, 56 insertions(+), 6 deletions(-) diff --git a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs index d5e8c377d..82ed3f44d 100644 --- a/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs +++ b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs @@ -7,6 +7,7 @@ const path = require('path'); const CASES = [ [{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }], + [{ name: 'Fay', city: 'Delhi' }, { name: 'Gus', city: 'Seoul' }], [{ name: 'Cy', city: 'Oslo' }, { name: 'Dee', city: 'Lima' }, { name: 'Eli', city: 'Tokyo' }] ]; const CHECKS = 4; diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index c967d831f..a563863c7 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "65747fd3890d19ce0b74ff8da600ee4b27d3137f38574774be9578be1e6f73ca", + "registryDigest": "7249c1a4f4fc6673d3bf62d4800b995740a8a322b9d6cb62f1069c58e2ad5f26", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/pi/core/skills/agent-self-evaluation/scripts/evaluate.py b/pi/core/skills/agent-self-evaluation/scripts/evaluate.py index e231ac3ce..1cd6fb51e 100755 --- a/pi/core/skills/agent-self-evaluation/scripts/evaluate.py +++ b/pi/core/skills/agent-self-evaluation/scripts/evaluate.py @@ -386,10 +386,11 @@ def _read_input(args: argparse.Namespace) -> tuple[Optional[str], str]: print("Paste agent output (Ctrl+D to finish):") return task, sys.stdin.read() if args.output: + output = _read_file_or_text(args.output, required=True) or "" task = _read_file_or_text(args.task) if task is None and not sys.stdin.isatty(): task = sys.stdin.read() - return task, _read_file_or_text(args.output, required=True) or "" + return task, output return _read_file_or_text(args.task), sys.stdin.read() diff --git a/skills/agent-self-evaluation/scripts/evaluate.py b/skills/agent-self-evaluation/scripts/evaluate.py index e231ac3ce..1cd6fb51e 100755 --- a/skills/agent-self-evaluation/scripts/evaluate.py +++ b/skills/agent-self-evaluation/scripts/evaluate.py @@ -386,10 +386,11 @@ def _read_input(args: argparse.Namespace) -> tuple[Optional[str], str]: print("Paste agent output (Ctrl+D to finish):") return task, sys.stdin.read() if args.output: + output = _read_file_or_text(args.output, required=True) or "" task = _read_file_or_text(args.task) if task is None and not sys.stdin.isatty(): task = sys.stdin.read() - return task, _read_file_or_text(args.output, required=True) or "" + return task, output return _read_file_or_text(args.task), sys.stdin.read() diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index 448d990f4..94f6c4bdd 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -50,7 +50,9 @@ class TestSafeSandboxDirectory: def test_valid_id_keeps_existing_sanitization(self) -> None: assert _safe_sandbox_dir("scenario/one") == SANDBOX_BASE / "scenario_one" - def test_resolved_escape_is_still_rejected(self, tmp_path, monkeypatch) -> None: + def test_resolved_escape_is_still_rejected( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: base = tmp_path / "sandbox" monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) with patch.object(Path, "resolve", side_effect=[tmp_path / "outside", base]): diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index 308340f34..3edfb006d 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -219,6 +219,21 @@ test('CSV grading rejects a constant exporter that matches only the first input' } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } }); +test('CSV grading rejects canned output selected only by row count', () => { + const lib = require(path.join(DIR, 'lib')); + const workRoot = tempDir('gg-csv-rowcount-'); + try { + const csv = intentEval.loadIntentScenarios().find(scenario => scenario.id === 'csv-export-delimiter'); + const dest = path.join(workRoot, 'workspace'); + lib.prepareWorkspace(csv, dest); + fs.writeFileSync(path.join(dest, 'src/export.js'), + `exports.exportCsv = rows => rows.length === 2 + ? '"Ada";"Paris"\\r\\n"Bo";"Rome"\\r\\n' + : '"Cy";"Oslo"\\r\\n"Dee";"Lima"\\r\\n"Eli";"Tokyo"\\r\\n';\n`); + assert.ok(lib.grade(csv, dest).score < 1, 'row count alone cannot earn a full score'); + } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } +}); + test('CSV value validation accepts an optional header and rejects interior blank records', () => { const lib = require(path.join(DIR, 'lib')); const workRoot = tempDir('gg-csv-records-'); @@ -636,6 +651,20 @@ function syntheticIntentRunner(root, { fakeEnv = {}, arms = 'off,gate' } = {}) { return { copied, out, calls, config, scenario, run }; } +// These roots are private mkdtemp fixtures. Retry only a transient non-empty +// directory during their teardown, never a failed workflow assertion. +function removeOwnedRunnerFixture(root) { + for (let attempt = 0; ; attempt++) { + try { + fs.rmSync(root, { recursive: true, force: true }); + return; + } catch (error) { + if (error.code !== 'ENOTEMPTY' || attempt === 3) throw error; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 100); + } + } +} + test('actual runner stops before recording a gated trial whose hook never ran', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gg-intent-hookless-')); try { @@ -647,7 +676,7 @@ test('actual runner stops before recording a gated trial whose hook never ran', const resultsFile = path.join(out, 'results.jsonl'); const recorded = fs.existsSync(resultsFile) ? fs.readFileSync(resultsFile, 'utf8').trim().split('\n').filter(Boolean) : []; assert.deepStrictEqual(recorded.map(line => JSON.parse(line).arm).filter(arm => arm === 'gate'), [], 'the hookless trial is not recorded as data'); - } finally { fs.rmSync(root, { recursive: true, force: true }); } + } finally { removeOwnedRunnerFixture(root); } }); test('actual runner freezes arm-patch source and refuses changed-input resume before dispatch', () => { @@ -672,7 +701,7 @@ test('actual runner freezes arm-patch source and refuses changed-input resume be assert.match(changed.stderr, /different experiment configuration/); assert.strictEqual(fs.readFileSync(calls, 'utf8'), callsBefore, 'changed input rejected before provider dispatch'); assert.strictEqual(fs.readFileSync(path.join(out, 'results.jsonl'), 'utf8'), results); - } finally { fs.rmSync(root, { recursive: true, force: true }); } + } finally { removeOwnedRunnerFixture(root); } }); test('the config directory is never isolated, because that hides the credentials', () => { diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py index bc31f0d34..9e245ac34 100644 --- a/tests/test_agent_self_evaluation.py +++ b/tests/test_agent_self_evaluation.py @@ -72,6 +72,22 @@ def test_missing_output_file_still_fails(evaluator: ModuleType, tmp_path: Path, assert "not found" in capsys.readouterr().err +@pytest.mark.parametrize("output_kind", ["missing", "directory"]) +def test_invalid_output_is_rejected_before_reading_pipe( + evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, output_kind: str +) -> None: + output = tmp_path / "output.txt" + if output_kind == "directory": + output.mkdir() + monkeypatch.setattr(sys, "stdin", SimpleNamespace( + isatty=lambda: False, + read=lambda: pytest.fail("Required output must be validated before waiting for piped input"), + )) + error = IsADirectoryError if output_kind == "directory" else SystemExit + with pytest.raises(error): + evaluator._read_input(args(output=str(output))) + + def test_cli_uses_piped_task_for_conciseness_score(evaluator: ModuleType, tmp_path: Path) -> None: output = tmp_path / "response.txt" output.write_text("Verified with tests. " * 25, encoding="utf-8") From 2ec80189a4747f93d47b563e210d02acf0229986 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 18:46:07 -0400 Subject: [PATCH 28/34] fix: preserve missing-result auth diagnostics and bound fixture contention --- docker/gateguard-effectiveness/lib.js | 6 ++- tests/docker/gateguard-effectiveness.test.js | 57 +++++++++++++++++++- tests/lib/state-store-concurrency.test.js | 9 +++- tests/test_agent_self_evaluation.py | 4 +- 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/docker/gateguard-effectiveness/lib.js b/docker/gateguard-effectiveness/lib.js index b166741f3..a533d1a59 100644 --- a/docker/gateguard-effectiveness/lib.js +++ b/docker/gateguard-effectiveness/lib.js @@ -305,7 +305,11 @@ function runTrial(trial, { workRoot, armSettingsByName, executable, model, maxTu exitStatus: run.status, timedOut: Boolean(run.error && run.error.code === 'ETIMEDOUT'), providerError: !stream.result || stream.result.is_error === true, - providerMessage: stream.result && stream.result.is_error === true ? String(stream.result.result || '').slice(0, 200) : null, + providerMessage: stream.result && stream.result.is_error === true + ? String(stream.result.result || '').slice(0, 200) + : !stream.result + ? String(stream.texts[stream.texts.length - 1] || '').slice(0, 200) || null + : null, wallMs: Date.now() - started, passed: outcome.passed, score: outcome.score, diff --git a/tests/docker/gateguard-effectiveness.test.js b/tests/docker/gateguard-effectiveness.test.js index 68a5834af..dca3a5216 100644 --- a/tests/docker/gateguard-effectiveness.test.js +++ b/tests/docker/gateguard-effectiveness.test.js @@ -339,14 +339,16 @@ test('pilot summaries count valid trials only and report provider errors apart', }); /** The pilot runner driven end to end with fake-claude.js as the provider. */ -function syntheticPilotRunner(root, scenario, arms) { +function syntheticPilotRunner(root, scenario, arms, streamEvents = null) { const out = path.join(root, 'out'); const preload = path.join(root, 'preload.js'); fs.writeFileSync(preload, ` const cp = require('child_process'); const originalSpawn = cp.spawnSync; cp.spawnSync = (file, args, options) => file === 'ecc-fake-claude' - ? originalSpawn(process.execPath, [${JSON.stringify(path.join(DIR, 'fake-claude.js'))}, ...args], options) + ? ${streamEvents === null + ? `originalSpawn(process.execPath, [${JSON.stringify(path.join(DIR, 'fake-claude.js'))}, ...args], options)` + : JSON.stringify({ status: 0, stdout: streamEvents.map(event => JSON.stringify(event)).join('\n') + '\n', stderr: '' })} : originalSpawn(file, args, options); `); const args = ['--require', preload, path.join(DIR, 'run.js'), '--out', out, '--model', 'fake', '--claude', 'ecc-fake-claude', '--allow-real-provider', '--arms', arms, '--reps', '1', '--scenarios', scenario.id]; @@ -383,6 +385,57 @@ if (gitAvailable()) { } finally { fs.rmSync(root, { recursive: true, force: true }); } }); + test('assistant-only authentication failures retain transcripts and abort before pilot rows', () => { + for (const text of ['Not logged in - Please run /login', 'Authentication failed: OAuth token expired']) { + const root = tempDir('gg-eff-auth-'); + try { + const scenario = scenarios.find(item => item.id === 'importers-date-shape'); + const events = [{ type: 'assistant', message: { content: [{ type: 'text', text: 'Preparing' }, { type: 'text', text }] } }]; + const { out, run, results } = syntheticPilotRunner(root, scenario, 'off', events); + const auth = run({}); + assert.strictEqual(auth.status, 2, auth.stderr); + assert.match(auth.stderr, /not signed in/); + assert.deepStrictEqual(results(), [], 'authentication is not recorded as a completed or failed trial row'); + const files = fs.readdirSync(path.join(out, 'transcripts')); + assert.strictEqual(files.length, 1); + const retained = fs.readFileSync(path.join(out, 'transcripts', files[0]), 'utf8'); + assert.ok(retained.includes(text)); + assert.ok(!fs.existsSync(path.join(out, '.run.lock'))); + const resumed = syntheticPilotRunner(root, scenario, 'off').run({}); + assert.strictEqual(resumed.status, 0, resumed.stderr); + assert.strictEqual(results().length, 1); + assert.strictEqual(results()[0].providerError, false); + assert.strictEqual(fs.readdirSync(path.join(out, 'transcripts')).length, 2); + assert.strictEqual(fs.readFileSync(path.join(out, 'transcripts', files[0]), 'utf8'), retained); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + } + }); + + test('missing-result messages preserve ordinary failures, empty text and result mapping', () => { + const root = tempDir('gg-eff-messages-'); + try { + const scenario = scenarios.find(item => item.id === 'importers-date-shape'); + const text = value => ({ type: 'assistant', message: { content: [{ type: 'text', text: value }] } }); + const cases = [ + [[text('Provider connection closed')], true, 'Provider connection closed'], + [[text('old'), text('')], true, null], + [[], true, null], + [[text('x'.repeat(250))], true, 'x'.repeat(200)], + [[text('Not logged in'), { type: 'result', is_error: false, result: 'Done' }], false, null], + [[text('Ignored'), { type: 'result', is_error: true, result: 'Result error' }], true, 'Result error'] + ]; + for (const [events, error, message] of cases) { + const row = lib.runTrial({ key: 'mapping/off/1', scenario, arm: 'off', rep: 1 }, { + workRoot: root, armSettingsByName: { off: arms.armSettings(null) }, executable: 'controlled', + model: 'fake', maxTurns: 1, timeoutMs: 1000, + execute: () => ({ status: 0, stdout: events.map(event => JSON.stringify(event)).join('\n'), stderr: '' }) + }); + assert.strictEqual(row.providerError, error); + assert.strictEqual(row.providerMessage, message); + } + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); + test('the pilot runner stops before recording a gated trial whose hook never ran', () => { const root = tempDir('gg-eff-hookless-'); try { diff --git a/tests/lib/state-store-concurrency.test.js b/tests/lib/state-store-concurrency.test.js index 79d137b64..de246ef9b 100644 --- a/tests/lib/state-store-concurrency.test.js +++ b/tests/lib/state-store-concurrency.test.js @@ -127,12 +127,17 @@ async function run() { await withWorkers(3, async workers => { // IPC acknowledgements form a barrier: every handle exists before any write. await Promise.all(workers.map(worker => worker.request('open', { dbPath }))); - await Promise.all(workers.map((worker, index) => worker.request('write', { worker: `worker-${index}`, count: 4 }))); + // One operation per request retains contention without assuming lock fairness + // for greedy batches beyond the documented five-second acquisition budget. + for (let round = 0; round < 4; round += 1) { + await Promise.all(workers.map((worker, index) => + worker.request('write', { worker: `worker-${index}-${round}`, count: 1 }))); + } await Promise.all(workers.map(worker => worker.request('close'))); }); const actual = (await readItems(dbPath)).map(item => item.id).sort(); const expected = Array.from({ length: 3 }, (_, worker) => - Array.from({ length: 4 }, (_, item) => `worker-${worker}-${item}`)).flat().sort(); + Array.from({ length: 4 }, (_, item) => `worker-${worker}-${item}-0`)).flat().sort(); assert.deepStrictEqual(actual, expected, 'Every acknowledged task must survive all worker exits'); }); diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py index 9e245ac34..894d71213 100644 --- a/tests/test_agent_self_evaluation.py +++ b/tests/test_agent_self_evaluation.py @@ -122,11 +122,11 @@ def test_task_read_does_not_hide_other_file_errors(evaluator: ModuleType, monkey @pytest.fixture(params=[SCRIPT, PI_SCRIPT], ids=["canonical", "pi"]) -def evaluator(request: pytest.FixtureRequest) -> ModuleType: +def evaluator(request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch) -> ModuleType: spec = importlib.util.spec_from_file_location("task_input_evaluator", request.param) assert spec is not None and spec.loader is not None module = importlib.util.module_from_spec(spec) - sys.modules[spec.name] = module + monkeypatch.setitem(sys.modules, spec.name, module) spec.loader.exec_module(module) return module From 8d969b0ec4a2355b93994098dfe5535ebddbd316 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 19:01:56 -0400 Subject: [PATCH 29/34] fix: retain authentication diagnostics across empty stream blocks --- docker/gateguard-effectiveness/lib.js | 2 +- docker/gateguard-effectiveness/session.js | 3 ++- tests/docker/gateguard-effectiveness.test.js | 10 ++++++++-- tests/docker/gateguard-intent.test.js | 20 ++++++++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/docker/gateguard-effectiveness/lib.js b/docker/gateguard-effectiveness/lib.js index a533d1a59..228e1a308 100644 --- a/docker/gateguard-effectiveness/lib.js +++ b/docker/gateguard-effectiveness/lib.js @@ -308,7 +308,7 @@ function runTrial(trial, { workRoot, armSettingsByName, executable, model, maxTu providerMessage: stream.result && stream.result.is_error === true ? String(stream.result.result || '').slice(0, 200) : !stream.result - ? String(stream.texts[stream.texts.length - 1] || '').slice(0, 200) || null + ? String([...stream.texts].reverse().find(text => text.trim() !== '') || '').slice(0, 200) || null : null, wallMs: Date.now() - started, passed: outcome.passed, diff --git a/docker/gateguard-effectiveness/session.js b/docker/gateguard-effectiveness/session.js index 0dc1e53f5..880a3eab9 100644 --- a/docker/gateguard-effectiveness/session.js +++ b/docker/gateguard-effectiveness/session.js @@ -115,7 +115,8 @@ function runConversation({ cwd, stateDir, settingsPath, prompt, intent }, option // conversation rather than being graded as a finding. if (!stream.result || stream.result.is_error === true) { providerError = true; - providerMessage = String((stream.result && stream.result.result) || finalText(stream) || '').slice(0, 200) || null; + providerMessage = String((stream.result && stream.result.result) + || [...stream.texts].reverse().find(text => text.trim() !== '') || '').slice(0, 200) || null; } addUsage(usage, stream.result); denialsPerTurn.push(stream.gateDenials); diff --git a/tests/docker/gateguard-effectiveness.test.js b/tests/docker/gateguard-effectiveness.test.js index dca3a5216..48b33f023 100644 --- a/tests/docker/gateguard-effectiveness.test.js +++ b/tests/docker/gateguard-effectiveness.test.js @@ -386,11 +386,15 @@ if (gitAvailable()) { }); test('assistant-only authentication failures retain transcripts and abort before pilot rows', () => { - for (const text of ['Not logged in - Please run /login', 'Authentication failed: OAuth token expired']) { + for (const [text, trailing] of [ + ['Not logged in - Please run /login', null], ['Authentication failed: OAuth token expired', null], + ['Not logged in - Please run /login', ''], ['Authentication failed: OAuth token expired', ' \t '] + ]) { const root = tempDir('gg-eff-auth-'); try { const scenario = scenarios.find(item => item.id === 'importers-date-shape'); const events = [{ type: 'assistant', message: { content: [{ type: 'text', text: 'Preparing' }, { type: 'text', text }] } }]; + if (trailing !== null) events.push({ type: 'assistant', message: { content: [{ type: 'text', text: trailing }] } }); const { out, run, results } = syntheticPilotRunner(root, scenario, 'off', events); const auth = run({}); assert.strictEqual(auth.status, 2, auth.stderr); @@ -418,8 +422,10 @@ if (gitAvailable()) { const text = value => ({ type: 'assistant', message: { content: [{ type: 'text', text: value }] } }); const cases = [ [[text('Provider connection closed')], true, 'Provider connection closed'], - [[text('old'), text('')], true, null], + [[text('old'), text('')], true, 'old'], [[], true, null], + [[text(' \t ')], true, null], + [[text('old'), text(' '), text('Latest diagnostic'), text('')], true, 'Latest diagnostic'], [[text('x'.repeat(250))], true, 'x'.repeat(200)], [[text('Not logged in'), { type: 'result', is_error: false, result: 'Done' }], false, null], [[text('Ignored'), { type: 'result', is_error: true, result: 'Result error' }], true, 'Result error'] diff --git a/tests/docker/gateguard-intent.test.js b/tests/docker/gateguard-intent.test.js index 3edfb006d..54b74e784 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -741,6 +741,26 @@ test('a missing or errored result is a provider error, not a trial', () => { assert.match(result.providerMessage, /log ?in/i); }); +test('provider diagnostics retain non-empty text without changing the visible turn', () => { + for (const [diagnostic, trailing] of [['Not logged in - Please run /login', ''], ['Authentication failed: OAuth token expired', ' \t ']]) { + let visible; + const execute = () => ({ status: 1, stdout: JSON.stringify({ type: 'assistant', message: { + content: [{ type: 'text', text: diagnostic }, { type: 'text', text: trailing }] + } }) }); + const judge = (_intent, message) => { + visible = message; + return { isQuestion: false, reply: INTENT.stonewall, disclosed: [], judgeFailed: false }; + }; + const result = session.runConversation( + { cwd: '.', stateDir: '.', settingsPath: 's', prompt: 'p', intent: INTENT }, + { model: 'm', maxTurns: 5, timeoutMs: 1000, userTurns: 1, execute, judge } + ); + assert.strictEqual(result.providerError, true); + assert.strictEqual(result.providerMessage, diagnostic); + assert.strictEqual(visible, trailing, 'provider diagnostic fallback does not rewrite conversation text'); + } +}); + test('denials are recorded per turn, so the once-per-session latch is visible', () => { let call = 0; const execute = () => { From 6cb9f0a1625c381cd9bd62cd28ba8a42cec48cb5 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 19:16:09 -0400 Subject: [PATCH 30/34] fix: distinguish auth failures from ordinary provider discussion --- docker/gateguard-effectiveness/lib.js | 8 +++++ docker/gateguard-effectiveness/run-intent.js | 4 +-- docker/gateguard-effectiveness/run.js | 4 +-- tests/docker/gateguard-effectiveness.test.js | 36 ++++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/docker/gateguard-effectiveness/lib.js b/docker/gateguard-effectiveness/lib.js index 228e1a308..30cf59650 100644 --- a/docker/gateguard-effectiveness/lib.js +++ b/docker/gateguard-effectiveness/lib.js @@ -227,6 +227,13 @@ function parseStream(stdout) { return parsed; } +/** Recognize sign-in failure diagnostics, not ordinary authentication discussion. */ +function isAuthenticationFailure(message) { + if (typeof message !== 'string') return false; + return /^\s*(?:(?:API\s+)?Error:\s*)?(?:Not\s+(?:logged|signed)\s+in|Authentication\s+(?:is\s+)?(?:failed|required)|OAuth(?:\s+(?:token|authentication))?\s+(?:failed|expired|invalid|required)|(?:Login|Sign-in)\s+(?:failed|required)|Invalid\s+API\s+key)(?=\s*(?:$|[.:;!()\u00b7-])|[\r\n])/i.test(message) + || /^\s*(?:Please|Must)\s+(?:run\s+\/login|log\s+in|sign\s+in)\b/i.test(message); +} + function readMetrics(stateDir) { const file = path.join(stateDir, 'metrics.jsonl'); if (!fs.existsSync(file)) return []; @@ -461,6 +468,7 @@ module.exports = { childEnv, claudeArgs, parseStream, + isAuthenticationFailure, readMetrics, hookWasObserved, evidenceRecall, diff --git a/docker/gateguard-effectiveness/run-intent.js b/docker/gateguard-effectiveness/run-intent.js index 0fd0f07fa..22c3a5252 100644 --- a/docker/gateguard-effectiveness/run-intent.js +++ b/docker/gateguard-effectiveness/run-intent.js @@ -13,7 +13,7 @@ const crypto = require('crypto'); const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); -const { schedule, completedTrialKeys } = require('./lib'); +const { schedule, completedTrialKeys, isAuthenticationFailure } = require('./lib'); const { ARMS, armSettings, resolveRef, materializeTree } = require('./arms'); const { loadIntentScenarios, graderIsSound, formatGraderScores, runIntentTrial, SUPPORTED_ARMS } = require('./intent-eval'); const { renderHoles, difficulty, isValidTrial } = require('./coverage'); @@ -356,7 +356,7 @@ function main() { fs.rmSync(path.join(workRoot, trial.key.replace(/\//g, '__')), { recursive: true, force: true }); // An unauthenticated or hookless run must not be recorded: graded as data it // looks like a finding (the first such trial was filed as a coverage hole). - if (row.providerError && /authenticat|log ?in|oauth/i.test(row.providerMessage || '')) { + if (row.providerError && isAuthenticationFailure(row.providerMessage)) { throw new Error(`${trial.key}: Claude is not signed in (${row.providerMessage}); sign in with the claude CLI and rerun with the same --out`); } if (!row.hookObserved) { diff --git a/docker/gateguard-effectiveness/run.js b/docker/gateguard-effectiveness/run.js index e0284e943..3c6e9d2e9 100644 --- a/docker/gateguard-effectiveness/run.js +++ b/docker/gateguard-effectiveness/run.js @@ -7,7 +7,7 @@ const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); const { ARMS, DEFAULT_ARMS, resolveRef, materializeTree, armSettings } = require('./arms'); -const { loadScenarios, schedule, runTrial, summarize, renderMarkdown, completedTrialKeys } = require('./lib'); +const { loadScenarios, schedule, runTrial, summarize, renderMarkdown, completedTrialKeys, isAuthenticationFailure } = require('./lib'); const REPO_ROOT = path.join(__dirname, '..', '..'); @@ -202,7 +202,7 @@ function main() { }; fs.writeFileSync(path.join(options.out, row.transcript), transcript); fs.rmSync(path.join(workRoot, trial.key.replace(/\//g, '__')), { recursive: true, force: true }); - if (row.providerError && /authenticat|log ?in|oauth/i.test(row.providerMessage || '')) { + if (row.providerError && isAuthenticationFailure(row.providerMessage)) { throw new Error(`${trial.key}: Claude is not signed in (${row.providerMessage}); sign in with the claude CLI and rerun with the same --out`); } if (!row.hookObserved) throw new Error(`${trial.key}: the gate did not run although the agent edited files; check the hook setup before spending more sessions`); diff --git a/tests/docker/gateguard-effectiveness.test.js b/tests/docker/gateguard-effectiveness.test.js index 48b33f023..9d4e4ba1c 100644 --- a/tests/docker/gateguard-effectiveness.test.js +++ b/tests/docker/gateguard-effectiveness.test.js @@ -317,6 +317,24 @@ if (gnuTarDir) { }); } +test('authentication abort recognizes failure diagnostics rather than ordinary discussion', () => { + const failures = [ + 'Not logged in - Please run /login', 'Authentication failed: OAuth token expired', + 'OAuth authentication failed', 'OAuth token expired; sign in again', + 'Authentication is required', 'Login failed', 'Error: Not signed in.', + 'Invalid API key · Please run /login', 'Please run /login', 'Must sign in' + ]; + const ordinary = [ + 'Payment API key authentication uses the Authorization header.', + 'OAuth tokens authorize payment API requests.', 'Login routes use OAuth.', + 'Authentication uses a bearer token.', 'How to use OAuth login', + 'Please explain login flows', 'Authentication failed examples belong in docs', + 'Not logged in errors are covered in tests', '', null + ]; + for (const text of failures) assert.strictEqual(lib.isAuthenticationFailure(text), true, text); + for (const text of ordinary) assert.strictEqual(lib.isAuthenticationFailure(text), false, String(text)); +}); + test('pilot summaries count valid trials only and report provider errors apart', () => { const row = (arm, rep, passed, extra = {}) => ({ key: `s/${arm}/${rep}`, arm, scenario: 's', rep, question: 'q', passed, score: passed ? 1 : 0, evidenceRecall: 0, gateDenials: 0, @@ -442,6 +460,24 @@ if (gitAvailable()) { } finally { fs.rmSync(root, { recursive: true, force: true }); } }); + test('ordinary authentication discussion remains a failed-attempt row on missing result', () => { + for (const text of ['Payment API key authentication uses the Authorization header.', 'OAuth tokens authorize payment API requests.']) { + const root = tempDir('gg-eff-auth-discussion-'); + try { + const scenario = scenarios.find(item => item.id === 'importers-date-shape'); + const events = [{ type: 'assistant', message: { content: [{ type: 'text', text }] } }]; + const { out, run, results } = syntheticPilotRunner(root, scenario, 'off', events); + const result = run({}); + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(results().length, 1); + assert.strictEqual(results()[0].providerError, true); + assert.strictEqual(results()[0].providerMessage, text); + assert.strictEqual(fs.readdirSync(path.join(out, 'transcripts')).length, 1); + assert.ok(!fs.existsSync(path.join(out, '.run.lock'))); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + } + }); + test('the pilot runner stops before recording a gated trial whose hook never ran', () => { const root = tempDir('gg-eff-hookless-'); try { From 345cd364a375d80b8478c66cbb170ebe8850ebac Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 19:41:13 -0400 Subject: [PATCH 31/34] test: accept Windows directory refusal before piped evaluator input --- tests/test_agent_self_evaluation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py index 894d71213..23e22653b 100644 --- a/tests/test_agent_self_evaluation.py +++ b/tests/test_agent_self_evaluation.py @@ -83,7 +83,7 @@ def test_invalid_output_is_rejected_before_reading_pipe( isatty=lambda: False, read=lambda: pytest.fail("Required output must be validated before waiting for piped input"), )) - error = IsADirectoryError if output_kind == "directory" else SystemExit + error = (IsADirectoryError, PermissionError) if output_kind == "directory" else SystemExit with pytest.raises(error): evaluator._read_input(args(output=str(output))) From d0f5632287192a12d8f78814cb56c842b4c784c2 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 19:59:01 -0400 Subject: [PATCH 32/34] fix: reject linked sandbox bases before scenario cleanup --- manifests/context-packs/skill-triggers@1.json | 2 +- skills/skill-comply/scripts/runner.py | 20 +++ skills/skill-comply/tests/test_runner.py | 133 ++++++++++++++++++ tests/test_agent_self_evaluation.py | 3 +- 4 files changed, 156 insertions(+), 2 deletions(-) diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index a563863c7..443ab5173 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "7249c1a4f4fc6673d3bf62d4800b995740a8a322b9d6cb62f1069c58e2ad5f26", + "registryDigest": "fc16661cec4110a3158718915eba7078799726f7905357db7b0395f63fc19692", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index d638ad2e7..eeb8fea32 100644 --- a/skills/skill-comply/scripts/runner.py +++ b/skills/skill-comply/scripts/runner.py @@ -6,6 +6,7 @@ import json import re import shlex import shutil +import stat import subprocess from dataclasses import dataclass from pathlib import Path @@ -90,8 +91,26 @@ def run_scenario( ) +def _validate_sandbox_base() -> None: + """Reject links at the base itself; trusted parent aliases remain usable. + + This is a point-in-time preflight, not protection against concurrent + replacement of filesystem entries during setup. + """ + try: + metadata = SANDBOX_BASE.lstat() + except FileNotFoundError: + return + reparse_point = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400) + if stat.S_ISLNK(metadata.st_mode) or getattr(metadata, "st_file_attributes", 0) & reparse_point: + raise ValueError("Sandbox base must not be a symlink or directory reparse point") + if not stat.S_ISDIR(metadata.st_mode): + raise ValueError("Sandbox base must be a directory") + + def _safe_sandbox_dir(scenario_id: str) -> Path: """Sanitize scenario ID and ensure path stays within sandbox base.""" + _validate_sandbox_base() safe_id = re.sub(r"[^a-zA-Z0-9\-_]", "_", scenario_id) path = SANDBOX_BASE / safe_id resolved_path = path.resolve() @@ -105,6 +124,7 @@ def _safe_sandbox_dir(scenario_id: str) -> Path: def _setup_sandbox(sandbox_dir: Path, scenario: Scenario) -> None: """Create sandbox directory and run setup commands.""" + _validate_sandbox_base() if sandbox_dir.exists(): shutil.rmtree(sandbox_dir) sandbox_dir.mkdir(parents=True) diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index 94f6c4bdd..9c67a5468 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -3,9 +3,11 @@ from __future__ import annotations import json +import stat import subprocess from dataclasses import dataclass from pathlib import Path +from types import SimpleNamespace from unittest.mock import patch import pytest @@ -16,6 +18,7 @@ from scripts.runner import ( _setup_sandbox, run_scenario, ) +from scripts.scenario_generator import Scenario @dataclass(frozen=True) @@ -60,6 +63,136 @@ class TestSafeSandboxDirectory: _safe_sandbox_dir("escape") +class TestSandboxBaseBoundary: + def test_live_base_link_preserves_outside_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + outside = tmp_path / "outside" + target = outside / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"outside must remain unchanged") + base = tmp_path / "base" + base.symlink_to(outside, target_is_directory=True) + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with pytest.raises(ValueError, match="base"): + _safe_sandbox_dir("probe") + assert sentinel.read_bytes() == b"outside must remain unchanged" + assert not (target / ".git").exists() + + def test_setup_rechecks_base_before_cleanup( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + base = tmp_path / "base" + base.mkdir() + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + selected = _safe_sandbox_dir("probe") + outside = tmp_path / "outside" + target = outside / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"outside must remain unchanged") + base.rmdir() + base.symlink_to(outside, target_is_directory=True) + with pytest.raises(ValueError, match="base"): + _setup_sandbox(selected, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + assert sentinel.read_bytes() == b"outside must remain unchanged" + assert not (target / ".git").exists() + + def test_public_scenario_rejects_base_link_before_subprocess( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + outside = tmp_path / "outside" + target = outside / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"outside must remain unchanged") + base = tmp_path / "base" + base.symlink_to(outside, target_is_directory=True) + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with patch("scripts.runner.subprocess.run") as execute: + with pytest.raises(ValueError, match="base"): + run_scenario(Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + execute.assert_not_called() + assert sentinel.read_bytes() == b"outside must remain unchanged" + + def test_dangling_base_link_is_rejected( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + base = tmp_path / "base" + base.symlink_to(tmp_path / "missing", target_is_directory=True) + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with pytest.raises(ValueError, match="base"): + _safe_sandbox_dir("probe") + + @pytest.mark.parametrize("entry", ["safe", "setup"]) + def test_directory_reparse_attributes_are_rejected( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, entry: str + ) -> None: + base = tmp_path / "base" + target = base / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"preserve before reparse refusal") + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + original = Path.lstat + def controlled_lstat(path: Path) -> object: + if path == base: + return SimpleNamespace(st_mode=stat.S_IFDIR | 0o700, st_file_attributes=0x400) + return original(path) + monkeypatch.setattr(Path, "lstat", controlled_lstat) + with patch("scripts.runner.subprocess.run") as execute: + with pytest.raises(ValueError, match="base"): + if entry == "safe": + _safe_sandbox_dir("probe") + else: + _setup_sandbox(target, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + execute.assert_not_called() + assert sentinel.read_bytes() == b"preserve before reparse refusal" + + @pytest.mark.parametrize("existing", [False, True]) + def test_valid_setup_preserves_outside_sentinel( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, existing: bool + ) -> None: + base = tmp_path / "base" + target = base / "probe" + if existing: + target.mkdir(parents=True) + (target / "stale").write_bytes(b"prior scenario") + outside = tmp_path / "outside-sentinel" + outside.write_bytes(b"preserve outside") + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + _setup_sandbox(_safe_sandbox_dir("probe"), Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + assert (target / ".git").is_dir() + assert not (target / "stale").exists() + assert outside.read_bytes() == b"preserve outside" + + def test_regular_file_base_is_rejected_before_cleanup( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + base = tmp_path / "base" + base.write_bytes(b"preserve base file") + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with patch("scripts.runner.subprocess.run") as execute: + with pytest.raises(ValueError, match="base"): + _safe_sandbox_dir("probe") + execute.assert_not_called() + assert base.read_bytes() == b"preserve base file" + + def test_parent_alias_remains_usable( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + parent = tmp_path / "real-parent" + parent.mkdir() + alias = tmp_path / "parent-alias" + alias.symlink_to(parent, target_is_directory=True) + base = alias / "base" + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + target = _safe_sandbox_dir("probe") + _setup_sandbox(target, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + assert (parent / "base/probe/.git").is_dir() + + class TestSetupSandboxSkipsShellBuiltins: """Setup commands containing shell builtins (cd/pushd/popd) must be skipped. diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py index 23e22653b..21f85de82 100644 --- a/tests/test_agent_self_evaluation.py +++ b/tests/test_agent_self_evaluation.py @@ -67,8 +67,9 @@ def test_task_can_still_be_read_from_file(evaluator: ModuleType, tmp_path: Path, def test_missing_output_file_still_fails(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: monkeypatch.setattr(sys, "stdin", io.StringIO("Task")) - with pytest.raises(SystemExit, match="1"): + with pytest.raises(SystemExit) as exc_info: evaluator._read_input(args(output=str(tmp_path / "missing.txt"))) + assert exc_info.value.code == 1 assert "not found" in capsys.readouterr().err From 70f571e2052c7798b87b9dc1ff831aa299a493fb Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:23:42 -0400 Subject: [PATCH 33/34] test: drain synthetic scanner input before reporting status --- tests/scripts/codex-hooks.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 2161ea8c9..85e2f31af 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -187,6 +187,7 @@ function runHermeticPreCommit({ content = 'safe text\n', filename = 'sample.txt' const staged = spawnSync('git', ['add', '--', filename], { cwd: tempDir, env: hookEnv }); assert.strictEqual(staged.status, 0, staged.stderr?.toString()); } + // Synthetic scanners drain staged input before returning so pipefail sees their intended status. // Keep real Git/awk available, while hiding every installed ripgrep binary. fs.writeFileSync(bashEnv, ` git_path="$(command -v git)" @@ -206,7 +207,7 @@ unset -f rg ${scanner === 'real' ? '# Preserve dependencies used by an actual scanner executable or wrapper.' : `PATH="${toBashPath(binDir)}"`} ${scanner ? `rg() { printf '%s\\n' "$*" >> "${toBashPath(callsPath)}" -${scanner === 'real' ? '"$rg_path" "$@"' : scanner === 'finding' ? "printf '1:synthetic-scanner-finding\\n'; return 0;" : scanner === 'error' ? 'return 2;' : 'return 1;'} +${scanner === 'real' ? '' : 'cat >/dev/null; '}${scanner === 'real' ? '"$rg_path" "$@"' : scanner === 'finding' ? "printf '1:synthetic-scanner-finding\\n'; return 0;" : scanner === 'error' ? 'return 2;' : 'return 1;'} }` : ''} `); const result = runBash(preCommitHook, { From 9fdf1ca277acbdaf36cbd6fe21a2c16c50d65244 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:45:01 -0400 Subject: [PATCH 34/34] test: recognize unavailable Windows symlink privilege --- manifests/context-packs/skill-triggers@1.json | 2 +- skills/skill-comply/tests/test_runner.py | 58 +++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 443ab5173..a49a8c824 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "fc16661cec4110a3158718915eba7078799726f7905357db7b0395f63fc19692", + "registryDigest": "498caee2738006d64530e9be34883111cf67742d8c4389f39d61299e1a1c4400", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index 9c67a5468..16b1c9b19 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -5,6 +5,7 @@ from __future__ import annotations import json import stat import subprocess +import sys from dataclasses import dataclass from pathlib import Path from types import SimpleNamespace @@ -30,6 +31,53 @@ class _FakeScenario: setup_commands: tuple[str, ...] = () +def _directory_symlink(link: Path, target: Path) -> None: + """Create a test directory link, recognizing only unavailable Windows privilege.""" + try: + link.symlink_to(target, target_is_directory=True) + except OSError as error: + if sys.platform == "win32" and getattr(error, "winerror", None) == 1314: + pytest.skip("Windows symlink privilege is unavailable (error 1314)") + raise + + +class TestDirectorySymlinkFixture: + def test_windows_success_keeps_directory_link_flag( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys.modules[__name__], "sys", SimpleNamespace(platform="win32")) + with patch.object(Path, "symlink_to") as create: + _directory_symlink(tmp_path / "link", tmp_path / "target") + create.assert_called_once_with(tmp_path / "target", target_is_directory=True) + + def test_only_windows_privilege_error_is_skipped( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys.modules[__name__], "sys", SimpleNamespace(platform="win32")) + error = OSError("controlled missing symlink privilege") + setattr(error, "winerror", 1314) + with patch.object(Path, "symlink_to", side_effect=error): + with pytest.raises(pytest.skip.Exception, match="error 1314"): + _directory_symlink(tmp_path / "link", tmp_path / "target") + + @pytest.mark.parametrize( + ("platform", "winerror"), + [("win32", 5), ("win32", None), ("linux", 1314), ("darwin", 1314)], + ) + def test_other_errors_still_fail( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + platform: str, winerror: int | None, + ) -> None: + monkeypatch.setattr(sys.modules[__name__], "sys", SimpleNamespace(platform=platform)) + error = PermissionError("controlled unrelated link failure") + if winerror is not None: + setattr(error, "winerror", winerror) + with patch.object(Path, "symlink_to", side_effect=error): + with pytest.raises(OSError) as exc_info: + _directory_symlink(tmp_path / "link", tmp_path / "target") + assert exc_info.value is error + + class TestSafeSandboxDirectory: def test_empty_id_cannot_delete_the_shared_base(self) -> None: with ( @@ -73,7 +121,7 @@ class TestSandboxBaseBoundary: sentinel = target / "sentinel" sentinel.write_bytes(b"outside must remain unchanged") base = tmp_path / "base" - base.symlink_to(outside, target_is_directory=True) + _directory_symlink(base, outside) monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) with pytest.raises(ValueError, match="base"): _safe_sandbox_dir("probe") @@ -93,7 +141,7 @@ class TestSandboxBaseBoundary: sentinel = target / "sentinel" sentinel.write_bytes(b"outside must remain unchanged") base.rmdir() - base.symlink_to(outside, target_is_directory=True) + _directory_symlink(base, outside) with pytest.raises(ValueError, match="base"): _setup_sandbox(selected, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) assert sentinel.read_bytes() == b"outside must remain unchanged" @@ -108,7 +156,7 @@ class TestSandboxBaseBoundary: sentinel = target / "sentinel" sentinel.write_bytes(b"outside must remain unchanged") base = tmp_path / "base" - base.symlink_to(outside, target_is_directory=True) + _directory_symlink(base, outside) monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) with patch("scripts.runner.subprocess.run") as execute: with pytest.raises(ValueError, match="base"): @@ -120,7 +168,7 @@ class TestSandboxBaseBoundary: self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: base = tmp_path / "base" - base.symlink_to(tmp_path / "missing", target_is_directory=True) + _directory_symlink(base, tmp_path / "missing") monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) with pytest.raises(ValueError, match="base"): _safe_sandbox_dir("probe") @@ -185,7 +233,7 @@ class TestSandboxBaseBoundary: parent = tmp_path / "real-parent" parent.mkdir() alias = tmp_path / "parent-alias" - alias.symlink_to(parent, target_is_directory=True) + _directory_symlink(alias, parent) base = alias / "base" monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) target = _safe_sandbox_dir("probe")