diff --git a/hosts/workbuddy/transcript.js b/hosts/workbuddy/transcript.js index 4fc0382..104644e 100644 --- a/hosts/workbuddy/transcript.js +++ b/hosts/workbuddy/transcript.js @@ -2,7 +2,7 @@ // Copyright (c) 2026 EvoMap import { CHECK_COMMAND_MAX_CHARS } from '../../lib/turn/record.js'; -import { isVerificationCommand, verificationFailureOf } from '../../lib/verification.js'; +import { isVerificationCommand, verificationExitOf, verificationFailureOf } from '../../lib/verification.js'; // FinalStop carries neither the answer nor the turn's tool calls, so a turn is read off the session transcript: one // JSON record per line, `function_call` / `function_call_result` pairs joined by `callId`, messages with a `role`. @@ -88,14 +88,11 @@ const exitCodeOf = (result) => { return exit ? Number(exit[1]) : null; }; -function shellFailed(result) { - if (result.status !== 'completed') return true; - return verificationFailureOf({ exit: exitCodeOf(result), text: shellOutputOf(result.text) }); -} - function checkOf(call) { - const failed = shellFailed(call.result); - return { command: call.args.command.slice(0, CHECK_COMMAND_MAX_CHARS), exit: exitCodeOf(call.result), failed }; + const command = call.args.command; + const exit = verificationExitOf(command, exitCodeOf(call.result)); + const failed = call.result.status !== 'completed' || verificationFailureOf({ exit, text: shellOutputOf(call.result.text) }); + return { command: command.slice(0, CHECK_COMMAND_MAX_CHARS), exit, failed }; } const isCheck = (call) => SHELL_TOOLS.has(call.name) && call.args.run_in_background !== true && isVerificationCommand(call.args.command); diff --git a/lib/agent/defaults/mcp.js b/lib/agent/defaults/mcp.js index f89ce7c..98bb858 100644 --- a/lib/agent/defaults/mcp.js +++ b/lib/agent/defaults/mcp.js @@ -3,6 +3,7 @@ import { AGENT_REPORTED } from '../../app/turn-reports.js'; import { CHECK_COMMAND_MAX_CHARS } from '../../turn/record.js'; +import { verificationExitOf, verificationFailureOf } from '../../verification.js'; import { EVENTS } from '../events.js'; // An agent without hooks reaches Evolver through MCP tools. The connection is the session; a turn runs from the @@ -58,7 +59,10 @@ export const mcpEvidence = Object.freeze({ turnRecord: (payload) => { const checks = list(payload.checks) .filter((check) => typeof check?.command === 'string' && check.command) - .map((check) => ({ command: check.command.slice(0, CHECK_COMMAND_MAX_CHARS), exit: check.exit, failed: check.exit !== 0 })); + .map(({ command, exit }) => { + const checkExit = verificationExitOf(command, exit) ?? null; + return { command: command.slice(0, CHECK_COMMAND_MAX_CHARS), exit: checkExit, failed: verificationFailureOf({ exit: checkExit }) }; + }); const files = list(payload.files).filter((file) => typeof file === 'string' && file); return { checks, commands: checks.map((check) => check.command), paths: files, edits: files, answer: '' }; }, diff --git a/lib/turn/record.js b/lib/turn/record.js index e146a5b..35d25e6 100644 --- a/lib/turn/record.js +++ b/lib/turn/record.js @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2026 EvoMap -import { isVerificationCommand, verificationFailureOf } from '../verification.js'; +import { isVerificationCommand, verificationExitOf, verificationFailureOf } from '../verification.js'; export const CHECK_COMMAND_MAX_CHARS = 2_000; @@ -11,8 +11,8 @@ export const CHECK_COMMAND_MAX_CHARS = 2_000; * { kind: 'edit', paths, lines?, identifiers? } * { kind: 'tool', command?, paths? } * { kind: 'answer', text } the turn's final answer, for a host that tells it before the turn's end - * An exit code decides a check when the host gives one; the output is read too, since a failing run piped - * through `tail` still exits zero. Without an exit code or a supported passing summary, `failed: null` keeps + * An exit code decides a check when it can be attributed to that check; a failing run piped through `tail` + * can still exit zero. Without an attributable exit or a supported passing summary, `failed: null` keeps * the check's result unknown, and `failed: true` is a run the host saw fail without giving its code (a timeout, an * error step); no exit code is made up for either. A command still running has no verdict: a later note with its `callId` * settles it, and one still pending when the turn ends counts as not run. `label` names a check to the person, and @@ -40,14 +40,15 @@ export function turnRecordOf(notes) { if (note.callId) pending.add(note.callId); continue; } - const failed = checkFailed(note); - record.checks.push({ command: note.command.slice(0, CHECK_COMMAND_MAX_CHARS), exit: note.exit ?? null, failed, ...(note.label && { label: note.label }) }); + const exit = verificationExitOf(note.command, note.exit) ?? null; + const failed = checkFailed(note, exit); + record.checks.push({ command: note.command.slice(0, CHECK_COMMAND_MAX_CHARS), exit, failed, ...(note.label && { label: note.label }) }); } return record; } -function checkFailed(note) { - return note.failed === true || verificationFailureOf({ exit: note.exit, text: note.output }); +function checkFailed(note, exit = verificationExitOf(note.command, note.exit)) { + return note.failed === true || verificationFailureOf({ exit, text: note.output }); } /** Whether a tool note is a finished test, build, lint or type-check that failed. */ diff --git a/lib/verification.js b/lib/verification.js index ecd59cf..63792df 100644 --- a/lib/verification.js +++ b/lib/verification.js @@ -7,15 +7,15 @@ import { detectSignals } from './signals.js'; // verdict an automatic reuse report carries is read off the final test, build, // lint or type-check command, never off how the turn ended. const VERIFICATION_COMMANDS = [ - /\b(?:npm|pnpm|yarn|bun)\s+(?:run\s+)?(?:test|build|lint|typecheck|type-check|check)\b/, - /\bnode\s+--test\b/, - /\b(?:jest|vitest|mocha|pytest|rspec|phpunit|tsc|eslint|ruff|mypy)\b/, - /\bgo\s+(?:test|build|vet)\b/, - /\bcargo\s+(?:test|build|check|clippy)\b/, - /\bdotnet\s+(?:test|build)\b/, - /\b(?:mvn|gradle|gradlew)\b/, - /\bmake\b/, - /\bclaude\s+plugin\s+(?:validate|test)\b/, + /^(?:npm|pnpm|yarn|bun)\s+(?:run\s+)?(?:test|build|lint|typecheck|type-check|check)\b/, + /^node\s+--test\b/, + /^(?:jest|vitest|mocha|pytest|rspec|phpunit|tsc|eslint|ruff|mypy)(?:\s|$)/, + /^go\s+(?:test|build|vet)\b/, + /^cargo\s+(?:test|build|check|clippy)\b/, + /^dotnet\s+(?:test|build)\b/, + /^(?:mvn|gradle|gradlew)(?:\s|$)/, + /^make(?:\s|$)/, + /^claude\s+plugin\s+(?:validate|test)\b/, ]; const FAILURE_SIGNALS = new Set(['test_failure', 'log_error']); @@ -31,8 +31,99 @@ const HEREDOC = /<<-?\s*(['"]?)(\w+)\1([^\n]*)\n[\s\S]*?\n\s*\2[ \t]*(?=\n|$)/g; const withoutHeredocBodies = (command) => command.replace(HEREDOC, '<<$2$3'); +// Keep separators inside strings, backticks and comments out of the command scan. A name in an argument is no +// check. This recognises direct commands and known wrappers; it does not evaluate arbitrary shell control flow. +// An escaped separator stays in the shell word and makes an immediately following hash literal. +const SHELL_TOKEN = /\\(?:[ \t;&|()]#|[\s\S])|\x60[^\x60]*(?:\x60|$)|'(?:''|[^'])*(?:'|$)|"(?:\\[\s\S]|\x60[\s\S]|[^"\\\x60])*(?:"|$)|(? path.replaceAll('\\', '/')); + let depth = 0; + let start = 0; + let output = ''; + for (const { 0: token, index } of input.matchAll(SHELL_TOKEN)) { + if (depth === 0) { + if (token === '(' || token === ')') continue; + const quoted = token[0] === '"' || token[0] === "'"; + const masked = quoted ? '\0' + (quotedTokens.push(shell.slice(index, index + token.length)) - 1) + '\0' : '\0'; + output += input.slice(start, index) + masked; + start = index + token.length; + } + if (token === '$(' || (depth > 0 && token === '(')) depth += 1; + if (depth > 0 && token === ')') { + depth -= 1; + if (depth === 0) start = index + 1; + } + } + return depth === 0 ? output + input.slice(start) : output; +} + +const WRAPPERS = [ + /^(?:[A-Za-z_]\w*=\S*\s+)+/, + /^env\s+(?:(?:-i|--ignore-environment)\s+|(?:-u|--unset|-C|--chdir)(?:=\S+|\s+\S+)\s+|[A-Za-z_]\w*=\S*\s+)*(?:--\s+)?/, + /^npx\s+(?:(?:-y|--yes|--no-install)\s+|(?:-p|--package)(?:=\S+|\s+\S+)\s+)*(?:--\s+)?/, + /^npm\s+exec\s+(?:(?:-y|--yes|--no)\s+|--package(?:=\S+|\s+\S+)\s+)*(?:--\s+)?/, + /^pnpm\s+(?:(?:-r|--recursive)\s+)?exec\s+(?:(?:-r|--recursive)\s+)?(?:--\s+)?/, + /^corepack\s+(?=(?:npm|pnpm|yarn)\s)/, + /^command\s+(?:-p\s+)?(?:--\s+)?/, + /^sudo\s+(?:(?:-n|--non-interactive)\s+|(?:-u|--user|-g|--group)(?:=\S+|\s+\S+)\s+)*(?:--\s+)?/, + /^time\s+(?:(?:-p|--portability)\s+)?(?:--\s+)?(?:!\s+)?/, + /^(?:nohup|bunx|exec)\s+(?:--\s+)?/, + /^timeout\s+(?:(?:--foreground|--preserve-status)\s+|(?:-s|--signal)(?:=\S+|\s+\S+)\s+)*(?:--\s+)?\d+(?:\.\d+)?[smhd]?\s+/, + /^(?:uv|poetry)\s+run\s+/, + /^python(?:3(?:\.\d+)?)?\s+-m\s+(?=pytest(?:\s|$))/, + /^yarn\s+(?=(?:jest|vitest)(?:\s|$))/, +]; +const executableName = (path) => path.split('/').at(-1).replace(/\.(?:exe|cmd|bat)$/i, ''); + +function literalExecutable(token) { + const quote = token[0]; + if ((quote !== "'" && quote !== '"') || token.length < 2 || !token.endsWith(quote)) return null; + const path = token.slice(1, -1); + const windows = /^[A-Za-z]:[\\/]/.test(path); + if (quote === '"' && (/[#$%!\x60"]/.test(path) || (!windows && path.includes('\\')))) return null; + const name = executableName(windows ? path.replaceAll('\\', '/') : path); + return /^[\w.-]+$/.test(name) ? name : null; +} + +function withoutExecutablePath(command, quotedTokens) { + return command.replace(/^\S+/, (token) => { + const quoted = /^\0(\d+)\0$/.exec(token); + if (quoted) return literalExecutable(quotedTokens[Number(quoted[1])]) ?? token; + return token.includes('\0') ? token : executableName(token); + }); +} + +function executableCommandOf(part, quotedTokens) { + let check = part.trim().replace(/^(?:(?:if|then|elif|else|do|!)\s+|[({]\s*)+/, ''); + while (check) { + // An assignment may contain slashes but is still a wrapper before the executable. + if (!/^[A-Za-z_]\w*=/.test(check)) check = withoutExecutablePath(check, quotedTokens); + const wrapper = WRAPPERS.find((pattern) => pattern.test(check)); + if (!wrapper) break; + check = check.replace(wrapper, ''); + } + return check; +} + +// Only unwrap a whole literal argument. Double-quoted expansions belong to the outer shell. +function literalScriptOf(check, quotedTokens) { + const argument = /^(?:bash|sh)[ \t]+-l?c[ \t]+\0(\d+)\0$/.exec(check); + if (!argument) return null; + const script = /^(?:'([^']*)'|"([^"\x60$\\]*)")$/.exec(quotedTokens[Number(argument[1])]); + return script ? script[1] ?? script[2] : null; +} + export function isVerificationCommand(command) { - return typeof command === 'string' && VERIFICATION_COMMANDS.some((pattern) => pattern.test(withoutHeredocBodies(command))); + if (typeof command !== 'string' || command.includes('\0')) return false; + const quotedTokens = []; + return withoutShellData(command, quotedTokens).split(/[;&|\r\n]+/).some((part) => { + const check = executableCommandOf(part, quotedTokens); + const script = literalScriptOf(check, quotedTokens); + return script === null ? VERIFICATION_COMMANDS.some((pattern) => pattern.test(check)) : isVerificationCommand(script); + }); } /** @param {{ isError?: boolean, text?: string }} result */ @@ -48,10 +139,31 @@ const PASSED_COUNT = /^\s*(?:#|ℹ)\s+pass\s+[1-9]\d*\b/m; const ZERO_FAILED_COUNT = /^\s*(?:#|ℹ)\s+fail\s+0\b/m; const FAILED_COUNT = /^\s*(?:#|ℹ)\s+(?:fail|cancelled)\s+[1-9]\d*\b/m; +const SHELL_CONTROL = /[;&|()\r\n]|(?:^|[ \t])!(?:[ \t]|$)|^(?:if|then|elif|else|do|\{)(?:\s|$)/; + +/** A host's numeric exit belongs to a check only when shell control flow cannot hide the result. */ +export function verificationExitOf(command, exit) { + if (!Number.isInteger(exit) || typeof command !== 'string') return exit; + if (command.includes('\0')) return null; + const quotedTokens = []; + const shell = withoutShellData(command, quotedTokens).replace(/[<>]&|&>|>\|/g, ' ').trim(); + let direct = shell; + if (SHELL_CONTROL.test(shell)) { + // A successful directory change followed by one check implies the check ran and passed. + const [before, after, extra] = shell.split('&&', 3); + if (exit !== 0 || !after || extra !== undefined || !/^cd\s+\S/.test(before) + || SHELL_CONTROL.test(before) || SHELL_CONTROL.test(after.trim())) return null; + direct = after; + } + const script = literalScriptOf(executableCommandOf(direct, quotedTokens), quotedTokens); + return script === null ? exit : verificationExitOf(script, exit); +} + /** A finished check's failed flag: true = failed, false = passed, null = unknown. */ -export function verificationFailureOf({ exit, text }) { +export function verificationFailureOf({ command, exit, text }) { const output = String(text ?? ''); - if (verificationFailed({ isError: Number.isInteger(exit) && exit !== 0, text: output }) || FAILED_COUNT.test(output)) return true; - if (exit === 0 || (PASSED_COUNT.test(output) && ZERO_FAILED_COUNT.test(output))) return false; + const checkExit = verificationExitOf(command, exit); + if (verificationFailed({ isError: Number.isInteger(checkExit) && checkExit !== 0, text: output }) || FAILED_COUNT.test(output)) return true; + if (checkExit === 0 || (PASSED_COUNT.test(output) && ZERO_FAILED_COUNT.test(output))) return false; return null; } diff --git a/test/agent/driver.test.js b/test/agent/driver.test.js index 7602888..2d635ff 100644 --- a/test/agent/driver.test.js +++ b/test/agent/driver.test.js @@ -107,6 +107,15 @@ test('the MCP profile turns tool calls into a turn whose report is agent-reporte assert.match(status.content[0].text, /verified turn waiting to be distilled/); }); +test('a successful compound command reported through MCP does not open a distillation candidate', async () => { + const made = memoryPorts(); + const { driver } = driverOf(mcpGeneric, made); + await driver.handle('evolver_recall', { session_id: 'm1', turn_id: 't1', task: PROMPT }); + const reported = await driver.handle('evolver_report', { session_id: 'm1', turn_id: 't1', outcome: 'success', summary: 'Fixed the retries.', checks: [{ command: 'true || npm test', exit: 0 }], files: ['lib/redis.js'] }); + assert.equal(reported.content[0].text, 'Reported.'); + assert.equal((await made.ports.candidates.read()).m1, undefined); +}); + test('a session start answers the engine notice once, and a session end schedules distillation', async () => { const made = memoryPorts(); made.ports.engine.takeUnannounced = async () => ({ status: 'install_failed', error: 'npm missing', at: 1 }); diff --git a/test/app/turn-lifecycle.test.js b/test/app/turn-lifecycle.test.js index 5efb798..df5bc88 100644 --- a/test/app/turn-lifecycle.test.js +++ b/test/app/turn-lifecycle.test.js @@ -38,6 +38,33 @@ test('a recalled strategy ends with the spec line, and its turn end reports it w assert.ok(scheduled.includes('drain')); }); +test('printing a check command does not verify a turn or raise its reuse evidence', async () => { + const { ports } = memoryPorts(); + await promptSubmitted(ports, { sessionId: 's1', prompt: PROMPT, ...WAIT }); + await toolFinished(ports, { sessionId: 's1', note: { kind: 'edit', paths: ['/proj/lib/redis.js'] } }); + await toolFinished(ports, { sessionId: 's1', note: { + kind: 'shell', command: 'echo "npm test"', exit: 0, output: 'npm test', label: 'Run tests', + } }); + const { record, reports } = await end(ports, 's1'); + assert.deepEqual(record.checks, []); + assert.equal((await ports.candidates.read()).s1, undefined); + assert.equal(reports[0].evidence.tier, 'none'); + assert.equal(reports[0].evidence.check, undefined); +}); + +test('a successful shell wrapper does not verify a turn when its check may have been skipped', async () => { + const { ports } = memoryPorts(); + const command = 'true || npm test'; + await promptSubmitted(ports, { sessionId: 's1', prompt: PROMPT, ...WAIT }); + await toolFinished(ports, { sessionId: 's1', note: { kind: 'edit', paths: ['/proj/lib/redis.js'] } }); + await toolFinished(ports, { sessionId: 's1', note: { kind: 'shell', command, exit: 0 } }); + const { record, reports } = await end(ports, 's1'); + assert.deepEqual(record.checks, [{ command, exit: null, failed: null }]); + assert.equal((await ports.candidates.read()).s1, undefined); + assert.equal(reports[0].evidence.tier, 'none'); + assert.equal(reports[0].evidence.check, undefined); +}); + test('a verified turn becomes the candidate; a correction drops it before revising the reports', async () => { const { ports, queued } = memoryPorts(); await verifiedTurn(ports, 's1'); diff --git a/test/hosts/antigravity/readers.test.js b/test/hosts/antigravity/readers.test.js index bc05b74..33cdc18 100644 --- a/test/hosts/antigravity/readers.test.js +++ b/test/hosts/antigravity/readers.test.js @@ -34,7 +34,7 @@ test('a turn record pairs each call with its result in step order: commands, che assert.deepEqual(record.checks, [ { command: 'node --test', exit: 1, failed: true }, { command: 'npm run lint', exit: 0, failed: false }, - { command: 'node --test 2>&1 | tail -5', exit: 0, failed: false }, + { command: 'node --test 2>&1 | tail -5', exit: null, failed: false }, ], 'the build moved to the background has no verdict'); assert.deepEqual(record.paths, ['/workspace/project/src/index.js', '/workspace/project/src/lru.js', '/workspace/project/src/index.js']); assert.deepEqual(record.edits, ['/workspace/project/src/lru.js'], 'the failed replacement edited nothing'); @@ -48,7 +48,7 @@ test('an exit code decides a check; without one an ERROR step fails it and the o assert.deepEqual(checksOf(turn(call(1, 'npm test'), row({ step_index: 2, source: 'MODEL', type: 'RUN_COMMAND', status: 'ERROR', error: 'exit status 2' }))), [{ command: 'npm test', exit: 2, failed: true }]); assert.deepEqual(checksOf(turn(call(1, 'npm test'), row({ step_index: 2, source: 'MODEL', type: 'RUN_COMMAND', status: 'ERROR' }))), [{ command: 'npm test', exit: null, failed: true }]); assert.deepEqual(checksOf(turn(call(1, 'npm test'), row({ step_index: 2, source: 'MODEL', type: 'RUN_COMMAND', content: 'done' }))), [{ command: 'npm test', exit: null, failed: null }]); - assert.deepEqual(checksOf(turn(call(1, 'npm test | tail -3'), row({ step_index: 2, source: 'MODEL', type: 'RUN_COMMAND', content: 'The command exited with code 0.\nOutput:\nAssertionError: expected 1 to equal 2' }))), [{ command: 'npm test | tail -3', exit: 0, failed: true }]); + assert.deepEqual(checksOf(turn(call(1, 'npm test | tail -3'), row({ step_index: 2, source: 'MODEL', type: 'RUN_COMMAND', content: 'The command exited with code 0.\nOutput:\nAssertionError: expected 1 to equal 2' }))), [{ command: 'npm test | tail -3', exit: null, failed: true }]); assert.deepEqual(checksOf(turn(call(1, 'npm test'))), [], 'a call no result answered was not run'); }); diff --git a/test/hosts/dsh/plugin.test.js b/test/hosts/dsh/plugin.test.js index 110f7e9..dd6fb71 100644 --- a/test/hosts/dsh/plugin.test.js +++ b/test/hosts/dsh/plugin.test.js @@ -60,7 +60,7 @@ test('a verified turn is reported with its last check and becomes the session\'s const candidate = stateFile(proxy, 'dsh-distill-pending.json')[agent.session.id]; assert.equal(candidate.prompt, PROMPT); assert.deepEqual(candidate.model, { provider: 'deepseek-official', model: 'deepseek-flash' }); - assert.deepEqual(candidate.checks, [{ command: 'npm test 2>&1 | tail -5', exit: 0, failed: false }]); + assert.deepEqual(candidate.checks, [{ command: 'npm test 2>&1 | tail -5', exit: null, failed: false }]); assert.equal(candidate.answer, 'Set maxRetriesPerRequest to null; the suite passes.'); assert.equal(candidate.pid, process.pid); }); diff --git a/test/hosts/grok/readers.test.js b/test/hosts/grok/readers.test.js index a938f87..a4dbb49 100644 --- a/test/hosts/grok/readers.test.js +++ b/test/hosts/grok/readers.test.js @@ -49,15 +49,26 @@ test('a shell result carries its exit code; failure then pass reads as a fixed c assert.deepEqual(record.paths, ['/Users/you/project/README.md']); }); -test('a call grok moves to the background has no verdict until its task\'s output settles it under the same call id', () => { +test('Grok joins background task output by call id and requires check evidence for a compound command', () => { const started = noteOf('PostToolUse.run_terminal_command.background'); assert.deepEqual(started, { kind: 'shell', command: 'sleep 2; npm test', running: true, callId: 'call_3' }); assert.deepEqual(turnRecordOf([started]).checks, [], 'a check still running is no pass'); const output = noteOf('PostToolUse.get_command_or_subagent_output'); assert.deepEqual(output, { kind: 'shell', command: 'sleep 2; npm test', exit: 1, output: '\n> test\n> sh check.sh\n\n1 failing\n', callId: 'call_3' }); - const record = turnRecordOf([noteOf('PostToolUse.run_terminal_command.backgroundForTaskOutput'), output]); - assert.deepEqual(record.checks, [{ command: 'sleep 2; npm test', exit: 1, failed: true }]); + const pending = noteOf('PostToolUse.run_terminal_command.backgroundForTaskOutput'); + const record = turnRecordOf([pending, output]); + assert.deepEqual(record.checks, [{ command: 'sleep 2; npm test', exit: null, failed: null }]); assert.deepEqual(record.commands, ['sleep 2; npm test'], 'its output is the same command, not a second one'); + for (const [stdout, failed] of [ + ['done', null], + ['TAP version 13\n# pass 1\n# fail 0\n', false], + ['TAP version 13\n# pass 0\n# fail 1\n', true], + ]) { + const completed = payload('PostToolUse.get_command_or_subagent_output'); + Object.assign(completed.toolResult.Result, { status: 'completed', exit_code: 0, output: stdout }); + const note = grok.evidence.toolNote(completed); + assert.deepEqual(turnRecordOf([pending, note]).checks, [{ command: 'sleep 2; npm test', exit: null, failed }]); + } const stillRunning = { ...payload('PostToolUse.get_command_or_subagent_output') }; stillRunning.toolResult = { ...stillRunning.toolResult, Result: { ...stillRunning.toolResult.Result, status: 'running', exit_code: null } }; assert.deepEqual(grok.evidence.toolNote(stillRunning), { kind: 'shell', command: 'sleep 2; npm test', running: true, callId: 'call_3' }); diff --git a/test/hosts/opencode/server.test.js b/test/hosts/opencode/server.test.js index b5799b0..9c0d611 100644 --- a/test/hosts/opencode/server.test.js +++ b/test/hosts/opencode/server.test.js @@ -57,7 +57,7 @@ test('a verified turn reports its strategy with the last check and becomes the s const candidate = pending(proxy).ses_ok; assert.equal(candidate.prompt, PROMPT); assert.deepEqual(candidate.model, { providerID: 'anthropic', modelID: 'claude-sonnet' }); - assert.deepEqual(candidate.checks, [{ command: 'npm test 2>&1 | tail -5', exit: 0, failed: false }]); + assert.deepEqual(candidate.checks, [{ command: 'npm test 2>&1 | tail -5', exit: null, failed: false }]); assert.equal(candidate.pid, process.pid); }); diff --git a/test/hosts/opencode/session-end.test.js b/test/hosts/opencode/session-end.test.js index 9ab9347..1f48299 100644 --- a/test/hosts/opencode/session-end.test.js +++ b/test/hosts/opencode/session-end.test.js @@ -42,7 +42,7 @@ test('a verified turn is distilled once its session is deleted, in a tool-less h assert.equal(body.trigger, 'session_end'); assert.equal(body.session_id, 'opencode:ses_d'); assert.equal(body.user_prompt, PROMPT); - assert.deepEqual(body.execution, { trace: [{ command: 'npm test 2>&1 | tail -5', exit: 0, source: 'observed' }] }); + assert.deepEqual(body.execution, { trace: [{ command: 'npm test 2>&1 | tail -5', ok: true, source: 'observed' }] }); assert.deepEqual(body.distill_usage, { input: 900, output: 120 }); assert.equal((await stores.drafts.read()).length, 1); }); diff --git a/test/hosts/workbuddy/readers.test.js b/test/hosts/workbuddy/readers.test.js index 4d413bb..97d8ce1 100644 --- a/test/hosts/workbuddy/readers.test.js +++ b/test/hosts/workbuddy/readers.test.js @@ -68,8 +68,8 @@ test('a WorkBuddy turn yields its commands, paths, edited files and checks from appendFileSync(file, LRU_TURN); const turn = turnOf(file, mark); assert.deepEqual(turn.checks, [ - { command: 'cd /tmp/lru-e2e && node --test 2>&1 | tail -12', exit: 1, failed: true }, - { command: 'cd /tmp/lru-e2e && node --test 2>&1 | tail -9', exit: 0, failed: false }, + { command: 'cd /tmp/lru-e2e && node --test 2>&1 | tail -12', exit: null, failed: true }, + { command: 'cd /tmp/lru-e2e && node --test 2>&1 | tail -9', exit: null, failed: false }, ]); assert.deepEqual(turn.edits, ['/tmp/lru-e2e/lru-cache.test.js', '/tmp/lru-e2e/lru-cache.js']); assert.ok(turn.paths.includes('/tmp/lru-e2e/lru-cache.js')); @@ -86,9 +86,12 @@ test('an edit the host refused is no edited file, and a background run is no che test('a non-zero exit or failure output fails a check; an incomplete call failed; one without a result says nothing', () => { const checksOf = (records) => turnOf(transcriptWith(records.map(line).join('')), null).checks; assert.equal(checksOf(bashRecords('npm run build', shellText('npm run build', 'done', 2)))[0].failed, true); + assert.equal(checksOf(bashRecords('true || npm test', shellText('true || npm test', 'done', 0)))[0].failed, null); + assert.equal(checksOf(bashRecords('false && npm test', shellText('false && npm test', 'done', 1)))[0].failed, null); assert.equal(checksOf(bashRecords('npm test | tail -3', shellText('npm test | tail -3', 'AssertionError: expected 1 to equal 2\n1 test failed', 0)))[0].failed, true); assert.equal(checksOf(bashRecords('npm test -- --grep "test failed"', shellText('npm test', 'ℹ pass 4\nℹ fail 0', 0)))[0].failed, false); assert.equal(checksOf(bashRecords('npm test', 'Interrupted', { status: 'incomplete' }))[0].failed, true); + assert.equal(checksOf(bashRecords('npm test | tail -3', 'Interrupted', { status: 'incomplete' }))[0].failed, true); assert.equal(checksOf(bashRecords('npm test', 'boom\n'))[0].failed, null, 'completed says the tool returned, not that its process exited zero'); assert.deepEqual(checksOf([bashRecords('npm test', '')[0]]), []); assert.equal(checksOf(bashRecords('npm test', shellText('npm test', 'ℹ fail 0', 0)).map((record) => ({ ...record, name: 'PowerShell' }))).length, 1); diff --git a/test/verification.test.js b/test/verification.test.js index a06c60b..4b6cfec 100644 --- a/test/verification.test.js +++ b/test/verification.test.js @@ -1,6 +1,8 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; +import { checkOutcomeOf } from '../lib/checks.js'; +import { isFailedCheck, turnRecordOf } from '../lib/turn/record.js'; import { isVerificationCommand, verificationFailed, verificationFailureOf } from '../lib/verification.js'; test('test, build, lint and type-check runs are verifications; other commands are not', () => { @@ -13,6 +15,120 @@ test('test, build, lint and type-check runs are verifications; other commands ar } }); +test('check names in arguments, quoted data and comments are not executions', () => { + for (const command of [ + 'echo "npm test"', 'echo npm test', 'printf "%s\\n" "make"', 'rg pytest README.md', + 'node -e "console.log(\'npm test\')"', 'git status # npm test', '# npm test\npwd', + 'echo "notes; npm test"', "printf 'notes\nnpm test\n'", 'echo `pwd; npm test`', + 'npx echo npm test', 'timeout --signal TERM 60 echo npm test', 'sudo -n echo npm test', 'npm exec echo "npm test"', + 'echo # note; npm test', 'echo bash -c "npm test"', 'bash -c "echo npm test"', 'echo $(printf x; npm test)', 'echo $(echo $(printf x); npm test)', + ]) { + assert.equal(isVerificationCommand(command), false, command); + } +}); + +test('a hash after an escaped separator stays in the current shell word', () => { + for (const command of [ + String.raw`echo arg\ #literal && npm test`, 'echo arg\\\t#literal; npm test', + String.raw`echo arg\\\ #literal; npm test`, String.raw`echo $(echo arg\ #literal) && npm test`, + String.raw`echo arg\;#literal && npm test`, + ]) assert.equal(isVerificationCommand(command), true, command); + for (const command of [ + String.raw`echo arg\\ # comment; npm test`, String.raw`echo arg\ # comment; npm test`, + 'echo "arg " # "$(; npm test\npwd', + ]) assert.equal(isVerificationCommand(command), false, command); + assert.equal(isVerificationCommand('echo # "$( ignored\nnpm test'), true); +}); + +test('command positions in chains, pipelines, environment assignments and npx wrappers still count', () => { + for (const command of [ + 'sleep 2; node --test', 'npm test 2>&1 | tail -5', 'CI=1 npm test', 'env CI=1 npm test', + 'npx --yes vitest run', 'npx --package=typescript tsc -p .', 'npx --package typescript tsc -p .', + 'echo "notes; npm test"\nnode --test', 'curl http://x/#f && npm test', 'git log --format=%h#%s; npm test', + 'cd $(git rev-parse --show-toplevel) && npm test', 'cd $(echo "$(pwd)") && npm test', + 'bash -c "npm test"', "sh -c 'cd app && npm test'", 'timeout 60 npm test', 'timeout --signal=TERM 60 npm test', 'sudo npm test', 'sudo -n npm test', 'time npm test', 'time -p npm test', 'nohup npm test', 'env -i npm test', + 'yarn vitest', 'bunx vitest', 'python -m pytest', 'uv run pytest', 'poetry run pytest', + '{ npm test; }', 'if true; then npm test; fi', 'C:\\tools\\make.exe', + ]) { + assert.equal(isVerificationCommand(command), true, command); + } +}); + +test('a known executable reached through a path still verifies, while a path in an argument does not', () => { + for (const command of [ + './gradlew test', 'gradlew.bat test', 'npm.cmd test', 'C:\\tools\\pnpm.cmd lint', './node_modules/.bin/tsc --noEmit', '/usr/bin/make', + 'cd app && ./gradlew test', 'CI=1 ./node_modules/.bin/tsc --noEmit', + '/usr/bin/npx --yes ./node_modules/.bin/tsc --noEmit', + ]) { + assert.equal(isVerificationCommand(command), true, command); + } + for (const command of [ + 'echo ./gradlew test', 'cat ./node_modules/.bin/tsc', 'echo /usr/bin/make', + 'echo "notes; ./gradlew test"', '/usr/bin/npx echo ./gradlew test', './scripts/make-not-a-check', + ]) { + assert.equal(isVerificationCommand(command), false, command); + } +}); + +test('supported wrapper options still leave the actual command in command position', () => { + for (const command of [ + 'sudo -u root npm test', 'sudo --user=root -- npm test', 'env -u CI npm test', + 'env --unset=CI --chdir app npm test', 'timeout -s KILL 60 npm test', 'time -p npm test', + 'exec npm test', '! npm test', 'time ! npm test', + ]) assert.equal(isVerificationCommand(command), true, command); + for (const command of [ + 'sudo -u root echo npm test', 'sudo -u npm test', 'env -u npm test', + 'env --chdir app echo npm test', 'exec echo npm test', '! echo npm test', + ]) assert.equal(isVerificationCommand(command), false, command); +}); + +test('literal shell and package-manager wrappers recognise only the command they execute', () => { + for (const command of [ + "/bin/bash -c 'npm test'", 'bash -lc "npm test"', "/usr/bin/bash -lc 'cd app && npm test'", + 'corepack pnpm test', 'corepack yarn vitest run', 'command npm test', 'command -p -- npm test', + 'npm exec -- vitest run', 'npm exec --yes --package typescript -- tsc --noEmit', + 'corepack npm exec --package=vitest -- vitest run', + 'pnpm exec vitest run', 'pnpm exec -- vitest run', 'corepack pnpm exec vitest run', 'pnpm -r exec jest', 'pnpm exec --recursive jest', + ]) assert.equal(isVerificationCommand(command), true, command); + for (const command of [ + "echo /bin/bash -c 'npm test'", '/bin/bash -c "echo npm test"', 'bash -lc "$CHECK"', "bash -lc\n'npm test'", + 'corepack enable pnpm', 'corepack echo npm test', 'command -v npm test', 'command -V npm test', + 'command -- echo npm test', 'command ! npm test', 'npm exec -- echo "vitest run"', 'npm exec --package=vitest -- echo vitest', + "npm exec -c 'echo vitest run'", + 'echo pnpm exec vitest run', 'pnpm exec echo vitest run', 'pnpm exec --help vitest', + ]) assert.equal(isVerificationCommand(command), false, command); +}); + +test('a literal shell script runs in a chained or wrapped command position', () => { + for (const command of [ + "cd app && /bin/bash -c 'npm test'", + 'env CI=1 bash -c "npm test"', + "sudo -u root -- bash -c 'npm test'", + "cd app && env -i CI=1 sudo -n '/usr/bin/bash' -lc 'npm test'", + ]) assert.equal(isVerificationCommand(command), true, command); + for (const command of [ + "cd app && echo bash -c 'npm test'", + "env CI=1 bash -c 'echo npm test'", + "sudo -u bash -c 'npm test'", + 'sudo -n bash -c "$CHECK"', + 'cd app && bash -c "$(echo npm test)"', + "cd app && bash -c 'npm test'$EXTRA", + "echo $(bash -c 'npm test')", + ]) assert.equal(isVerificationCommand(command), false, command); +}); + +test('literal quoted executables count only in command position', () => { + for (const command of [ + '"./node_modules/.bin/jest"', "'./path with spaces/gradlew' build", + 'sudo -u "build user" "./node_modules/.bin/jest"', '"/usr/bin/env" -u CI "./node_modules/.bin/jest"', + '"C:\\Program Files\\nodejs\\npm.cmd" test', + ]) assert.equal(isVerificationCommand(command), true, command); + for (const command of [ + 'echo "./node_modules/.bin/jest"', '"npm test"', '"$runner/jest"', + '"./node_modules/.bin/jest"-other', '"notes; npm test"', + ]) assert.equal(isVerificationCommand(command), false, command); +}); + test('a non-zero exit fails the verification', () => { assert.equal(verificationFailed({ isError: true, text: '' }), true); }); @@ -63,3 +179,55 @@ test('a heredoc body that names a check is data, not a check; the command lines assert.equal(isVerificationCommand("cat > notes.md < { + const compounds = ['true || npm test', 'npm test || true', 'npm test | tail -5', 'npm test &', + 'npm test; echo done', 'false && npm test', 'echo start\nnpm test', '! npm test', 'time ! npm test', + 'if npm test; then true; fi', 'bash -c "npm test || true"', + "/bin/bash -c 'true || npm test'", 'bash -lc "npm test || true"', + 'corepack pnpm test | tail -5', 'command npm test || true', 'npm exec -- vitest run || true']; + for (const command of compounds) { + const result = { kind: 'shell', command }; + for (const exit of [0, 1]) { + assert.deepEqual(turnRecordOf([{ ...result, exit }]).checks, [{ command, exit: null, failed: null }]); + assert.equal(isFailedCheck({ ...result, exit }), false, command); + assert.equal(turnRecordOf([{ ...result, exit, output: '# pass 3\n# fail 0' }]).checks[0].failed, false, command); + assert.equal(isFailedCheck({ ...result, exit, output: '# pass 3\n# fail 1' }), true, command); + } + } +}); + +test('a wrapped shell script does not give its compound exit to the check inside it', () => { + for (const command of [ + "env CI=1 bash -c 'true || npm test'", + "sudo -n bash -c 'npm test | tail -5'", + "cd app && /bin/bash -c 'npm test || true'", + ]) { + assert.equal(verificationFailureOf({ command, exit: 0 }), null, command); + assert.equal(verificationFailureOf({ command, exit: 1 }), null, command); + assert.equal(verificationFailureOf({ command, exit: 0, text: '# pass 1\n# fail 0' }), false, command); + assert.equal(verificationFailureOf({ command, exit: 0, text: '# pass 0\n# fail 1' }), true, command); + const result = { kind: 'shell', command, exit: 0 }; + const [unverified] = turnRecordOf([result]).checks; + assert.deepEqual(unverified, { command, exit: null, failed: null }); + assert.equal(checkOutcomeOf(unverified), null); + const [verified] = turnRecordOf([{ ...result, output: '# pass 1\n# fail 0' }]).checks; + assert.deepEqual(checkOutcomeOf(verified), { ok: true }); + } +}); + +test('literal shell text and descriptor redirects preserve a direct check exit', () => { + for (const command of ['cd app && npm test', "cd app && /bin/bash -c 'npm test'"]) { + const directoryCheck = { kind: 'shell', command }; + assert.deepEqual(turnRecordOf([{ ...directoryCheck, exit: 0 }]).checks[0], { command, exit: 0, failed: false }); + assert.deepEqual(turnRecordOf([{ ...directoryCheck, exit: 1 }]).checks[0], { command, exit: null, failed: null }); + } + for (const command of ['npm test -- --grep "a|b; c && d"', 'npm test 2>&1', + 'npm test &> report.log', 'npm test >| report.log', 'npm test # after; ignore', + "sh -c 'npm test'", 'npm test\n', 'npm test <