diff --git a/docker/gateguard-effectiveness/README.md b/docker/gateguard-effectiveness/README.md index 646446118..35ea5d1e8 100644 --- a/docker/gateguard-effectiveness/README.md +++ b/docker/gateguard-effectiveness/README.md @@ -33,6 +33,17 @@ 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. +`run-intent.js --check-graders`, and each intent 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: 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 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/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/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..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 []; @@ -305,7 +312,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].reverse().find(text => text.trim() !== '') || '').slice(0, 200) || null + : null, wallMs: Date.now() - started, passed: outcome.passed, score: outcome.score, @@ -374,8 +385,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; provider failures and timeouts are counted apart. 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 +413,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 => row.providerError || row.timedOut).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)), @@ -445,12 +468,15 @@ module.exports = { childEnv, claudeArgs, parseStream, + isAuthenticationFailure, readMetrics, hookWasObserved, evidenceRecall, 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..22c3a5252 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, isAuthenticationFailure } = 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 = {}; @@ -360,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 8824b4a5c..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 } = require('./lib'); +const { loadScenarios, schedule, runTrial, summarize, renderMarkdown, completedTrialKeys, isAuthenticationFailure } = 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) { @@ -183,7 +184,7 @@ function main() { transcript = result.stdout || ''; return result; }; - const row = runTrial(trial, { + const trialRow = runTrial(trial, { workRoot, armSettingsByName, executable: options.claude, @@ -192,9 +193,16 @@ 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. + 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 || '')) { + 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/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..f060fa295 --- /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 '"Ada";"Paris"\r\n"Bo";"Rome"\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/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/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs b/docker/gateguard-effectiveness/scenarios-intent/csv-export-delimiter/grader.cjs index c010c214c..82ed3f44d 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,45 @@ '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'); -let checks = []; +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; + +function valuesMatch(lines, rowCount, header, expected) { + const parsed = lines.map(line => line.slice(1, -1).split('";"')); + const data = parsed.length === rowCount + 1 && JSON.stringify(parsed[0]) === header ? parsed.slice(1) : parsed; + return JSON.stringify(data) === expected; +} + +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([{ name: 'Ada', city: 'Paris' }, { name: 'Bo', city: 'Rome' }]); - if (typeof out !== 'string') throw new Error('not a string'); - const lines = out.split('\r\n').filter(Boolean); - 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))); + 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 = [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/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/docs/gateguard/question-effectiveness.md b/docs/gateguard/question-effectiveness.md index 3135ebca5..5f9af94c2 100644 --- a/docs/gateguard/question-effectiveness.md +++ b/docs/gateguard/question-effectiveness.md @@ -103,8 +103,13 @@ 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 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 also has no `results.jsonl` record; the run stops instead. ## Analysis @@ -253,9 +258,15 @@ 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 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. ### Hidden-intent results diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 4ab1c8a55..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": "c6f543a29a359ce7738bf215886b53a5081da2cfc5b6f98d4f9416c58d0dfb36", + "registryDigest": "498caee2738006d64530e9be34883111cf67742d8c4389f39d61299e1a1c4400", "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 2d129c407..1cd6fb51e 100755 --- a/pi/core/skills/agent-self-evaluation/scripts/evaluate.py +++ b/pi/core/skills/agent-self-evaluation/scripts/evaluate.py @@ -19,6 +19,7 @@ For production use, pair with an LLM judge for semantic understanding. """ import argparse +import errno import re import sys from dataclasses import dataclass, field @@ -368,7 +369,10 @@ def _read_file_or_text(path: Optional[str], *, required: bool = False) -> Option try: with open(path) as f: return f.read() - except FileNotFoundError: + except OSError as error: + 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) sys.exit(1) @@ -382,7 +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: - return _read_file_or_text(args.task), _read_file_or_text(args.output, required=True) or "" + 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, output return _read_file_or_text(args.task), sys.stdin.read() diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index 1d19e5c35..99589b299 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -579,13 +579,24 @@ function readSedExecute(script, i, budget) { return { end: j, parts: [], stdinExec: true }; } -// Text, label and filename operands run to end of line; all are data. +// Text and filename operands run to end of line; all are data. function readSedLineOperand(script, i, budget) { let end = i + 1; while (end < script.length && script[end] !== '\n') { budget.spend(); end++; } 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/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/scripts/lib/claude-scope-migration.js b/scripts/lib/claude-scope-migration.js index acb789f3b..2ebac390a 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,75 @@ function recoveryCommands(sourceScope, destinationScope) { return commands; } +function enableRecoveryCommands(destinationScope, enableScope = destinationScope) { + return [ + `claude plugin enable ${CURRENT_PLUGIN_ID} --scope ${enableScope}`, + ...recoveryCommands(null, destinationScope), + ]; +} + +function destinationSettingsPaths(paths) { + const projectSettingsDir = path.join(paths.projectRoot, '.claude'); + return [ + { 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 readPluginEnabledSetting(settingsPath) { + try { + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + 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 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 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 ` + + `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: destinationEnableRecoveryCommands(paths, destinationScope, effectiveSetting), + } + ); +} + function readPluginInventory(run, projectRoot, phase) { return parsePluginList( run( @@ -95,7 +165,7 @@ function assertMigrationInventory(plugins, destinationScope) { { phase: 'destination-verification', observedScopes, - recovery: recoveryCommands(null, destinationScope), + recovery: enableRecoveryCommands(destinationScope), } ); } @@ -171,8 +241,10 @@ function plannedActions(migration, destinationScope, marketplaceAction) { function verifySourceAndDestination(run, paths, migration, destinationScope, phase) { const expectedScopes = [migration.sourceScope, destinationScope]; + const plugins = readPluginInventory(run, paths.projectRoot, phase); + const destination = currentEccPlugins(plugins).find(plugin => plugin.scope === destinationScope); return validateExpectedScopes( - readPluginInventory(run, paths.projectRoot, phase), + plugins, expectedScopes, { code: phase === 'concurrency-check' @@ -183,7 +255,9 @@ function verifySourceAndDestination(run, paths, migration, destinationScope, pha ? 'Claude plugin scopes changed during migration; the source was not removed.' : `Could not verify ${CURRENT_PLUGIN_ID} at the destination before source cleanup.`, phase, - recovery: recoveryCommands(null, destinationScope), + recovery: destination && destination.enabled !== true + ? destinationEnableRecoveryCommands(paths, destinationScope) + : recoveryCommands(null, destinationScope), } ); } @@ -208,23 +282,32 @@ 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', + ...(observedScopes.length === 0 + ? [`ecc setup --mode claude-plugin --scope ${destinationScope} --yes`] + : recoveryCommands(sourcePresent ? migration.sourceScope : null, destinationScope)), + ], } ); } @@ -265,6 +348,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/scripts/lib/context-profiles.js b/scripts/lib/context-profiles.js index de80d1142..31b0a2266 100644 --- a/scripts/lib/context-profiles.js +++ b/scripts/lib/context-profiles.js @@ -86,13 +86,28 @@ 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 = {}) { + return compileContextProfileWithRegistry(options).plan; +} + +// Keep registry reuse inside this call to the canonical loader. Callers can +// consume the snapshot, but cannot supply a cached or caller-built registry. +function compileContextProfileWithRegistry({ repoRoot = DEFAULT_REPO_ROOT, ...options } = {}) { + validatePlanOptions(options); + const registry = loadContextRegistry({ repoRoot }); + return { registry, plan: compileContextPlan(registry, { repoRoot, ...options }) }; +} + +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 +145,4 @@ function compileContextProfile({ return plan; } -module.exports = { compileContextProfile, explainContextEntry, loadContextProfile }; +module.exports = { compileContextProfile, compileContextProfileWithRegistry, explainContextEntry, loadContextProfile }; diff --git a/scripts/lib/context-selection.js b/scripts/lib/context-selection.js index 5a642d455..f732543ad 100644 --- a/scripts/lib/context-selection.js +++ b/scripts/lib/context-selection.js @@ -1,8 +1,8 @@ 'use strict'; const yaml = require('js-yaml'); -const { loadContextRegistry, loadSkillTriggers } = require('./context-pack-registry'); -const { compileContextProfile } = require('./context-profiles'); +const { loadSkillTriggers } = require('./context-pack-registry'); +const { compileContextProfileWithRegistry } = require('./context-profiles'); const { buildRetrievalIndex, searchRetrieval } = require('./context-retrieval'); const { DEFAULT_REPO_ROOT, createSourceReader, digestObject } = require('./context-profile-support'); @@ -310,10 +310,9 @@ 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 }); - const registry = loadContextRegistry({ repoRoot }); + // One registry load serves the plan and the routing entries. + const { registry, plan } = compileContextProfileWithRegistry({ 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 +334,10 @@ 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 }); - const registry = loadContextRegistry({ repoRoot }); + // One registry load serves the plan and the selection; skill bytes are + // still verified against it when they are read. + const { registry, plan } = compileContextProfileWithRegistry({ 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); 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/scripts/lib/skill-evolution/tracker.js b/scripts/lib/skill-evolution/tracker.js index effa107eb..2964028d1 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 failureReasons = [input.failure_reason, input.failureReason]; 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 (failureReasons.some(value => value != null && typeof value !== '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'); } diff --git a/skills/agent-self-evaluation/scripts/evaluate.py b/skills/agent-self-evaluation/scripts/evaluate.py index 2d129c407..1cd6fb51e 100755 --- a/skills/agent-self-evaluation/scripts/evaluate.py +++ b/skills/agent-self-evaluation/scripts/evaluate.py @@ -19,6 +19,7 @@ For production use, pair with an LLM judge for semantic understanding. """ import argparse +import errno import re import sys from dataclasses import dataclass, field @@ -368,7 +369,10 @@ def _read_file_or_text(path: Optional[str], *, required: bool = False) -> Option try: with open(path) as f: return f.read() - except FileNotFoundError: + except OSError as error: + 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) sys.exit(1) @@ -382,7 +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: - return _read_file_or_text(args.task), _read_file_or_text(args.output, required=True) or "" + 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, output return _read_file_or_text(args.task), sys.stdin.read() diff --git a/skills/skill-comply/scripts/classifier.py b/skills/skill-comply/scripts/classifier.py index 2e4207c82..76d7c87aa 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,16 @@ 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 {} - 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"classifier returned non-dict JSON: {type(parsed).__name__}" + ) + 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, OverflowError) as e: + raise ClassifierParseError( + f"failed to parse classification output: {e}" + ) from e diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index ffad8447f..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,17 +91,40 @@ 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() + 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 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_classifier.py b/skills/skill-comply/tests/test_classifier.py new file mode 100644 index 000000000..a94f44fe5 --- /dev/null +++ b/skills/skill-comply/tests/test_classifier.py @@ -0,0 +1,70 @@ +"""Tests for scripts.classifier._parse_classification.""" + +import json +import subprocess +from unittest.mock import patch + +import pytest +from scripts import classifier +from scripts.classifier import _parse_classification +from scripts.grader import grade +from scripts.parser import ComplianceSpec, Detector, ObservationEvent, Step + + +class TestParseClassification: + def test_plain_json(self) -> None: + assert _parse_classification('{"a": [0, 2]}') == {"a": [0, 2]} + + def test_fenced_json(self) -> None: + assert _parse_classification('```json\n{"a": [1]}\n```') == {"a": [1]} + + @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("{}") == {} + + 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) -> None: + with pytest.raises(classifier.ClassifierParseError, match="non-dict"): + _parse_classification("[1, 2, 3]") + + def test_non_integer_indices_raise(self) -> None: + 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}', '{"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( + id="fixture", name="Fixture", source_rule="fixture", version="1", + steps=(Step("a", "Read a file", True, Detector("Read a file")),), + threshold_promote_to_hook=0.6, + ) + trace = [ObservationEvent("T0000", "tool_complete", "Read", "fixture", "", "")] + result = subprocess.CompletedProcess(["claude"], 0, stdout=response, stderr="") + with patch("scripts.classifier.subprocess.run", return_value=result) as execute: + with pytest.raises(classifier.ClassifierParseError): + grade(spec, trace) + execute.assert_called_once() diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index f8141d184..16b1c9b19 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -3,13 +3,23 @@ from __future__ import annotations import json +import stat import subprocess +import sys from dataclasses import dataclass from pathlib import Path +from types import SimpleNamespace 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, +) +from scripts.scenario_generator import Scenario @dataclass(frozen=True) @@ -21,6 +31,216 @@ 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 ( + 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) -> None: + 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) -> None: + assert _safe_sandbox_dir("scenario/one") == SANDBOX_BASE / "scenario_one" + + 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]): + with pytest.raises(ValueError): + _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" + _directory_symlink(base, outside) + 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() + _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" + 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" + _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"): + 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" + _directory_symlink(base, tmp_path / "missing") + 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" + _directory_symlink(alias, parent) + 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/docker/gateguard-effectiveness.test.js b/tests/docker/gateguard-effectiveness.test.js index 143844b0e..9d4e4ba1c 100644 --- a/tests/docker/gateguard-effectiveness.test.js +++ b/tests/docker/gateguard-effectiveness.test.js @@ -317,7 +317,179 @@ 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, + 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('gate', 4, false, { judgeFailed: 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, 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' + ? ${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]; + 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('assistant-only authentication failures retain transcripts and abort before pilot rows', () => { + 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); + 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, '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'] + ]; + 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('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 { + 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..54b74e784 100644 --- a/tests/docker/gateguard-intent.test.js +++ b/tests/docker/gateguard-intent.test.js @@ -170,13 +170,99 @@ 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)}`); } }); +test('the CSV grader rejects the right dialect around the wrong values', () => { + const lib = require(path.join(DIR, 'lib')); + const workRoot = tempDir('gg-decoy-'); + try { + 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', 'decoy-mutates']); + 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]}`); + } finally { fs.rmSync(workRoot, { recursive: true, force: true }); } + +}); + +test('a grader that passes one of its decoys is not sound', () => { + const workRoot = tempDir('gg-decoy-pass-'); + try { + 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); + } 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 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-'); + 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 => { + 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(options)); + const score = lib.grade(csv, dest).score; + 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 }); } +}); + // --- behaviour is classified into the right hole --- test('classify maps each behaviour to its hole', () => { @@ -515,50 +601,88 @@ 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'); - 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); + 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 }; +} + +// 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); } - 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 }); + } +} + +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 { + // 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 { removeOwnedRunnerFixture(root); } +}); + +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')); @@ -577,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', () => { @@ -617,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 = () => { 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/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js index 56549e721..2cf25a1e1 100644 --- a/tests/hooks/block-no-verify.test.js +++ b/tests/hooks/block-no-verify.test.js @@ -2183,7 +2183,18 @@ 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], + ['e command after a newline label is scanned', "sed ':a\ne 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}`, () => { 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'); diff --git a/tests/lib/claude-scope-migration.test.js b/tests/lib/claude-scope-migration.test.js index c3ab6606f..c7fe81582 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,258 @@ 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 --yes', + ]); + }); +}); + +test('a two-scope disabled destination offers enable recovery without removing the source', () => { + withFixture({ + plugins: [plugin('user'), plugin('project', { enabled: false })], + marketplaces: [marketplace('user')], + }, fixture => { + const error = captureError(() => migrateClaudePluginScope(migrationOptions(fixture, 'project'))); + assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED'); + assert.deepStrictEqual(error.recovery, [ + 'claude plugin enable ecc@ecc --scope project', + 'ecc setup --mode claude-plugin --scope project --move-scope --yes', + ]); + assert.ok(!readCalls(fixture).some(argv => argv[1] === 'uninstall')); + }); +}); + +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); diff --git a/tests/lib/context-selection.test.js b/tests/lib/context-selection.test.js index c04f943d0..53755c62e 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/); @@ -727,3 +747,11 @@ test('singular forms do not create exact directive admission', () => { assert.ok(result.candidates.some(candidate => candidate.id === 'skill:database-migrations')); assert.ok(!result.receipt.autoSelection || !result.receipt.autoSelection.exact); }); + +test('invalid resolution and routing options are rejected before reading a missing registry', () => { + const repoRoot = path.join(__dirname, 'missing-registry'); + const values = { repoRoot, target: 'unknown-host' }; + assert.throws(() => resolveTaskContext({ ...values, task: task() }), /target/i); + assert.throws(() => routingEntries(values), /target/i); + assert.throws(() => routingEntries({ repoRoot, selectionMode: 'invalid' }), /Unknown selection mode/); +}); 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/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 }; 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; diff --git a/tests/lib/skill-evolution.test.js b/tests/lib/skill-evolution.test.js index 1860de247..53ed517b2 100644 --- a/tests/lib/skill-evolution.test.js +++ b/tests/lib/skill-evolution.test.js @@ -445,11 +445,41 @@ 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 + ); + } + for (const snake of ['', null, undefined]) { + assert.strictEqual(tracker.normalizeExecutionRecord({ + ...valid, failure_reason: snake, failureReason: 'Useful detail', + }).failure_reason, 'Useful detail'); + } + assert.strictEqual(tracker.normalizeExecutionRecord({ + ...valid, failure_reason: 'Primary detail', failureReason: 'Other detail', + }).failure_reason, 'Primary detail'); + for (const value of [42, 0, false, {}, []]) { + for (const payload of [ + { ...valid, failure_reason: value, failureReason: 'Useful detail' }, + { ...valid, failure_reason: '', failureReason: value }, + ]) assert.throws(() => tracker.normalizeExecutionRecord(payload), /failure_reason must be a string or 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', () => { 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/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++; diff --git a/tests/lib/windows-test-env.test.js b/tests/lib/windows-test-env.test.js index be888271f..05d0da48f 100644 --- a/tests/lib/windows-test-env.test.js +++ b/tests/lib/windows-test-env.test.js @@ -10,9 +10,23 @@ 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 { withoutShadowingClaude, toRelativeTarPath } = require('./helpers/windows-test-env'); +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'); @@ -55,6 +69,78 @@ 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('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; + 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 }); } 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, { 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-'); 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', ]); diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py new file mode 100644 index 000000000..21f85de82 --- /dev/null +++ b/tests/test_agent_self_evaluation.py @@ -0,0 +1,169 @@ +"""Input-mode regressions for the standalone agent output evaluator.""" + +import argparse +import errno +import importlib.util +import io +import re +import subprocess +import sys +from pathlib import Path +from types import ModuleType, SimpleNamespace +from typing import NoReturn + +import pytest + +SCRIPT = Path(__file__).resolve().parents[1] / "skills/agent-self-evaluation/scripts/evaluate.py" +PI_SCRIPT = SCRIPT.parents[3] / "pi/core/skills/agent-self-evaluation/scripts/evaluate.py" + + +def args(**overrides: object) -> argparse.Namespace: + return argparse.Namespace(**{"interactive": False, "task": None, "output": None, **overrides}) + + +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")) + + assert evaluator._read_input(args(output=str(output))) == ("Add retry logic", "All tests passed.") + + +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") + 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(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)) + + assert evaluator._read_input(args(output=str(output))) == (None, "Answer") + + +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(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(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(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + monkeypatch.setattr(sys, "stdin", io.StringIO("Task")) + 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 + + +@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, PermissionError) 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") + result = subprocess.run( + [sys.executable, "-X", "utf8", str(evaluator.__file__), "--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) + + +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 + monkeypatch.setattr(sys, "stdin", io.StringIO("Ignored pipe")) + assert evaluator._read_input(args(task=task, output=str(output))) == (task, "Answer") + + +@pytest.mark.parametrize("error_code", [errno.EACCES, errno.EISDIR]) +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, 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) + monkeypatch.setitem(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