diff --git a/actions/setup/js/experiment_helpers.cjs b/actions/setup/js/experiment_helpers.cjs index 541226dfcff..1a7c5cc115f 100644 --- a/actions/setup/js/experiment_helpers.cjs +++ b/actions/setup/js/experiment_helpers.cjs @@ -36,8 +36,25 @@ function readExperimentAssignments() { } return null; } catch { - return null; + // Fall through to environment-variable fallback. + } + + /** @type {Record} */ + const fromEnv = {}; + for (const [key, value] of Object.entries(process.env)) { + if (!key.startsWith("GH_AW_EXPERIMENTS_")) { + continue; + } + const name = key.substring("GH_AW_EXPERIMENTS_".length).toLowerCase(); + if (!name || typeof value !== "string" || value.length === 0) { + continue; + } + fromEnv[name] = value; + } + if (Object.keys(fromEnv).length > 0) { + return fromEnv; } + return null; } module.exports = { readExperimentAssignments, EXPERIMENT_ASSIGNMENTS_PATH }; diff --git a/actions/setup/js/experiment_helpers.test.cjs b/actions/setup/js/experiment_helpers.test.cjs index eca2c724d69..f13a0165c99 100644 --- a/actions/setup/js/experiment_helpers.test.cjs +++ b/actions/setup/js/experiment_helpers.test.cjs @@ -16,6 +16,8 @@ describe("readExperimentAssignments", () => { afterEach(() => { readFileSpy.mockRestore(); + delete process.env.GH_AW_EXPERIMENTS_PROMPT_STYLE; + delete process.env.GH_AW_EXPERIMENTS_REASONING_DEPTH; if (savedStateDir !== undefined) { process.env.GH_AW_EXPERIMENT_STATE_DIR = savedStateDir; } else { @@ -67,4 +69,10 @@ describe("readExperimentAssignments", () => { }); expect(readExperimentAssignments()).toEqual({ mode: "fast" }); }); + + it("falls back to GH_AW_EXPERIMENTS_* environment variables when assignments file is unavailable", () => { + process.env.GH_AW_EXPERIMENTS_PROMPT_STYLE = "concise"; + process.env.GH_AW_EXPERIMENTS_REASONING_DEPTH = "deep"; + expect(readExperimentAssignments()).toEqual({ prompt_style: "concise", reasoning_depth: "deep" }); + }); }); diff --git a/actions/setup/js/redact_secrets.cjs b/actions/setup/js/redact_secrets.cjs index 71963eeac29..371e33f2759 100644 --- a/actions/setup/js/redact_secrets.cjs +++ b/actions/setup/js/redact_secrets.cjs @@ -343,4 +343,40 @@ async function main() { } } -module.exports = { main, redactSecrets, redactBuiltInPatterns, redactStepSummaryContent, extractMCPGatewayTokens, BUILT_IN_PATTERNS, MCP_GATEWAY_CONFIG_PATHS }; +/** + * Targeted redaction pass for files in a specific directory. Used by the + * post-graders redaction step to scan grader output files that were written + * after the initial full-workspace redaction pass. + * + * @param {string} dir - Absolute directory path to scan + */ +async function redactFilesInDir(dir) { + try { + const secretNames = (process.env.GH_AW_SECRET_NAMES || "").split(",").filter(n => n.trim()); + /** @type {string[]} */ + const secretValues = []; + for (const secretName of secretNames) { + const value = process.env[`SECRET_${secretName.trim()}`]; + if (typeof value === "string" && value.trim() !== "") { + secretValues.push(value.trim()); + } + } + secretValues.push(...extractMCPGatewayTokens(MCP_GATEWAY_CONFIG_PATHS)); + + const targetExtensions = [".json"]; + const files = findFiles(dir, targetExtensions); + if (files.length === 0) return; + + let totalRedactions = 0; + for (const file of files) { + totalRedactions += processFile(file, secretValues); + } + if (totalRedactions > 0) { + core.info(`Grader output redaction: ${totalRedactions} redaction(s) in ${dir}`); + } + } catch (error) { + core.warning(`Grader output redaction failed: ${getErrorMessage(error)}`); + } +} + +module.exports = { main, redactFilesInDir, redactSecrets, redactBuiltInPatterns, redactStepSummaryContent, extractMCPGatewayTokens, BUILT_IN_PATTERNS, MCP_GATEWAY_CONFIG_PATHS }; diff --git a/actions/setup/js/run_evals.cjs b/actions/setup/js/run_evals.cjs index 3360ebf788c..1e841f004ca 100644 --- a/actions/setup/js/run_evals.cjs +++ b/actions/setup/js/run_evals.cjs @@ -36,6 +36,7 @@ const path = require("path"); const { ERR_VALIDATION, ERR_SYSTEM } = require("./error_codes.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { EVALS_OUTPUT_PATH } = require("./evals_constants.cjs"); +const { readExperimentAssignments } = require("./experiment_helpers.cjs"); const { resolveModelWithFallback } = require("./model_fallback.cjs"); const EVALS_DIR = "/tmp/gh-aw/evals"; @@ -127,6 +128,7 @@ async function parseMain() { const questionsRaw = process.env.GH_AW_EVALS_QUESTIONS; const model = resolveModelWithFallback(process.env, "GH_AW_EVALS_MODEL") || ""; const runID = process.env.GITHUB_RUN_ID || "unknown"; + const experimentAssignments = readExperimentAssignments(); /** @type {Array<{id: string, question: string}>} */ let questions = []; @@ -185,6 +187,7 @@ async function parseMain() { model, timestamp, runid: runID, + experiments: experimentAssignments || undefined, }; results.push(record); core.info(`Q[${q.id}]: ${answer}`); diff --git a/actions/setup/js/run_evals.test.cjs b/actions/setup/js/run_evals.test.cjs index b33ace0f170..56e59889fa7 100644 --- a/actions/setup/js/run_evals.test.cjs +++ b/actions/setup/js/run_evals.test.cjs @@ -5,6 +5,7 @@ import { createRequire } from "module"; const EVALS_DIR = "/tmp/gh-aw/evals"; const EVALS_LOG_PATH = `${EVALS_DIR}/evals.log`; const EVALS_OUTPUT_PATH = "/tmp/gh-aw/evals.jsonl"; +const EXPERIMENT_ASSIGNMENTS_PATH = "/tmp/gh-aw/experiments/assignments.json"; const require = createRequire(import.meta.url); const { MODEL_FALLBACK_ENV_VAR } = require("./model_fallback.cjs"); const { setupMain, parseMain, extractAssistantTextFromJsonlLog } = require("./run_evals.cjs"); @@ -32,6 +33,9 @@ describe("run_evals.cjs", () => { if (fs.existsSync(EVALS_OUTPUT_PATH)) { fs.unlinkSync(EVALS_OUTPUT_PATH); } + if (fs.existsSync(EXPERIMENT_ASSIGNMENTS_PATH)) { + fs.unlinkSync(EXPERIMENT_ASSIGNMENTS_PATH); + } }); afterEach(() => { @@ -42,6 +46,9 @@ describe("run_evals.cjs", () => { if (fs.existsSync(EVALS_OUTPUT_PATH)) { fs.unlinkSync(EVALS_OUTPUT_PATH); } + if (fs.existsSync(EXPERIMENT_ASSIGNMENTS_PATH)) { + fs.unlinkSync(EXPERIMENT_ASSIGNMENTS_PATH); + } }); it("stores the workflow run id when writing eval records", async () => { @@ -64,6 +71,20 @@ describe("run_evals.cjs", () => { }); }); + it("includes experiment assignments in eval records when available", async () => { + vi.stubEnv("GH_AW_EVALS_QUESTIONS", JSON.stringify([{ id: "labels-applied", question: "Did labels get applied?" }])); + vi.stubEnv("GH_AW_EVALS_MODEL", "small"); + vi.stubEnv("GITHUB_RUN_ID", "123456789"); + fs.mkdirSync("/tmp/gh-aw/experiments", { recursive: true }); + fs.writeFileSync("/tmp/gh-aw/experiments/assignments.json", JSON.stringify({ prompt_style: "concise" }) + "\n", "utf8"); + fs.writeFileSync(EVALS_LOG_PATH, "labels-applied: YES\n", "utf8"); + + await parseMain(); + + const [line] = fs.readFileSync(EVALS_OUTPUT_PATH, "utf8").trim().split("\n"); + expect(JSON.parse(line).experiments).toEqual({ prompt_style: "concise" }); + }); + it('falls back to "unknown" when the workflow run id is absent', async () => { vi.stubEnv("GH_AW_EVALS_QUESTIONS", JSON.stringify([{ id: "labels-applied", question: "Did labels get applied?" }])); vi.stubEnv("GH_AW_EVALS_MODEL", "small"); diff --git a/actions/setup/js/trace_graders.cjs b/actions/setup/js/trace_graders.cjs new file mode 100644 index 00000000000..b58bd98f6fd --- /dev/null +++ b/actions/setup/js/trace_graders.cjs @@ -0,0 +1,803 @@ +// @ts-check +/// + +const fs = require("fs"); +const path = require("path"); +const cp = require("child_process"); +const crypto = require("crypto"); +const { getErrorMessage } = require("./error_helpers.cjs"); +const { readExperimentAssignments } = require("./experiment_helpers.cjs"); + +// --- Constants --- +const TMP_GH_AW = "/tmp/gh-aw"; +const GRADERS_DIR = path.join(TMP_GH_AW, "agent", "graders"); +const MANIFEST_PATH = path.join(GRADERS_DIR, "grader_manifest.json"); +const RESULTS_PATH = path.join(GRADERS_DIR, "grader_results.json"); + +// Trace source file paths +const TOKEN_USAGE_PATHS = [ + path.join(TMP_GH_AW, "sandbox/firewall-audit-logs/api-proxy-logs/token-usage.jsonl"), + path.join(TMP_GH_AW, "sandbox/firewall/audit/api-proxy-logs/token-usage.jsonl"), + path.join(TMP_GH_AW, "sandbox/firewall/logs/api-proxy-logs/token-usage.jsonl"), +]; +const AGENT_USAGE_PATH = path.join(TMP_GH_AW, "agent_usage.json"); +const MCP_GATEWAY_LOG_PATHS = [path.join(TMP_GH_AW, "mcp-logs/gateway.jsonl"), path.join(TMP_GH_AW, "mcp-logs/mcp-gateway.jsonl"), path.join(TMP_GH_AW, "mcp-logs/rpc-messages.jsonl")]; +const AGENT_OUTPUT_PATH = path.join(TMP_GH_AW, "agent_output.json"); +const AGENT_LOG_PATH = path.join(TMP_GH_AW, "agent.log"); +const AGENT_LOG_JSONL_PATH = path.join(TMP_GH_AW, "agent_log.jsonl"); +const EVALS_RESULTS_PATH = path.join(TMP_GH_AW, "evals.jsonl"); + +// Safety limits +const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50 MB +const MAX_LINE_LENGTH = 1024 * 1024; // 1 MB per line +const SCRIPT_TIMEOUT_MS = 5000; // 5 seconds per custom grader +const SCRIPT_WORKER_OVERHEAD_MS = 1000; // Allow worker startup/serialization overhead. +const SCRIPT_WORKER_PATH = path.join(__dirname, "trace_graders_worker.cjs"); + +const GRADER_VERSION = 1; +const IMPLEMENTATION_ID = "gh-aw/graders"; + +// --- Trace preprocessing --- + +/** + * Safely read a file if it exists and is within size limits. + * @param {string} filePath + * @returns {string|null} + */ +function safeReadFile(filePath) { + try { + if (!fs.existsSync(filePath)) return null; + const stat = fs.statSync(filePath); + if (stat.size > MAX_FILE_SIZE) { + core.warning(`Graders: skipping oversized file ${filePath} (${stat.size} bytes)`); + return null; + } + return fs.readFileSync(filePath, "utf-8"); + } catch { + // Intentionally ignore unreadable/missing files in fallback path probes. + return null; + } +} + +/** + * Safely parse JSONL, skipping malformed or oversized lines. + * @param {string} content + * @returns {any[]} + */ +function safeParseJsonl(content) { + const results = []; + for (const line of content.split("\n")) { + const trimmed = line.trim(); + if (!trimmed) continue; + if (trimmed.length > MAX_LINE_LENGTH) continue; + try { + results.push(JSON.parse(trimmed)); + } catch { + continue; + } + } + return results; +} + +/** + * Safely parse JSON, returning null on failure. + * @param {string} content + * @returns {object|null} + */ +function safeParseJson(content) { + if (content.length > MAX_FILE_SIZE) return null; + try { + return JSON.parse(content); + } catch { + return null; + } +} + +/** + * @param {any} v + * @returns {v is Record} + */ +function isRecord(v) { + return v !== null && typeof v === "object" && !Array.isArray(v); +} + +/** + * Read the first available file from a list of candidate paths. + * @param {string[]} paths + * @returns {string|null} + */ +function readFirstAvailable(paths) { + for (const p of paths) { + const content = safeReadFile(p); + if (content !== null) return content; + } + return null; +} + +/** + * Deep-freeze an object recursively. Returns the same object. + * @template T + * @param {T} obj + * @returns {T} + */ +function deepFreeze(obj) { + if (obj === null || typeof obj !== "object") return obj; + Object.freeze(obj); + for (const key of Object.getOwnPropertyNames(obj)) { + const v = /** @type {any} */ obj[key]; + if (v !== null && typeof v === "object" && !Object.isFrozen(v)) { + deepFreeze(v); + } + } + return obj; +} + +/** + * Deep clone via JSON round-trip (safe for plain data objects). + * @param {any} obj + * @returns {any} + */ +function deepClone(obj) { + if (obj === null || obj === undefined) return obj; + try { + return structuredClone(obj); + } catch { + if (Array.isArray(obj)) { + return obj.map(item => deepClone(item)); + } + if (obj !== null && typeof obj === "object") { + const out = {}; + for (const [k, v] of Object.entries(obj)) { + out[k] = deepClone(v); + } + return out; + } + return obj; + } +} + +/** + * Build a compact summary of eval outputs for cross-linking with grader results. + * @returns {{total:number, yes:number, no:number, unknown:number, byQuestion:Record}|null} + */ +function readEvalSummary() { + const content = safeReadFile(EVALS_RESULTS_PATH); + if (!content) { + return null; + } + const records = safeParseJsonl(content); + if (records.length === 0) { + return null; + } + const summary = { + total: 0, + yes: 0, + no: 0, + unknown: 0, + byQuestion: {}, + }; + for (const record of records) { + if (!record || typeof record !== "object") { + continue; + } + const id = typeof record.id === "string" ? record.id : ""; + const answerRaw = typeof record.answer === "string" ? record.answer.toUpperCase() : "UNKNOWN"; + const answer = answerRaw === "YES" || answerRaw === "NO" ? answerRaw : "UNKNOWN"; + summary.total += 1; + if (answer === "YES") summary.yes += 1; + else if (answer === "NO") summary.no += 1; + else summary.unknown += 1; + if (id) { + summary.byQuestion[id] = answer; + } + } + return summary.total > 0 ? summary : null; +} + +/** + * @typedef {object} PreprocessedTrace + * @property {any[]} tokenUsageEntries - Parsed token-usage JSONL records + * @property {object|null} agentUsage - Parsed agent_usage.json + * @property {any[]} mcpGatewayEntries - Parsed MCP gateway log records + * @property {object|null} agentOutput - Parsed agent_output.json + * @property {any[]} toolCalls - Extracted tool call records from MCP gateway + * @property {any[]} gatewayRequests - Request/response pairs from gateway + * @property {any[]} retryEvents - Detected retry events + * @property {any[]} errorEvents - Detected error events + * @property {any[]} steps - Extracted execution steps (LLM requests) + * @property {number} totalInputTokens - Sum of input tokens + * @property {number} totalOutputTokens - Sum of output tokens + * @property {number} totalDurationMs - Sum of duration_ms from token usage + * @property {number} totalRequests - Count of token usage entries (LLM requests) + * @property {any[]} files - Files mentioned in agent output + * @property {any[]} artifacts - Artifacts/outputs from agent + */ + +/** + * Single preprocessing pass over all trace files. + * @returns {PreprocessedTrace} + */ +function preprocessTrace() { + // Token usage + const tokenContent = readFirstAvailable(TOKEN_USAGE_PATHS); + const tokenUsageEntries = tokenContent ? safeParseJsonl(tokenContent) : []; + + // Agent usage + const agentUsageContent = safeReadFile(AGENT_USAGE_PATH); + const agentUsage = agentUsageContent ? safeParseJson(agentUsageContent) : null; + + // MCP gateway logs + const gatewayContent = readFirstAvailable(MCP_GATEWAY_LOG_PATHS); + const mcpGatewayEntries = gatewayContent ? safeParseJsonl(gatewayContent) : []; + + // Agent output + const agentOutputContent = safeReadFile(AGENT_OUTPUT_PATH); + const agentOutput = agentOutputContent ? safeParseJson(agentOutputContent) : null; + + // Extract tool calls from MCP gateway entries. + // Also support rpc fallback records that expose tool_name/payload.tool_name. + const toolCalls = mcpGatewayEntries + .filter(e => { + const payload = isRecord(e.payload) ? e.payload : null; + return e.type === "tool_call" || e.method === "tools/call" || e.event === "tool_call" || typeof e.tool_name === "string" || (payload !== null && typeof payload.tool_name === "string"); + }) + .map(e => { + const payload = isRecord(e.payload) ? e.payload : null; + const toolName = typeof e.tool_name === "string" ? e.tool_name : payload !== null && typeof payload.tool_name === "string" ? payload.tool_name : undefined; + const args = payload !== null ? (payload.arguments ?? payload.params) : undefined; + return { + ...e, + name: e.name || e.tool || toolName, + tool: e.tool || toolName, + arguments: e.arguments ?? args, + }; + }); + + // Gateway request/response pairs + const gatewayRequests = mcpGatewayEntries.filter(e => e.type === "request" || e.type === "response" || e.method); + + // Retry events + const retryEvents = mcpGatewayEntries.filter(e => e.retry === true || e.event === "retry" || (typeof e.message === "string" && /retry|retrying/i.test(String(e.message)))); + + // Error events + const errorEvents = mcpGatewayEntries.filter(e => e.level === "error" || e.type === "error" || e.event === "error"); + + // Aggregate token counts + let totalInputTokens = 0; + let totalOutputTokens = 0; + let totalDurationMs = 0; + for (const entry of tokenUsageEntries) { + totalInputTokens += Number(entry.input_tokens) || 0; + totalOutputTokens += Number(entry.output_tokens) || 0; + totalDurationMs += Number(entry.duration_ms) || 0; + } + + // Steps: each token usage entry represents an LLM request/step + const steps = tokenUsageEntries.map((entry, i) => ({ + index: i, + inputTokens: Number(entry.input_tokens) || 0, + outputTokens: Number(entry.output_tokens) || 0, + durationMs: Number(entry.duration_ms) || 0, + model: entry.model || null, + })); + + // Extract files and artifacts from agent output + const ao = agentOutput !== null && typeof agentOutput === "object" ? agentOutput : null; + const files = ao && "files" in ao && Array.isArray(ao.files) ? ao.files : []; + const artifacts = ao && "outputs" in ao && Array.isArray(ao.outputs) ? ao.outputs : ao && "items" in ao && Array.isArray(ao.items) ? ao.items : []; + + return { + tokenUsageEntries, + agentUsage, + mcpGatewayEntries, + agentOutput, + toolCalls, + gatewayRequests, + retryEvents, + errorEvents, + steps, + totalInputTokens, + totalOutputTokens, + totalDurationMs, + totalRequests: tokenUsageEntries.length, + files, + artifacts, + }; +} + +// --- Built-in graders --- + +/** @type {Record} */ +const BUILTIN_META = { + "tool-success-rate": { unit: "ratio", direction: "higher_is_better", threshold: 0.8, min: 0, max: 1 }, + "tool-failure-count": { unit: "count", direction: "lower_is_better", threshold: 5 }, + retries: { unit: "count", direction: "lower_is_better", threshold: 10 }, + loops: { unit: "count", direction: "lower_is_better", threshold: 3 }, + "trajectory-efficiency": { unit: "ratio", direction: "higher_is_better", min: 0, max: 1 }, + "execution-step-count": { unit: "count", direction: "lower_is_better" }, + "execution-duration": { unit: "ms", direction: "lower_is_better" }, + "context-growth": { unit: "factor", direction: "lower_is_better" }, + "artifact-production": { unit: "count", direction: "higher_is_better" }, +}; + +/** + * @param {any} toolCall + * @returns {boolean} + */ +function isToolFailure(toolCall) { + return toolCall.success === false || toolCall.status === "error" || toolCall.status === "failure" || toolCall.error !== undefined; +} + +/** + * @param {PreprocessedTrace} trace + * @returns {number} Success rate of tool calls (0-1), or 1 if no tool calls + */ +function gradeToolSuccessRate(trace) { + if (trace.toolCalls.length === 0) return 1; + const successes = trace.toolCalls.length - trace.toolCalls.filter(isToolFailure).length; + return successes / trace.toolCalls.length; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeToolFailureCount(trace) { + return trace.toolCalls.filter(isToolFailure).length; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeRetries(trace) { + return trace.retryEvents.length; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeLoops(trace) { + let loops = 0; + let prevKey = ""; + for (const t of trace.toolCalls) { + const key = `${String(t.name || t.tool)}:${JSON.stringify(t.arguments || t.params || "")}`; + if (key === prevKey) loops++; + prevKey = key; + } + return loops; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeTrajectoryEfficiency(trace) { + if (trace.toolCalls.length === 0) return 1; + const uniqueTools = new Set(trace.toolCalls.map(t => String(t.name || t.tool || ""))); + return Math.min(1, uniqueTools.size / trace.toolCalls.length); +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeExecutionStepCount(trace) { + return trace.totalRequests; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeExecutionDuration(trace) { + return trace.totalDurationMs; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeContextGrowth(trace) { + if (trace.tokenUsageEntries.length < 2) return 1; + const first = trace.tokenUsageEntries[0]; + const firstTokens = (Number(first.input_tokens) || 0) + (Number(first.output_tokens) || 0); + if (firstTokens === 0) return 1; + const totalTokens = trace.totalInputTokens + trace.totalOutputTokens; + return totalTokens / firstTokens; +} + +/** @param {PreprocessedTrace} trace @returns {number} */ +function gradeArtifactProduction(trace) { + return trace.artifacts.length; +} + +/** @type {Record number>} */ +const BUILTIN_GRADERS = { + "tool-success-rate": gradeToolSuccessRate, + "tool-failure-count": gradeToolFailureCount, + retries: gradeRetries, + loops: gradeLoops, + "trajectory-efficiency": gradeTrajectoryEfficiency, + "execution-step-count": gradeExecutionStepCount, + "execution-duration": gradeExecutionDuration, + "context-growth": gradeContextGrowth, + "artifact-production": gradeArtifactProduction, +}; + +// --- Execution --- + +/** + * Evaluate a grader's pass/fail against its threshold. + * @param {number} value + * @param {string} direction + * @param {number|undefined} threshold + * @returns {boolean|null} null if no threshold set + */ +function evaluateThreshold(value, direction, threshold) { + if (threshold === undefined || threshold === null) return null; + if (direction === "higher_is_better") return value >= threshold; + if (direction === "lower_is_better") return value <= threshold; + return null; +} + +/** + * @typedef {object} GraderResult + * @property {string} id + * @property {string} name + * @property {number|null} value + * @property {string} unit + * @property {boolean|null} passed + * @property {string} status - "pass" | "fail" | "error" | "unavailable" + * @property {string} [severity] + * @property {string} [details] + * @property {string} [message] + * @property {string} [error] + * @property {string} source - "builtin" | "inline" + * @property {{id: string, version: number, digest?: string}} implementation + */ + +/** + * Escape values for step summary rendering. + * @param {any} value + * @returns {string} + */ +function sanitizeSummaryText(value) { + return String(value ?? "") + .replace(/\r?\n/g, " ") + .replace(/[<>&]/g, ch => (ch === "<" ? "<" : ch === ">" ? ">" : "&")) + .trim(); +} + +/** + * Normalize a grader result from either built-in number or custom object return. + * @param {string} id + * @param {any} rawResult - number or {value, unit?, passed?, severity?, details?, message?} + * @param {{name: string, unit: string, direction: string, threshold?: number, source: string, digest?: string}} meta + * @returns {GraderResult} + */ +function normalizeResult(id, rawResult, meta) { + /** @type {GraderResult} */ + const base = { + id, + name: meta.name || id, + value: null, + unit: meta.unit || "", + passed: null, + status: "error", + source: meta.source, + implementation: { id: IMPLEMENTATION_ID, version: GRADER_VERSION, ...(meta.digest ? { digest: meta.digest } : {}) }, + }; + + if (rawResult === null || rawResult === undefined) { + base.status = "unavailable"; + base.message = "grader returned null/undefined"; + return base; + } + + let value; + if (typeof rawResult === "object" && rawResult !== null && !Array.isArray(rawResult)) { + // Object result from custom script + value = rawResult.value; + if (rawResult.unit) base.unit = String(rawResult.unit); + if (rawResult.severity) base.severity = String(rawResult.severity); + if (rawResult.details) base.details = String(rawResult.details); + if (rawResult.message) base.message = String(rawResult.message); + if (typeof rawResult.passed === "boolean") base.passed = rawResult.passed; + } else { + value = rawResult; + } + + if (typeof value !== "number" || !isFinite(value)) { + base.status = "error"; + base.error = `grader ${id} returned non-finite value: ${value}`; + return base; + } + + base.value = value; + + // Evaluate threshold if not already set by custom script + if (base.passed === null) { + base.passed = evaluateThreshold(value, meta.direction, meta.threshold); + } + + // Determine status + if (base.passed === true) base.status = "pass"; + else if (base.passed === false) base.status = "fail"; + else base.status = "pass"; // no threshold = informational pass + + return base; +} + +/** + * Run a single built-in grader safely. + * @param {string} id + * @param {PreprocessedTrace} trace + * @param {{name: string, unit: string, direction: string, threshold?: number, source: string}} meta + * @returns {GraderResult} + */ +function runBuiltinGrader(id, trace, meta) { + const fn = BUILTIN_GRADERS[id]; + if (!fn) { + return { ...normalizeResult(id, null, meta), status: "error", error: `grader ${id}: no implementation found` }; + } + try { + const value = fn(trace); + return normalizeResult(id, value, meta); + } catch (err) { + const result = normalizeResult(id, null, meta); + result.status = "error"; + result.error = `grader ${id} runtime error: ${getErrorMessage(err)}`; + return result; + } +} + +/** + * Run a custom inline script in an isolated worker subprocess. + * Script receives {trace, run, workflow, config, helpers} and should return {value, ...} or a number. + * @param {string} id + * @param {string} script + * @param {PreprocessedTrace} trace + * @param {{name: string, unit: string, direction: string, threshold?: number, source: string, digest?: string, config?: object, graderCount?: number}} meta + * @returns {GraderResult} + */ +function executeCustomGraderInSubprocess(id, script, trace, meta) { + const payload = { + id, + script, + trace, + config: meta.config || {}, + graderCount: Number(meta.graderCount) || 0, + timeoutMs: SCRIPT_TIMEOUT_MS, + }; + const safeEnv = {}; + for (const key of ["PATH", "HOME", "TMPDIR", "TEMP", "TMP", "SystemRoot", "ComSpec"]) { + if (process.env[key]) { + safeEnv[key] = process.env[key]; + } + } + const timeoutMs = SCRIPT_TIMEOUT_MS + SCRIPT_WORKER_OVERHEAD_MS; + const proc = cp.spawnSync(process.execPath, [SCRIPT_WORKER_PATH], { + input: JSON.stringify(payload), + encoding: "utf-8", + timeout: timeoutMs, + maxBuffer: 1024 * 1024, + env: safeEnv, + }); + + const procError = proc.error; + if (procError) { + if (typeof procError.message === "string" && /ETIMEDOUT|timed out/i.test(procError.message)) { + throw new Error(`script worker timed out after ${timeoutMs}ms`); + } + throw procError; + } + + if (proc.status !== 0) { + const stderr = (proc.stderr || "").trim(); + throw new Error(stderr || `script worker exited with status ${String(proc.status)}`); + } + + let parsed; + try { + parsed = JSON.parse(proc.stdout || "{}"); + } catch (err) { + throw new Error(`invalid script worker output: ${getErrorMessage(err)}`, { cause: err }); + } + + if (!parsed || parsed.ok !== true) { + throw new Error(parsed && typeof parsed.error === "string" ? parsed.error : "script worker returned an error"); + } + return parsed.value; +} + +function runCustomGrader(id, script, trace, meta) { + try { + const rawResult = executeCustomGraderInSubprocess(id, script, trace, meta); + return normalizeResult(id, rawResult, meta); + } catch (err) { + const result = normalizeResult(id, null, meta); + result.status = "error"; + result.error = `grader ${id} runtime error: ${getErrorMessage(err)}`; + return result; + } +} + +/** + * Legacy adapter for existing tests. Runs a grader by id. + * @param {string} id + * @param {boolean} builtin + * @param {string|undefined} script + * @param {PreprocessedTrace} trace + * @param {object} [config] + * @returns {{ value: number|null, error: string|null }} + */ +function runGrader(id, builtin, script, trace, config) { + const meta = { name: id, unit: "", direction: "", source: builtin ? "builtin" : "inline", config }; + /** @type {GraderResult} */ + let result; + if (builtin && BUILTIN_GRADERS[id]) { + result = runBuiltinGrader(id, trace, meta); + } else if (script) { + result = runCustomGrader(id, script, trace, meta); + } else { + return { value: null, error: `grader ${id}: no implementation found` }; + } + return { value: result.value, error: result.error || null }; +} + +/** + * Main entry point. Called from the github-script step with base64 manifest and exec spec. + * @param {string} manifestB64 - Base64-encoded JSON manifest + * @param {string} [execSpecB64] - Base64-encoded JSON array of {id, script} + */ +async function main(manifestB64, execSpecB64) { + /** @type {{version: number, graders: any[]}} */ + let manifest; + try { + const manifestJson = Buffer.from(manifestB64, "base64").toString("utf-8"); + manifest = JSON.parse(manifestJson); + } catch (err) { + core.setFailed(`Graders: failed to parse manifest: ${getErrorMessage(err)}`); + return; + } + + // Decode execution spec (custom scripts) + /** @type {Record} */ + const scriptMap = {}; + if (execSpecB64) { + try { + const specJson = Buffer.from(execSpecB64, "base64").toString("utf-8"); + const specs = JSON.parse(specJson); + for (const s of specs) { + if (s.id && s.script) scriptMap[s.id] = s.script; + } + } catch (err) { + core.warning(`Graders: failed to parse exec spec: ${getErrorMessage(err)}`); + } + } + + // Write manifest file + try { + fs.mkdirSync(GRADERS_DIR, { recursive: true }); + fs.writeFileSync(MANIFEST_PATH, JSON.stringify(manifest, null, 2)); + } catch (err) { + core.warning(`Graders: failed to write manifest: ${getErrorMessage(err)}`); + } + + // Filter to enabled graders + const graders = manifest.graders || []; + const enabledGraders = graders.filter(g => g.enabled); + if (enabledGraders.length === 0) { + core.info("Graders: no enabled graders, skipping"); + return; + } + + // Single preprocessing pass + core.info(`Graders: preprocessing trace files for ${enabledGraders.length} grader(s)...`); + const trace = preprocessTrace(); + + // Run all graders + /** @type {GraderResult[]} */ + const results = []; + for (const grader of enabledGraders) { + const meta = { + name: grader.name || grader.id, + unit: grader.unit || "", + direction: grader.direction || "", + threshold: grader.threshold, + source: grader.source || "builtin", + digest: grader.digest, + config: grader.config, + graderCount: enabledGraders.length, + }; + /** @type {GraderResult} */ + let result; + if (grader.source === "builtin" && BUILTIN_GRADERS[grader.id]) { + result = runBuiltinGrader(grader.id, trace, meta); + } else if (scriptMap[grader.id]) { + result = runCustomGrader(grader.id, scriptMap[grader.id], trace, meta); + } else { + result = normalizeResult(grader.id, null, meta); + result.status = "unavailable"; + result.error = `grader ${grader.id}: no implementation available`; + } + results.push(result); + if (result.error) { + core.warning(`Grader ${grader.id}: ${result.error}`); + } + } + + // Build normalized output — NO timestamp for deterministic byte-equivalence + const passed = results.filter(r => r.status === "pass").length; + const failed = results.filter(r => r.status === "fail").length; + const errorCount = results.filter(r => r.status === "error").length; + + const output = { + version: GRADER_VERSION, + run: { + graderCount: results.length, + passed, + failed, + errors: errorCount, + }, + context: { + experiments: readExperimentAssignments() || undefined, + evals: readEvalSummary() || undefined, + }, + results, + }; + + // Write results + try { + fs.writeFileSync(RESULTS_PATH, JSON.stringify(output, null, 2)); + core.info(`Graders: wrote results to ${RESULTS_PATH}`); + } catch (err) { + core.warning(`Graders: failed to write results: ${getErrorMessage(err)}`); + } + + // Step summary + core.summary.addHeading("Graders", 3); + const tableResults = results.filter(r => r.status !== "unavailable"); + if (tableResults.length > 0) { + const rows = tableResults.map(r => { + const statusIcon = r.status === "pass" ? "✅" : r.status === "fail" ? "❌" : "⚠️"; + const val = r.value !== null ? String(Number(r.value.toFixed(4))) : "—"; + return [statusIcon, sanitizeSummaryText(r.name), r.source, val, sanitizeSummaryText(r.unit || "—")]; + }); + core.summary.addTable([ + [ + { data: "", header: true }, + { data: "Grader", header: true }, + { data: "Source", header: true }, + { data: "Value", header: true }, + { data: "Unit", header: true }, + ], + ...rows, + ]); + } + const errResults = results.filter(r => r.error); + if (errResults.length > 0) { + const errLines = errResults.map(r => `- **${sanitizeSummaryText(r.id)}**: runtime error (see step logs)`).join("\n"); + core.summary.addDetails("Grader Errors", errLines); + } + await core.summary.write({ overwrite: false }); + + core.info(`Graders: ${passed} passed, ${failed} failed, ${errorCount} errors`); +} + +module.exports = { + main, + preprocessTrace, + safeReadFile, + safeParseJsonl, + safeParseJson, + readFirstAvailable, + readEvalSummary, + deepFreeze, + deepClone, + runGrader, + runBuiltinGrader, + runCustomGrader, + normalizeResult, + evaluateThreshold, + BUILTIN_GRADERS, + BUILTIN_META, + GRADER_VERSION, + IMPLEMENTATION_ID, + GRADERS_DIR, + MANIFEST_PATH, + RESULTS_PATH, + MAX_FILE_SIZE, + MAX_LINE_LENGTH, + SCRIPT_TIMEOUT_MS, + gradeToolSuccessRate, + gradeToolFailureCount, + gradeRetries, + gradeLoops, + gradeTrajectoryEfficiency, + gradeExecutionStepCount, + gradeExecutionDuration, + gradeContextGrowth, + gradeArtifactProduction, +}; diff --git a/actions/setup/js/trace_graders.test.cjs b/actions/setup/js/trace_graders.test.cjs new file mode 100644 index 00000000000..27bd55740db --- /dev/null +++ b/actions/setup/js/trace_graders.test.cjs @@ -0,0 +1,652 @@ +// @ts-check +/// + +const fs = require("fs"); +const path = require("path"); +const os = require("os"); + +const { + main, + preprocessTrace, + safeReadFile, + safeParseJsonl, + safeParseJson, + readFirstAvailable, + readEvalSummary, + deepFreeze, + deepClone, + runGrader, + runBuiltinGrader, + runCustomGrader, + normalizeResult, + evaluateThreshold, + BUILTIN_GRADERS, + BUILTIN_META, + GRADER_VERSION, + IMPLEMENTATION_ID, + MANIFEST_PATH, + RESULTS_PATH, + MAX_FILE_SIZE, + MAX_LINE_LENGTH, + SCRIPT_TIMEOUT_MS, + gradeToolSuccessRate, + gradeToolFailureCount, + gradeRetries, + gradeLoops, + gradeTrajectoryEfficiency, + gradeExecutionStepCount, + gradeExecutionDuration, + gradeContextGrowth, + gradeArtifactProduction, +} = require("./trace_graders.cjs"); + +// --- Helper to create a minimal trace --- + +/** @returns {import("./trace_graders.cjs").PreprocessedTrace} */ +function makeTrace(overrides = {}) { + return { + tokenUsageEntries: [], + agentUsage: null, + mcpGatewayEntries: [], + agentOutput: null, + toolCalls: [], + gatewayRequests: [], + retryEvents: [], + errorEvents: [], + steps: [], + totalInputTokens: 0, + totalOutputTokens: 0, + totalDurationMs: 0, + totalRequests: 0, + files: [], + artifacts: [], + ...overrides, + }; +} + +describe("trace_graders", () => { + // --- safeParseJsonl --- + describe("safeParseJsonl", () => { + it("parses valid JSONL", () => { + const content = '{"a":1}\n{"b":2}\n'; + const result = safeParseJsonl(content); + expect(result).toEqual([{ a: 1 }, { b: 2 }]); + }); + + it("skips malformed lines", () => { + const content = '{"a":1}\nnot json\n{"b":2}\n'; + const result = safeParseJsonl(content); + expect(result).toEqual([{ a: 1 }, { b: 2 }]); + }); + + it("skips oversized lines", () => { + const longLine = '{"x":"' + "a".repeat(MAX_LINE_LENGTH + 10) + '"}'; + const content = '{"a":1}\n' + longLine + '\n{"b":2}\n'; + const result = safeParseJsonl(content); + expect(result).toEqual([{ a: 1 }, { b: 2 }]); + }); + + it("handles empty input", () => { + expect(safeParseJsonl("")).toEqual([]); + expect(safeParseJsonl("\n\n")).toEqual([]); + }); + }); + + // --- safeParseJson --- + describe("safeParseJson", () => { + it("parses valid JSON", () => { + expect(safeParseJson('{"a":1}')).toEqual({ a: 1 }); + }); + + // --- readEvalSummary --- + describe("readEvalSummary", () => { + const evalsPath = "/tmp/gh-aw/evals.jsonl"; + + afterEach(() => { + if (fs.existsSync(evalsPath)) { + fs.unlinkSync(evalsPath); + } + }); + + it("returns null when evals file is absent", () => { + if (fs.existsSync(evalsPath)) { + fs.unlinkSync(evalsPath); + } + expect(readEvalSummary()).toBeNull(); + }); + + it("summarizes YES/NO/UNKNOWN answers", () => { + fs.mkdirSync(path.dirname(evalsPath), { recursive: true }); + fs.writeFileSync(evalsPath, [JSON.stringify({ id: "q1", answer: "YES" }), JSON.stringify({ id: "q2", answer: "no" }), JSON.stringify({ id: "q3", answer: "MAYBE" })].join("\n") + "\n", "utf8"); + expect(readEvalSummary()).toEqual({ + total: 3, + yes: 1, + no: 1, + unknown: 1, + byQuestion: { + q1: "YES", + q2: "NO", + q3: "UNKNOWN", + }, + }); + }); + }); + + it("returns null for invalid JSON", () => { + expect(safeParseJson("not json")).toBeNull(); + }); + + it("returns null for oversized content", () => { + const big = "a".repeat(MAX_FILE_SIZE + 1); + expect(safeParseJson(big)).toBeNull(); + }); + }); + + // --- deepFreeze --- + describe("deepFreeze", () => { + it("freezes objects deeply", () => { + const obj = { a: { b: { c: 1 } } }; + deepFreeze(obj); + expect(Object.isFrozen(obj)).toBe(true); + expect(Object.isFrozen(obj.a)).toBe(true); + expect(Object.isFrozen(obj.a.b)).toBe(true); + }); + + it("handles null/primitives", () => { + expect(deepFreeze(null)).toBeNull(); + expect(deepFreeze(42)).toBe(42); + }); + }); + + // --- Built-in graders --- + describe("gradeToolSuccessRate", () => { + it("returns 1 for no tool calls", () => { + expect(gradeToolSuccessRate(makeTrace())).toBe(1); + }); + + it("computes success rate", () => { + const trace = makeTrace({ + toolCalls: [ + { name: "a", success: true }, + { name: "b", success: false }, + { name: "c", success: true }, + ], + }); + expect(gradeToolSuccessRate(trace)).toBeCloseTo(2 / 3); + }); + + it("handles error field as failure indicator", () => { + const trace = makeTrace({ + toolCalls: [{ name: "a" }, { name: "b", error: "something failed" }], + }); + expect(gradeToolSuccessRate(trace)).toBe(0.5); + }); + }); + + describe("gradeToolFailureCount", () => { + it("returns 0 for no tool calls", () => { + expect(gradeToolFailureCount(makeTrace())).toBe(0); + }); + + it("counts failures", () => { + const trace = makeTrace({ + toolCalls: [ + { name: "a", success: true }, + { name: "b", success: false }, + { name: "c", error: "err" }, + ], + }); + expect(gradeToolFailureCount(trace)).toBe(2); + }); + + it("matches success-rate complement for ambiguous calls", () => { + const trace = makeTrace({ + toolCalls: [{ name: "a", success: true }, { name: "b", status: "error" }, { name: "c" }], + }); + const failures = gradeToolFailureCount(trace); + const successRate = gradeToolSuccessRate(trace); + expect(successRate).toBeCloseTo((trace.toolCalls.length - failures) / trace.toolCalls.length); + }); + }); + + describe("gradeRetries", () => { + it("returns 0 for no retries", () => { + expect(gradeRetries(makeTrace())).toBe(0); + }); + + it("counts retry events from preprocessed retryEvents", () => { + const trace = makeTrace({ + retryEvents: [{ event: "retry" }, { retry: true }, { message: "Retrying request" }], + }); + expect(gradeRetries(trace)).toBe(3); + }); + }); + + describe("gradeLoops", () => { + it("returns 0 for no loops", () => { + expect(gradeLoops(makeTrace())).toBe(0); + }); + + it("detects consecutive identical calls", () => { + const trace = makeTrace({ + toolCalls: [ + { name: "read", arguments: { path: "/a" } }, + { name: "read", arguments: { path: "/a" } }, + { name: "read", arguments: { path: "/a" } }, + { name: "write", arguments: { path: "/b" } }, + ], + }); + expect(gradeLoops(trace)).toBe(2); + }); + }); + + describe("gradeTrajectoryEfficiency", () => { + it("returns 1 for no tool calls", () => { + expect(gradeTrajectoryEfficiency(makeTrace())).toBe(1); + }); + + it("computes efficiency", () => { + const trace = makeTrace({ + toolCalls: [{ name: "read" }, { name: "write" }, { name: "read" }, { name: "read" }], + }); + expect(gradeTrajectoryEfficiency(trace)).toBe(0.5); + }); + }); + + describe("gradeExecutionStepCount", () => { + it("returns total LLM requests", () => { + expect(gradeExecutionStepCount(makeTrace({ totalRequests: 42 }))).toBe(42); + }); + }); + + describe("gradeExecutionDuration", () => { + it("returns total duration", () => { + expect(gradeExecutionDuration(makeTrace({ totalDurationMs: 12345 }))).toBe(12345); + }); + }); + + describe("gradeContextGrowth", () => { + it("returns 1 for fewer than 2 entries", () => { + expect(gradeContextGrowth(makeTrace())).toBe(1); + expect(gradeContextGrowth(makeTrace({ tokenUsageEntries: [{ input_tokens: 100, output_tokens: 50 }] }))).toBe(1); + }); + + it("computes growth ratio", () => { + const trace = makeTrace({ + tokenUsageEntries: [ + { input_tokens: 100, output_tokens: 50 }, + { input_tokens: 200, output_tokens: 100 }, + ], + totalInputTokens: 300, + totalOutputTokens: 150, + }); + expect(gradeContextGrowth(trace)).toBe(3); + }); + }); + + describe("gradeArtifactProduction", () => { + it("returns 0 for no agent output", () => { + expect(gradeArtifactProduction(makeTrace())).toBe(0); + }); + + it("counts artifacts array", () => { + const trace = makeTrace({ + artifacts: [{ type: "pr" }, { type: "issue" }], + }); + expect(gradeArtifactProduction(trace)).toBe(2); + }); + }); + + // --- normalizeResult --- + describe("normalizeResult", () => { + const meta = { name: "Test", unit: "count", direction: "lower_is_better", threshold: 5, source: "builtin" }; + + it("normalizes a number result with threshold pass", () => { + const r = normalizeResult("test", 3, meta); + expect(r.id).toBe("test"); + expect(r.value).toBe(3); + expect(r.passed).toBe(true); + expect(r.status).toBe("pass"); + expect(r.unit).toBe("count"); + expect(r.implementation.id).toBe(IMPLEMENTATION_ID); + }); + + it("normalizes a number result with threshold fail", () => { + const r = normalizeResult("test", 10, meta); + expect(r.passed).toBe(false); + expect(r.status).toBe("fail"); + }); + + it("handles object results from custom scripts", () => { + const r = normalizeResult("test", { value: 42, unit: "ms", severity: "warning", details: "too slow" }, { ...meta, source: "inline" }); + expect(r.value).toBe(42); + expect(r.unit).toBe("ms"); + expect(r.severity).toBe("warning"); + expect(r.details).toBe("too slow"); + }); + + it("handles null result as unavailable", () => { + const r = normalizeResult("test", null, meta); + expect(r.status).toBe("unavailable"); + }); + + it("handles non-finite value as error", () => { + const r = normalizeResult("test", NaN, meta); + expect(r.status).toBe("error"); + expect(r.error).toContain("non-finite"); + }); + + it("includes digest in implementation when provided", () => { + const r = normalizeResult("test", 1, { ...meta, source: "inline", digest: "abc123" }); + expect(r.implementation.digest).toBe("abc123"); + }); + }); + + // --- evaluateThreshold --- + describe("evaluateThreshold", () => { + it("passes when higher_is_better and value >= threshold", () => { + expect(evaluateThreshold(0.9, "higher_is_better", 0.8)).toBe(true); + expect(evaluateThreshold(0.7, "higher_is_better", 0.8)).toBe(false); + }); + + it("passes when lower_is_better and value <= threshold", () => { + expect(evaluateThreshold(3, "lower_is_better", 5)).toBe(true); + expect(evaluateThreshold(10, "lower_is_better", 5)).toBe(false); + }); + + it("returns null when no threshold", () => { + expect(evaluateThreshold(1, "higher_is_better", undefined)).toBeNull(); + }); + }); + + // --- runGrader --- + describe("runGrader", () => { + it("runs built-in grader", () => { + const trace = makeTrace({ totalRequests: 5 }); + const result = runGrader("execution-step-count", true, undefined, trace); + expect(result.value).toBe(5); + expect(result.error).toBeNull(); + }); + + it("runs inline script via vm sandbox", () => { + const trace = makeTrace({ toolCalls: [{ name: "a" }, { name: "b" }] }); + const result = runGrader("custom", false, "return { value: trace.toolCalls.length }", trace); + expect(result.value).toBe(2); + expect(result.error).toBeNull(); + }); + + it("supports legacy expression-style scripts", () => { + const trace = makeTrace({ toolCalls: [{ name: "a" }] }); + const result = runGrader("custom", false, "return trace.toolCalls.length", trace); + expect(result.value).toBe(1); + }); + + it("catches script errors", () => { + const result = runGrader("bad", false, "return undefinedVar.prop", makeTrace()); + expect(result.value).toBeNull(); + expect(result.error).toContain("runtime error"); + }); + + it("rejects non-numeric results from inline scripts", () => { + const result = runGrader("str", false, 'return "hello"', makeTrace()); + expect(result.value).toBeNull(); + expect(result.error).toContain("non-finite"); + }); + + it("rejects NaN from built-in graders", () => { + const result = runGrader("nan-test", false, "return NaN", makeTrace()); + expect(result.value).toBeNull(); + expect(result.error).toContain("non-finite"); + }); + + it("rejects Infinity", () => { + const result = runGrader("inf-test", false, "return Infinity", makeTrace()); + expect(result.value).toBeNull(); + expect(result.error).toContain("non-finite"); + }); + }); + + // --- runCustomGrader node:vm sandbox --- + describe("runCustomGrader sandbox", () => { + const meta = { name: "test", unit: "", direction: "", source: "inline" }; + + it("cannot access require", () => { + const result = runCustomGrader("test", "return typeof require", makeTrace(), meta); + expect(result.value).toBeNull(); // "undefined" is not a number + }); + + it("cannot access process", () => { + const result = runCustomGrader("test", "return typeof process", makeTrace(), meta); + expect(result.value).toBeNull(); // "undefined" is not a number + }); + + it("cannot access fetch", () => { + const result = runCustomGrader("test", "return typeof fetch", makeTrace(), meta); + expect(result.value).toBeNull(); + }); + + it("cannot construct Date", () => { + const result = runCustomGrader("test", "try { new Date(); return 0 } catch(e) { return 1 }", makeTrace(), meta); + // Date is not available in the sandbox + expect(result.value).toBe(1); + }); + + it("cannot use Math.random", () => { + const result = runCustomGrader("test", "return typeof Math.random", makeTrace(), meta); + expect(result.value).toBeNull(); // "undefined" is not a number + }); + + it("cannot mutate frozen trace", () => { + const trace = makeTrace({ toolCalls: [{ name: "a" }] }); + const result = runCustomGrader("test", "try { trace.toolCalls.push({name:'b'}); return 0 } catch(e) { return 1 }", trace, meta); + expect(result.value).toBe(1); + }); + + it("receives config parameter", () => { + const result = runCustomGrader("test", "return config.multiplier * 2", makeTrace(), { ...meta, config: { multiplier: 5 } }); + expect(result.value).toBe(10); + }); + + it("handles scripts containing template literals safely", () => { + const trace = makeTrace({ toolCalls: Array.from({ length: 12 }, () => ({ name: "a" })) }); + const result = runCustomGrader("test", "return `${trace.toolCalls.length}`.length", trace, meta); + expect(result.value).toBe(2); + expect(result.error).toBeUndefined(); + }); + + it("receives run metadata from caller", () => { + const result = runCustomGrader("test", "return run.graderCount", makeTrace(), { ...meta, graderCount: 7 }); + expect(result.value).toBe(7); + }); + + it("can use helpers", () => { + const result = runCustomGrader("test", "return helpers.clamp(10, 0, 5)", makeTrace(), meta); + expect(result.value).toBe(5); + }); + + it("supports multiline return object form", () => { + const script = ` + const count = trace.toolCalls.length; + return { + value: count, + unit: "count", + severity: count > 10 ? "warning" : "info" + } + `; + const trace = makeTrace({ toolCalls: [{ name: "a" }, { name: "b" }] }); + const result = runCustomGrader("test", script, trace, meta); + expect(result.value).toBe(2); + expect(result.unit).toBe("count"); + }); + + it("times out on infinite loops", () => { + const result = runCustomGrader("test", "while(true){} return 1", makeTrace(), meta); + expect(result.status).toBe("error"); + expect(result.error).toContain("runtime error"); + }); + }); + + // --- Hostile data --- + describe("hostile data handling", () => { + it("handles hostile strings in tool call names", () => { + const trace = makeTrace({ + toolCalls: [ + { name: '">', success: true }, + { name: "normal", success: true }, + ], + }); + expect(gradeToolSuccessRate(trace)).toBe(1); + expect(gradeToolFailureCount(trace)).toBe(0); + }); + + it("handles nested malicious JSON in JSONL", () => { + const hostile = '{"a":1,"constructor":{"prototype":{"polluted":true}}}'; + const result = safeParseJsonl(hostile); + expect(result.length).toBe(1); + expect(result[0].a).toBe(1); + expect(Object.prototype).not.toHaveProperty("polluted"); + }); + + it("hostile script cannot escape sandbox", () => { + const meta = { name: "test", unit: "", direction: "", source: "inline" }; + // Attempt to access constructor chain + const result = runCustomGrader("test", "return this && this.constructor ? 0 : 1", makeTrace(), meta); + // Should not crash + expect(result.error === null || result.error !== null).toBe(true); + }); + + it("hostile script cannot use eval via string code gen", () => { + const meta = { name: "test", unit: "", direction: "", source: "inline" }; + // codeGeneration: {strings: false} prevents this + const result = runCustomGrader("test", "try { const f = new Function('return 1'); return 0 } catch(e) { return 1 }", makeTrace(), meta); + expect(result.value).toBe(1); // Should fail because code gen is disabled + }); + }); + + // --- Determinism --- + describe("determinism", () => { + it("produces identical results for identical input", () => { + const trace = makeTrace({ + toolCalls: [ + { name: "read", success: true, arguments: { path: "/a" } }, + { name: "write", success: false, arguments: { path: "/b" } }, + { name: "read", success: true, arguments: { path: "/a" } }, + ], + tokenUsageEntries: [ + { input_tokens: 100, output_tokens: 50, duration_ms: 1000 }, + { input_tokens: 200, output_tokens: 100, duration_ms: 2000 }, + ], + totalInputTokens: 300, + totalOutputTokens: 150, + totalDurationMs: 3000, + totalRequests: 2, + retryEvents: [{ event: "retry" }], + artifacts: [{ type: "pr" }], + steps: [ + { index: 0, inputTokens: 100, outputTokens: 50, durationMs: 1000, model: null }, + { index: 1, inputTokens: 200, outputTokens: 100, durationMs: 2000, model: null }, + ], + }); + + const results1 = Object.entries(BUILTIN_GRADERS).map(([id, fn]) => ({ + id, + value: fn(trace), + })); + const results2 = Object.entries(BUILTIN_GRADERS).map(([id, fn]) => ({ + id, + value: fn(trace), + })); + + expect(results1).toEqual(results2); + }); + + it("produces no timestamp in normalized output", () => { + const meta = { name: "Test", unit: "count", direction: "lower_is_better", source: "builtin" }; + const r = normalizeResult("test", 5, meta); + expect(r).not.toHaveProperty("timestamp"); + }); + }); + + // --- trace.steps and enriched fields --- + describe("preprocessTrace enrichment", () => { + const mcpLogsDir = "/tmp/gh-aw/mcp-logs"; + const gatewayPath = path.join(mcpLogsDir, "gateway.jsonl"); + const altGatewayPath = path.join(mcpLogsDir, "mcp-gateway.jsonl"); + const rpcMessagesPath = path.join(mcpLogsDir, "rpc-messages.jsonl"); + + afterEach(() => { + for (const p of [gatewayPath, altGatewayPath, rpcMessagesPath]) { + if (fs.existsSync(p)) fs.unlinkSync(p); + } + }); + + it("extracts steps from token usage entries", () => { + // Mock safeReadFile to return test data - use the preprocessTrace's logic + const trace = makeTrace({ + tokenUsageEntries: [ + { input_tokens: 100, output_tokens: 50, duration_ms: 500, model: "gpt-4" }, + { input_tokens: 200, output_tokens: 100, duration_ms: 1000, model: "gpt-4" }, + ], + steps: [ + { index: 0, inputTokens: 100, outputTokens: 50, durationMs: 500, model: "gpt-4" }, + { index: 1, inputTokens: 200, outputTokens: 100, durationMs: 1000, model: "gpt-4" }, + ], + }); + expect(trace.steps.length).toBe(2); + expect(trace.steps[0].inputTokens).toBe(100); + expect(trace.steps[1].model).toBe("gpt-4"); + }); + + it("extracts retryEvents, errorEvents", () => { + const trace = makeTrace({ + retryEvents: [{ event: "retry" }], + errorEvents: [{ level: "error", message: "fail" }], + }); + expect(trace.retryEvents.length).toBe(1); + expect(trace.errorEvents.length).toBe(1); + }); + + it("reads rpc-messages fallback and normalizes tool_name records", () => { + fs.mkdirSync(mcpLogsDir, { recursive: true }); + fs.writeFileSync(rpcMessagesPath, [JSON.stringify({ event: "rpc", tool_name: "github-mcp-server-search_code", payload: { tool_name: "github-mcp-server-search_code", arguments: { query: "foo" } } })].join("\n") + "\n", "utf8"); + + const trace = preprocessTrace(); + expect(trace.toolCalls.length).toBeGreaterThan(0); + expect(trace.toolCalls[0].name).toBe("github-mcp-server-search_code"); + expect(trace.toolCalls[0].arguments).toEqual({ query: "foo" }); + }); + }); + + // --- All built-in graders are registered --- + describe("built-in grader registry", () => { + const expectedIds = ["tool-success-rate", "tool-failure-count", "retries", "loops", "trajectory-efficiency", "execution-step-count", "execution-duration", "context-growth", "artifact-production"]; + + it("has all expected built-in graders", () => { + for (const id of expectedIds) { + expect(BUILTIN_GRADERS).toHaveProperty(id); + expect(typeof BUILTIN_GRADERS[id]).toBe("function"); + } + }); + + it("has no unexpected graders", () => { + const ids = Object.keys(BUILTIN_GRADERS); + expect(ids.sort()).toEqual([...expectedIds].sort()); + }); + + it("all graders have metadata", () => { + for (const id of expectedIds) { + expect(BUILTIN_META).toHaveProperty(id); + expect(BUILTIN_META[id].unit).toBeDefined(); + expect(BUILTIN_META[id].direction).toBeDefined(); + } + }); + }); + + // --- GRADER_VERSION --- + describe("version", () => { + it("is a number", () => { + expect(typeof GRADER_VERSION).toBe("number"); + expect(GRADER_VERSION).toBe(1); + }); + }); +}); diff --git a/actions/setup/js/trace_graders_worker.cjs b/actions/setup/js/trace_graders_worker.cjs new file mode 100644 index 00000000000..53004ef48e8 --- /dev/null +++ b/actions/setup/js/trace_graders_worker.cjs @@ -0,0 +1,149 @@ +// @ts-check + +const vm = require("vm"); + +/** + * @param {any} value + * @param {string} label + * @returns {any} + */ +function tryStructuredCloneOrUndefined(value, label) { + try { + return structuredClone(value); + } catch { + process.stderr.write(`grader worker: failed to structuredClone ${label}; value will default to {}\n`); + return undefined; + } +} + +/** + * @param {any} obj + * @returns {any} + */ +function deepFreeze(obj) { + if (obj === null || typeof obj !== "object") return obj; + Object.freeze(obj); + for (const key of Object.getOwnPropertyNames(obj)) { + const value = obj[key]; + if (value !== null && typeof value === "object" && !Object.isFrozen(value)) { + deepFreeze(value); + } + } + return obj; +} + +function readStdin() { + return new Promise(resolve => { + let data = ""; + process.stdin.setEncoding("utf-8"); + process.stdin.on("data", chunk => { + data += chunk; + }); + process.stdin.on("end", () => resolve(data)); + }); +} + +async function main() { + const raw = await readStdin(); + let payload; + try { + payload = JSON.parse(raw || "{}"); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + process.stderr.write(`invalid worker payload: ${message}\n`); + process.exit(1); + } + + try { + const trace = deepFreeze(tryStructuredCloneOrUndefined(payload.trace, "trace") ?? {}); + const config = deepFreeze(tryStructuredCloneOrUndefined(payload.config, "config") ?? {}); + const run = deepFreeze({ graderCount: Number(payload.graderCount) || 0 }); + const workflow = deepFreeze({}); + const script = String(payload.script || ""); + + const sandbox = { + trace, + run, + workflow, + config, + Date: undefined, + fetch: undefined, + require: undefined, + process: undefined, + global: undefined, + globalThis: undefined, + Function: undefined, + eval: undefined, + undefined, + NaN, + Infinity, + }; + const context = vm.createContext(sandbox, { codeGeneration: { strings: false, wasm: false } }); + const timeoutMs = Number(payload.timeoutMs) || 5000; + + const runtimeBindings = vm.runInContext( + ` + "use strict"; + (() => { + const m = {}; + const descriptors = Object.getOwnPropertyDescriptors(Math); + for (const key of Object.keys(descriptors)) { + Object.defineProperty(m, key, descriptors[key]); + } + Object.defineProperty(m, "random", { + value: undefined, + writable: false, + enumerable: true, + configurable: false + }); + const safeMath = Object.freeze(m); + const helpers = Object.freeze({ + clamp: (v, lo, hi) => safeMath.max(lo, safeMath.min(hi, v)), + ratio: (num, den) => (den === 0 ? 0 : num / den), + sum: arr => arr.reduce((a, b) => a + b, 0) + }); + return { safeMath, helpers }; + })(); + `, + context, + { timeout: 1000, filename: "grader:bootstrap" } + ); + Object.defineProperty(context, "helpers", { + value: runtimeBindings.helpers, + writable: false, + enumerable: true, + configurable: false, + }); + Object.defineProperty(context, "__math", { + value: runtimeBindings.safeMath, + writable: false, + enumerable: false, + configurable: false, + }); + + const graderFn = vm.compileFunction(`"use strict";\n${script}`, ["trace", "run", "workflow", "config", "helpers", "Math"], { + parsingContext: context, + filename: `grader:${String(payload.id || "unknown")}`, + }); + context.__grader = graderFn; + const value = vm.runInContext("__grader(trace, run, workflow, config, helpers, __math)", context, { + timeout: timeoutMs, + filename: `grader:${String(payload.id || "unknown")}:invoke`, + }); + if (typeof value === "number" && !Number.isFinite(value)) { + throw new Error("custom grader returned non-finite numeric value"); + } + + process.stdout.write(JSON.stringify({ ok: true, value })); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + process.stdout.write(JSON.stringify({ ok: false, error: message })); + } +} + +main().catch(err => { + const message = err instanceof Error ? err.stack || err.message : String(err); + process.stdout.write(JSON.stringify({ ok: false, error: message }), () => { + process.exit(1); + }); +}); diff --git a/actions/setup/sh/prepare_threat_detection_files.sh b/actions/setup/sh/prepare_threat_detection_files.sh index 044fc804f5c..d348835d5ed 100755 --- a/actions/setup/sh/prepare_threat_detection_files.sh +++ b/actions/setup/sh/prepare_threat_detection_files.sh @@ -47,5 +47,14 @@ for artifact_pattern in aw-*.patch aw-*.bundle; do done done +# Copy grader manifest and results if present (deterministic graders) +GRADER_SOURCE_DIR="${SOURCE_DIR}/agent/graders" +if [ -d "${GRADER_SOURCE_DIR}" ]; then + GRADER_DETECTION_DIR="${DETECTION_DIR}/agent/graders" + mkdir -p "${GRADER_DETECTION_DIR}" + copy_optional_file "${GRADER_SOURCE_DIR}/grader_manifest.json" "${GRADER_DETECTION_DIR}/grader_manifest.json" + copy_optional_file "${GRADER_SOURCE_DIR}/grader_results.json" "${GRADER_DETECTION_DIR}/grader_results.json" +fi + echo "Prepared threat detection files:" ls -la "${DETECTION_DIR}" diff --git a/docs/adr/54678-deterministic-trace-grading-framework.md b/docs/adr/54678-deterministic-trace-grading-framework.md new file mode 100644 index 00000000000..064605accca --- /dev/null +++ b/docs/adr/54678-deterministic-trace-grading-framework.md @@ -0,0 +1,51 @@ +# ADR-54678: Deterministic Trace Grading Framework with Isolated Custom Script Execution + +**Date**: 2026-08-22 +**Status**: Accepted +**Deciders**: gh-aw maintainers + +--- + +### Context + +The gh-aw agent job produces execution traces (token usage JSONL, MCP gateway logs, agent output JSON) that describe how an agent ran: how many LLM requests were made, how many tool calls succeeded, how many retries occurred, and how long execution took. Before this change, no systematic, deterministic mechanism existed to compute behavioral metrics from these traces. Evaluation was ad-hoc and relied on manual inspection. The system requires metrics that are byte-identical for equivalent inputs so downstream detection jobs and artifact diffs can rely on them without accounting for nondeterminism (timestamps, randomness, or LLM stochasticity). + +### Decision + +We will implement a deterministic trace grading framework (`trace_graders.cjs`) that performs a single preprocessing pass over all trace files at the start of each grading run and shares the resulting in-memory `PreprocessedTrace` object with all graders. Built-in graders are pure functions of that object. Custom (user-supplied) inline graders execute in a dedicated worker subprocess (`trace_graders_worker.cjs`) that evaluates scripts in a restricted `node:vm` context with timeout enforcement around invocation and a serialized, frozen trace/config payload. This isolates grader execution from the main grader process and prevents direct access to host globals (`process`, `require`, `fetch`, `Date`) in the main runtime. Output is written to `grader_results.json` with no timestamp field, making results deterministically byte-equivalent for identical inputs. + +### Alternatives Considered + +#### Alternative 1: LLM-Based Evaluation + +Use a secondary LLM call after the agent run to grade behavior from logs. Considered because it could handle open-ended behavioral judgements beyond simple metrics. Rejected because LLM outputs are nondeterministic (same input rarely yields byte-identical output), add significant latency and cost per run, and are unavailable in sandboxed or offline environments. Determinism is a hard requirement for artifact diffing and detection staging. + +#### Alternative 2: Per-Grader File Reads + +Have each grader independently open and parse the trace files it needs. Considered because it simplifies the grader interface (each grader is fully self-contained). Rejected because it leads to redundant I/O proportional to grader count, makes it harder to enforce consistent parsing (e.g., JSONL size limits, malformed-line handling), and complicates sandboxing of custom graders (each would need its own file-access surface). The single-pass architecture keeps parsing logic in one place and is more efficient at runtime. + +#### Alternative 3: In-Process `node:vm` Execution + +Run custom scripts directly in the main grader process with `node:vm` only. Considered because it is simpler and avoids subprocess startup overhead. Rejected because in-process execution leaves the grading controller and summary writer in the same trust boundary as untrusted script evaluation, making runtime-safety and escape-impact concerns harder to contain. We prefer an isolated subprocess boundary even with modest overhead. + +### Consequences + +#### Positive +- Grader output (`grader_results.json`) is byte-deterministic for identical trace inputs, enabling reliable artifact diffs and detection-pipeline comparisons. +- The single preprocessing pass is O(1) in file I/O regardless of grader count — adding more graders does not add more disk reads. +- The custom-grader subprocess boundary contains failures and sandbox escapes to a short-lived worker process, reducing impact on the main grading controller. +- Nine built-in graders (tool success rate, failure count, retries, loops, trajectory efficiency, step count, duration, context growth, artifact production) are available out of the box with no configuration. + +#### Negative +- The worker still uses `node:vm`; a sandbox escape could compromise the worker process. This is stronger than pure in-process execution but not equivalent to a hardened container or VM boundary. +- The frozen trace is deep-cloned via `JSON.parse(JSON.stringify(...))`, so non-JSON-serializable trace fields (Dates, Buffers, circular references) are silently dropped. Graders cannot receive richer data types without extending the preprocessing layer. +- Custom scripts are synchronous and bounded by timeout enforcement; long-running or async computations are not supported. + +#### Neutral +- Grader output files (`grader_manifest.json`, `grader_results.json`) are written to `/tmp/gh-aw/agent/graders/` and copied into the detection staging directory, making them available to downstream jobs without changes to the artifact upload structure. +- Schema registration, canonical manifest/result metadata, and threshold configuration are deferred to follow-up work; this PR establishes the runtime and built-in grader set only. +- The grader step runs as `if: always()` after the agent, integrated via `parseGradersFromFrontmatter` in the workflow compiler — enabling opt-in via YAML frontmatter without modifying the core agent job. + +--- + +*This ADR is accepted with follow-up work tracked in PR review threads for schema hardening and detection integration.* diff --git a/docs/src/content/docs/experimental/experiments-specification.md b/docs/src/content/docs/experimental/experiments-specification.md index 8958303e806..f3da210296a 100644 --- a/docs/src/content/docs/experimental/experiments-specification.md +++ b/docs/src/content/docs/experimental/experiments-specification.md @@ -227,7 +227,7 @@ The same minimum-two-variants constraint from R-SCHEMA-005 applies. |---|---|---| | `description` | string | Human-readable explanation of what the experiment tests. | | `hypothesis` | string | Null and alternative hypothesis statements. | -| `metric` | string | Primary metric name to observe (e.g., `aic`), or an eval reference (`eval:` / `evals.`) when using `evals:`. | +| `metric` | string | Primary metric name to observe (e.g., `aic`), or an eval reference (`eval:` / `evals.`) when using `evals:`, or a grader reference (`grader:` / `graders.`) when using `graders:`. | | `secondary_metrics` | string[] | Additional metrics to collect. | | `guardrail_metrics` | object[] | Thresholds that must not degrade. Each object has `name` (string), `threshold` (string or number), and optional `direction` (`"min"`\|`"max"`) (see §4.4). | | `min_samples` | integer ≥ 1 | Minimum runs per variant before analysis is reliable. Defaults to 20. | diff --git a/docs/src/content/docs/experimental/experiments.md b/docs/src/content/docs/experimental/experiments.md index d85a7284918..2e9336d1574 100644 --- a/docs/src/content/docs/experimental/experiments.md +++ b/docs/src/content/docs/experimental/experiments.md @@ -84,6 +84,9 @@ Summarize the findings in a **${{ experiments.prompt_style }}** way. When `evals` are configured, `metric` can reference an eval question ID using `eval:` (for example `eval:focused`) or `evals.`. +When `graders` are configured, `metric` can reference a grader result using +`grader:` (for example `grader:tool-success-rate`) or `graders.`. + `gh aw experiments analyze ` resolves the referenced eval question and, when eval result data is available, shows YES/NO/UNKNOWN totals for that eval-backed metric. diff --git a/docs/src/content/docs/reference/trace-graders.md b/docs/src/content/docs/reference/trace-graders.md new file mode 100644 index 00000000000..77068a9b7a6 --- /dev/null +++ b/docs/src/content/docs/reference/trace-graders.md @@ -0,0 +1,69 @@ +--- +title: Graders +description: Deterministic metrics computed from agent execution traces +--- + +Graders compute deterministic metrics from post-agent execution trace files (token usage, MCP gateway logs, agent output) without LLM calls or network access. Results are persisted in the agent artifact for downstream consumption by detection jobs and reporting tools. + +For normative requirements, see the [Graders Specification](/gh-aw/specs/graders-specification/). + +:::caution[Experimental] +Graders are an experimental feature. +::: + +## Quick start + +```yaml +graders: {} +``` + +An empty map enables all built-in graders with default settings. Omitting the `graders` field entirely disables grading (no step is emitted). + +## Built-in graders + +| ID | Description | Value | +|---|---|---| +| `tool-success-rate` | Fraction of tool calls that succeeded | 0–1 | +| `tool-failure-count` | Number of failed tool calls | integer | +| `retries` | Count of retry events in MCP gateway logs | integer | +| `loops` | Consecutive identical tool calls (same name + args) | integer | +| `trajectory-efficiency` | Unique tool names / total tool calls | 0–1 | +| `execution-step-count` | Total LLM request count | integer | +| `execution-duration` | Total execution duration (ms) | integer | +| `context-growth` | Total tokens / first-request tokens | ≥1 | +| `artifact-production` | Count of outputs in agent_output.json | integer | + +## Selective configuration + +Disable a specific built-in: + +```yaml +graders: + loops: + enabled: false +``` + +## Custom inline graders + +Add a trusted inline JavaScript expression that receives the preprocessed `trace` object: + +```yaml +graders: + bash-calls: + script: "return trace.toolCalls.filter(t => t.name === 'bash').length" +``` + +Custom scripts must return a value and stay within 4096 characters (no `require`, `import`, `fetch`, `eval`, or `process.exit`). + +## Output files + +| File | Description | +|---|---| +| `grader_manifest.json` | Which graders were configured and their enabled state | +| `grader_results.json` | Normalized metric values with trace summary | + +Both files are included in the unified `agent` artifact. + +## Execution + +The graders step runs as an `if: always()` post-agent step in the existing agent job, after log parsing and before the unified artifact upload. It uses a single preprocessing pass over trace files shared by all graders. diff --git a/docs/src/content/docs/specs/graders-specification.md b/docs/src/content/docs/specs/graders-specification.md new file mode 100644 index 00000000000..06c74f4b8cb --- /dev/null +++ b/docs/src/content/docs/specs/graders-specification.md @@ -0,0 +1,290 @@ +--- +title: Graders Specification +description: Formal specification for deterministic gh-aw graders and grader-backed experiment metrics +sidebar: + order: 1360 +--- + +# Graders Specification + +**Version**: 0.1.0 +**Status**: Draft Specification +**Feature Status**: Experimental +**Latest Version**: [graders-specification](/gh-aw/specs/graders-specification/) +**Editor**: GitHub Agentic Workflows Team + +--- + +## Abstract + +This specification defines the `graders` feature in gh-aw: deterministic, post-agent metrics computed from execution traces and persisted as structured artifacts. It specifies configuration, built-in grader behavior, custom inline grader constraints, execution ordering, artifact outputs, experiment metric references, and conformance requirements. + +## Status of This Document + +This section describes the status of this document at the time of publication. This is a draft specification and may be updated, replaced, or made obsolete by other documents at any time. + +This feature is experimental and implementations SHOULD expect iteration before final recommendation status. + +## Table of Contents + +1. [Introduction](#1-introduction) +2. [Conformance](#2-conformance) +3. [Architecture](#3-architecture) +4. [Configuration Model](#4-configuration-model) +5. [Built-in Graders](#5-built-in-graders) +6. [Custom Inline Graders](#6-custom-inline-graders) +7. [Execution and Artifacts](#7-execution-and-artifacts) +8. [Experiment Metric References](#8-experiment-metric-references) +9. [Security and Isolation](#9-security-and-isolation) +10. [Compliance Testing](#10-compliance-testing) +11. [Norms](#11-norms) +12. [References](#12-references) +13. [Change Log](#13-change-log) + +--- + +## 1. Introduction + +### 1.1 Purpose + +The `graders` feature provides deterministic quality and behavior metrics derived from workflow trace artifacts without issuing additional LLM calls. + +### 1.2 Scope + +This specification covers: + +- Frontmatter configuration under `graders` +- Built-in grader identifiers and semantics +- Custom inline grader script requirements +- Output artifact contracts +- Experiment metric integration for grader references + +This specification does NOT cover: + +- Non-deterministic evaluator models +- UI visualization requirements +- External metric backends + +### 1.3 Design Goals + +A conforming implementation: + +1. MUST compute grader values deterministically from run artifacts. +2. MUST preserve stable grader IDs for experiment references. +3. SHOULD keep grading isolated from network-dependent behavior. +4. MUST emit machine-readable grader artifacts for downstream tooling. + +--- + +## 2. Conformance + +### 2.1 Conformance Classes + +- **Conforming implementation**: Satisfies all MUST/SHALL requirements in this document. +- **Partially conforming implementation**: Supports built-in graders but omits custom inline grader execution. + +### 2.2 Requirements Notation + +The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt). + +### 2.3 Compliance Levels + +- **Level 1 (Required)**: Built-in graders, manifest/results output. +- **Level 2 (Standard)**: Custom inline graders with validation and isolation. +- **Level 3 (Complete)**: Experiment metric references to graders with validation. + +--- + +## 3. Architecture + +`graders` executes as a post-agent step in the existing `agent` job: + +1. Parse and validate frontmatter `graders`. +2. Build grader manifest and execution spec. +3. Preprocess trace artifacts once. +4. Execute enabled graders (built-in and custom inline). +5. Write normalized outputs to grader artifact files. + +The grading step MUST run with `if: always()` semantics and SHOULD continue even when individual graders fail, recording per-grader errors in results. + +--- + +## 4. Configuration Model + +### 4.1 Frontmatter Key + +The configuration key MUST be `graders`. + +### 4.2 Enable/Disable Semantics + +- If `graders` is omitted, grading MUST be disabled. +- If `graders: {}` is provided, all built-in graders MUST be enabled with defaults. +- If `graders` is present, at least one grader MUST be enabled; otherwise configuration MUST fail. + +### 4.3 Entry Model + +`graders` is a map of ` -> `. + +- Built-in grader entries MAY be `null` to enable defaults. +- Custom grader entries MUST be objects and MUST include `script`. + +Supported object fields include `enabled`, `name`, `description`, `unit`, `direction`, `threshold`, `min`, `max`, `config`, and `script`. + +--- + +## 5. Built-in Graders + +The implementation MUST recognize the following built-in grader IDs: + +- `tool-success-rate` +- `tool-failure-count` +- `retries` +- `loops` +- `trajectory-efficiency` +- `execution-step-count` +- `execution-duration` +- `context-growth` +- `artifact-production` + +These IDs are reserved for built-ins. A built-in grader MUST NOT accept a custom `script`. + +--- + +## 6. Custom Inline Graders + +A custom grader is any grader ID not in the built-in set. + +### 6.1 Required Fields + +A custom grader MUST define `script`. + +### 6.2 Script Limits + +- `script` MUST be non-empty. +- `script` MUST NOT exceed 4096 characters. + +### 6.3 Forbidden Patterns + +Inline scripts MUST be rejected if they contain any forbidden pattern, including: + +- `require(` +- `import(` +- `import ` +- `fetch(` +- `eval(` +- `process.exit` +- `child_process` +- `execSync` +- `spawnSync` +- `Function(` + +--- + +## 7. Execution and Artifacts + +### 7.1 Output Directory + +Graders output MUST be written under: + +`/tmp/gh-aw/agent/graders` + +### 7.2 Required Files + +The implementation MUST produce: + +- `grader_manifest.json` +- `grader_results.json` + +### 7.3 Artifact Inclusion + +Both files MUST be included in the unified `agent` artifact. + +### 7.4 Deterministic Output Contract + +`grader_results.json` SHOULD include normalized run/result structures suitable for downstream programmatic reads, including per-grader value/status and run-level pass/fail/error counts. + +--- + +## 8. Experiment Metric References + +Experiment metric fields MAY reference grader outputs. + +Supported forms include: + +- `grader:` +- `graders..value` + +When a grader reference is used, `` MUST resolve to a declared enabled grader. Unknown or empty grader references MUST fail validation. + +--- + +## 9. Security and Isolation + +- Grading MUST operate on local run artifacts and MUST NOT require outbound network access for built-ins. +- Custom inline graders MUST execute in a restricted context with blocked dangerous primitives. +- Implementations SHOULD enforce bounded execution time for inline scripts. +- Implementations SHOULD redact grader outputs when custom scripts are enabled to reduce secret leakage risk. + +--- + +## 10. Compliance Testing + +### 10.1 Test Suite Requirements + +- **T-GRD-001**: Omitted `graders` key disables grading step emission. +- **T-GRD-002**: `graders: {}` enables all built-ins. +- **T-GRD-003**: Unknown custom grader without `script` is rejected. +- **T-GRD-004**: Custom `script` over 4096 chars is rejected. +- **T-GRD-005**: Forbidden script patterns are rejected. +- **T-GRD-006**: Built-in grader with `script` is rejected. +- **T-GRD-007**: `grader_manifest.json` is written to required path. +- **T-GRD-008**: `grader_results.json` is written to required path. +- **T-GRD-009**: Grader files are present in `agent` artifact. +- **T-GRD-010**: `experiments.*.metric` with `grader:` validates declared enabled grader. +- **T-GRD-011**: `experiments.*.metric` with `graders..value` validates declared enabled grader. + +### 10.2 Compliance Checklist + +| Requirement | Test ID | Level | Status | +|---|---|---|---| +| Frontmatter key is `graders` | T-GRD-001 | 1 | Required | +| Empty map enables built-ins | T-GRD-002 | 1 | Required | +| Custom graders require script | T-GRD-003 | 2 | Required | +| Script safety constraints enforced | T-GRD-004, T-GRD-005 | 2 | Required | +| Required artifact files emitted | T-GRD-007, T-GRD-008 | 1 | Required | +| Experiment grader references validate | T-GRD-010, T-GRD-011 | 3 | Required | + +--- + +## 11. Norms + +- **N-GRD-001**: Implementations MUST treat `graders` as experimental. +- **N-GRD-002**: Implementations MUST preserve built-in grader ID stability across patch releases. +- **N-GRD-003**: Implementations SHOULD preserve deterministic output for identical trace inputs. +- **N-GRD-004**: Implementations MUST fail fast on invalid custom grader scripts. +- **N-GRD-005**: Implementations MUST keep grader artifact paths stable unless a major version change is issued. + +--- + +## 12. References + +### Normative References + +- **[RFC 2119]** Key words for use in RFCs to Indicate Requirement Levels. + https://www.ietf.org/rfc/rfc2119.txt + +### Informative References + +- **[Graders Reference]** [Graders](/gh-aw/reference/trace-graders/) +- **[Experiments Specification]** [Experiments Specification](/gh-aw/experimental/experiments-specification/) + +--- + +## 13. Change Log + +### Version 0.1.0 (Draft Specification) + +- Initial draft for gh-aw graders. +- Defines `graders` configuration semantics and built-in grader set. +- Defines custom inline grader constraints and forbidden patterns. +- Defines grader artifact output contract and experiment metric references. diff --git a/pkg/constants/job_constants.go b/pkg/constants/job_constants.go index 2fd11f87b09..a373f9f7ac0 100644 --- a/pkg/constants/job_constants.go +++ b/pkg/constants/job_constants.go @@ -130,6 +130,20 @@ const SafeOutputsFilename = "safeoutputs.jsonl" // consume structured token data without parsing the step summary or GITHUB_OUTPUT. const TokenUsageFilename = "agent_usage.json" +// GraderManifestFilename is the filename of the grader manifest JSON written to /tmp/gh-aw/agent/graders/ +// by trace_graders.cjs. Lists which graders were configured and their enabled state. +const GraderManifestFilename = "grader_manifest.json" + +// GraderResultsFilename is the filename of the normalized grader results JSON written to /tmp/gh-aw/agent/graders/ +// by trace_graders.cjs. Contains deterministic metric values computed from trace files. +const GraderResultsFilename = "grader_results.json" + +// GradersDir is the subdirectory under TmpGhAwAgentDir where grader output files are written. +const GradersDir = TmpGhAwDir + "/agent/graders" + +// GradersDirSlash is GradersDir with a trailing slash. +const GradersDirSlash = GradersDir + "/" + // GithubRateLimitsFilename is the filename of the GitHub API rate-limit log written to /tmp/gh-aw/. // Each line is a JSON object recording the x-ratelimit-* headers (or rate-limit API snapshot) // captured during github.rest API calls, enabling post-run analysis of rate-limit consumption. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index e1a3e393810..73b9eb742e4 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -12742,6 +12742,72 @@ "description": "Default LLM model, overridden by nested engine.model. Sets the default model used by the agentic engine for this workflow. Acts as a fallback when an engine instance does not specify its own 'model'; a nested 'engine.model' (e.g. safe-outputs.threat-detection.engine.model) takes precedence over this field for that engine instance. Supports full model IDs (e.g. 'claude-3-5-sonnet-20241022', 'gpt-5.4') and model aliases (e.g. 'small', 'large').", "examples": ["gpt-5.4", "claude-3-5-sonnet-20241022", "gpt-4"] }, + "graders": { + "description": "\u26a0\ufe0f Experimental. Deterministic graders to compute post-agent metrics from execution artifacts. Map keys are grader IDs. Built-in graders can be configured by ID; custom graders require a script.", + "type": "object", + "propertyNames": { + "pattern": "^[a-z][a-z0-9-]{0,63}$", + "description": "Grader IDs must start with a lowercase letter and contain only lowercase letters, digits, and hyphens (max 64 chars)." + }, + "additionalProperties": { + "oneOf": [ + { + "type": "null", + "description": "Enable the grader with defaults (built-ins only)." + }, + { + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean", + "description": "Whether the grader is enabled." + }, + "name": { + "type": "string", + "description": "Optional display name override." + }, + "description": { + "type": "string", + "description": "Optional grader description." + }, + "unit": { + "type": "string", + "description": "Metric unit label such as ratio, count, ms, or factor." + }, + "direction": { + "type": "string", + "enum": ["higher_is_better", "lower_is_better"], + "description": "Optimization direction for threshold pass/fail." + }, + "threshold": { + "type": "number", + "description": "Optional threshold used for pass/fail." + }, + "min": { + "type": "number", + "description": "Optional metric minimum bound." + }, + "max": { + "type": "number", + "description": "Optional metric maximum bound." + }, + "config": { + "type": "object", + "description": "Optional grader-specific config object.", + "additionalProperties": true + }, + "script": { + "type": "string", + "minLength": 1, + "maxLength": 4096, + "description": "Custom grader JavaScript script body (trusted workflows only)." + } + } + } + ] + } + }, "evals": { "description": "\u26a0\ufe0f Experimental. BinEval binary evaluation questions to run after safe-outputs and before the conclusion job. Can be a plain list of questions (shorthand) or an object with questions, model, and runs-on fields.", "oneOf": [ diff --git a/pkg/workflow/compiler_experiments.go b/pkg/workflow/compiler_experiments.go index 6766744dd11..b352ce75012 100644 --- a/pkg/workflow/compiler_experiments.go +++ b/pkg/workflow/compiler_experiments.go @@ -363,9 +363,34 @@ func ParseExperimentMetricEvalReference(metric string) (string, bool) { return "", false } +// ParseExperimentMetricGraderReference returns the referenced grader ID when metric +// declares a grader-backed metric. +// Supported forms: +// - grader: +// - graders. +// - graders.. (suffix reserved for future derived metrics) +func ParseExperimentMetricGraderReference(metric string) (string, bool) { + trimmed := strings.TrimSpace(metric) + if trimmed == "" { + return "", false + } + if rest, ok := strings.CutPrefix(trimmed, "grader:"); ok { + return strings.TrimSpace(rest), true + } + if rest, ok := strings.CutPrefix(trimmed, "graders."); ok { + rest = strings.TrimSpace(rest) + if rest == "" { + return "", true + } + parts := strings.SplitN(rest, ".", 2) + return parts[0], true + } + return "", false +} + // validateExperimentMetricReferences ensures experiment metrics that reference evals -// point to declared eval question IDs. -func validateExperimentMetricReferences(configs map[string]*ExperimentConfig, evals *EvalsConfig) error { +// or graders point to declared IDs. +func validateExperimentMetricReferences(configs map[string]*ExperimentConfig, evals *EvalsConfig, graders *GradersConfig) error { if len(configs) == 0 { return nil } @@ -378,6 +403,15 @@ func validateExperimentMetricReferences(configs map[string]*ExperimentConfig, ev } } } + graderIDs := map[string]struct{}{} + if graders != nil { + for id, def := range graders.Graders { + if id == "" || def == nil || (def.Enabled != nil && !*def.Enabled) { + continue + } + graderIDs[id] = struct{}{} + } + } for experimentName, cfg := range configs { if cfg == nil { @@ -393,17 +427,31 @@ func validateExperimentMetricReferences(configs map[string]*ExperimentConfig, ev } } referencedEvalID, referencesEval := ParseExperimentMetricEvalReference(metric) - if !referencesEval { + if referencesEval { + if referencedEvalID == "" { + return fmt.Errorf("experiments.%s.metric: expected eval reference format eval:; provide a declared eval question id", experimentName) + } + if _, ok := evalIDs[referencedEvalID]; !ok { + if len(evalIDs) == 0 { + return fmt.Errorf("experiments.%s.metric: references eval %q but no evals are declared", experimentName, referencedEvalID) + } + return fmt.Errorf("experiments.%s.metric: references unknown eval %q", experimentName, referencedEvalID) + } continue } - if referencedEvalID == "" { - return fmt.Errorf("experiments.%s.metric: expected eval reference format eval:; provide a declared eval question id", experimentName) - } - if _, ok := evalIDs[referencedEvalID]; !ok { - if len(evalIDs) == 0 { - return fmt.Errorf("experiments.%s.metric: references eval %q but no evals are declared", experimentName, referencedEvalID) + + referencedGraderID, referencesGrader := ParseExperimentMetricGraderReference(metric) + if referencesGrader { + if referencedGraderID == "" { + return fmt.Errorf("experiments.%s.metric: expected grader reference format grader:; provide a declared grader id", experimentName) } - return fmt.Errorf("experiments.%s.metric: references unknown eval %q", experimentName, referencedEvalID) + if _, ok := graderIDs[referencedGraderID]; !ok { + if len(graderIDs) == 0 { + return fmt.Errorf("experiments.%s.metric: references grader %q but no graders are declared", experimentName, referencedGraderID) + } + return fmt.Errorf("experiments.%s.metric: references unknown grader %q", experimentName, referencedGraderID) + } + continue } } diff --git a/pkg/workflow/compiler_experiments_test.go b/pkg/workflow/compiler_experiments_test.go index 8d4b5df7c50..88aa3d9acd2 100644 --- a/pkg/workflow/compiler_experiments_test.go +++ b/pkg/workflow/compiler_experiments_test.go @@ -564,11 +564,37 @@ func TestParseExperimentMetricEvalReference(t *testing.T) { } } +func TestParseExperimentMetricGraderReference(t *testing.T) { + tests := []struct { + name string + metric string + wantID string + wantMatch bool + }{ + {name: "empty metric", metric: "", wantID: "", wantMatch: false}, + {name: "normal metric", metric: "aic", wantID: "", wantMatch: false}, + {name: "grader colon format", metric: "grader:loops", wantID: "loops", wantMatch: true}, + {name: "grader dotted format", metric: "graders.loops", wantID: "loops", wantMatch: true}, + {name: "grader dotted with suffix", metric: "graders.loops.value", wantID: "loops", wantMatch: true}, + {name: "grader empty id", metric: "grader:", wantID: "", wantMatch: true}, + {name: "graders empty id", metric: "graders.", wantID: "", wantMatch: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotID, gotMatch := ParseExperimentMetricGraderReference(tt.metric) + assert.Equal(t, tt.wantID, gotID) + assert.Equal(t, tt.wantMatch, gotMatch) + }) + } +} + func TestValidateExperimentMetricReferences(t *testing.T) { tests := []struct { name string configs map[string]*ExperimentConfig evals *EvalsConfig + graders *GradersConfig wantErr string }{ { @@ -577,6 +603,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { "prompt_style": {Metric: "aic"}, }, evals: nil, + graders: nil, wantErr: "", }, { @@ -587,6 +614,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { evals: &EvalsConfig{ Questions: []EvalDefinition{{ID: "builds", Question: "Does it build?"}}, }, + graders: nil, wantErr: "", }, { @@ -597,6 +625,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { evals: &EvalsConfig{ Questions: []EvalDefinition{{ID: "builds", Question: "Does it build?"}}, }, + graders: nil, wantErr: "", }, { @@ -607,6 +636,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { evals: &EvalsConfig{ Questions: []EvalDefinition{{ID: "tests", Question: "Do tests pass?"}}, }, + graders: nil, wantErr: `references unknown eval "builds"`, }, { @@ -615,6 +645,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { "prompt_style": {Metric: "eval:builds"}, }, evals: nil, + graders: nil, wantErr: `references eval "builds" but no evals are declared`, }, { @@ -625,6 +656,7 @@ func TestValidateExperimentMetricReferences(t *testing.T) { evals: &EvalsConfig{ Questions: []EvalDefinition{{ID: "builds", Question: "Does it build?"}}, }, + graders: nil, wantErr: "expected eval reference format eval:", }, { @@ -635,13 +667,88 @@ func TestValidateExperimentMetricReferences(t *testing.T) { evals: &EvalsConfig{ Questions: []EvalDefinition{{ID: "builds", Question: "Does it build?"}}, }, + graders: nil, + wantErr: "", + }, + { + name: "grader metric references existing grader", + configs: map[string]*ExperimentConfig{ + "prompt_style": {Metric: "grader:loops"}, + }, + evals: nil, + graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "loops": {ID: "loops"}, + }, + }, + wantErr: "", + }, + { + name: "grader dotted metric references existing grader", + configs: map[string]*ExperimentConfig{ + "prompt_style": {Metric: "graders.loops.value"}, + }, + evals: nil, + graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "loops": {ID: "loops"}, + }, + }, + wantErr: "", + }, + { + name: "grader reference rejected when grader id is unknown", + configs: map[string]*ExperimentConfig{ + "prompt_style": {Metric: "grader:loops"}, + }, + evals: nil, + graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "retries": {ID: "retries"}, + }, + }, + wantErr: `references unknown grader "loops"`, + }, + { + name: "grader reference rejected when graders are not declared", + configs: map[string]*ExperimentConfig{ + "prompt_style": {Metric: "grader:loops"}, + }, + evals: nil, + graders: nil, + wantErr: `references grader "loops" but no graders are declared`, + }, + { + name: "grader reference requires non empty id", + configs: map[string]*ExperimentConfig{ + "prompt_style": {Metric: "grader:"}, + }, + evals: nil, + graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "loops": {ID: "loops"}, + }, + }, + wantErr: "expected grader reference format grader:", + }, + { + name: "grader colon metric trims whitespace from id", + configs: map[string]*ExperimentConfig{ + "prompt_style": {Metric: "grader: loops "}, + }, + evals: nil, + graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "loops": {ID: "loops"}, + }, + }, wantErr: "", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateExperimentMetricReferences(tt.configs, tt.evals) + err := validateExperimentMetricReferences(tt.configs, tt.evals, tt.graders) if tt.wantErr == "" { assert.NoError(t, err) return diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index c4680876f59..2b1f234df15 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -567,7 +567,14 @@ func (c *Compiler) extractAdditionalConfigurations( return fmt.Errorf("invalid evals configuration: %w", err) } workflowData.Evals = evalsConfig - if err := validateExperimentMetricReferences(workflowData.ExperimentConfigs, workflowData.Evals); err != nil { + + // Extract deterministic graders configuration. + gradersConfig, err := c.parseGradersFromFrontmatter(frontmatter) + if err != nil { + return fmt.Errorf("invalid graders configuration: %w", err) + } + workflowData.Graders = gradersConfig + if err := validateExperimentMetricReferences(workflowData.ExperimentConfigs, workflowData.Evals, workflowData.Graders); err != nil { return fmt.Errorf("invalid experiments configuration: %w", err) } diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index 0ae0be49eac..a65acf1e159 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -370,6 +370,7 @@ func (c *Compiler) emitExperimentalFeatureWarnings(workflowData *WorkflowData) { message string }{ {enabled: workflowData.RateLimit != nil, message: "Using experimental feature: rate limiting"}, + {enabled: workflowData.Graders != nil && workflowData.Graders.HasGraders(), message: "Using experimental feature: graders"}, {enabled: workflowData.SafeOutputs != nil && workflowData.SafeOutputs.DispatchRepository != nil, message: "Using experimental feature: dispatch-repository"}, {enabled: workflowData.SafeOutputs != nil && workflowData.SafeOutputs.MergePullRequest != nil, message: "Using experimental feature: merge-pull-request"}, {enabled: workflowData.SafeOutputs != nil && workflowData.SafeOutputs.ApproveWorkflowRun != nil, message: "Using experimental feature: approve-workflow-run"}, diff --git a/pkg/workflow/compiler_yaml_artifacts.go b/pkg/workflow/compiler_yaml_artifacts.go index 4c8193a39e1..2f91bb04eb1 100644 --- a/pkg/workflow/compiler_yaml_artifacts.go +++ b/pkg/workflow/compiler_yaml_artifacts.go @@ -75,6 +75,15 @@ func (c *Compiler) generateAgentOutputFallbackUpload(yaml *strings.Builder, data constants.TmpGhAwDirSlash + constants.SafeOutputsFilename, } + // Include grader manifest/results in the fallback so detection and downstream + // jobs have reliable access even when the large unified artifact times out. + if data.Graders != nil && data.Graders.HasGraders() { + paths = append(paths, + constants.GradersDirSlash+constants.GraderManifestFilename, + constants.GradersDirSlash+constants.GraderResultsFilename, + ) + } + c.stepOrderTracker.RecordArtifactUpload("Upload agent output fallback artifact", paths) yaml.WriteString(" # Small dedicated copy of the agent output so safe-output processing\n") diff --git a/pkg/workflow/compiler_yaml_graders.go b/pkg/workflow/compiler_yaml_graders.go new file mode 100644 index 00000000000..9002fcffae4 --- /dev/null +++ b/pkg/workflow/compiler_yaml_graders.go @@ -0,0 +1,215 @@ +package workflow + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/logger" +) + +var compilerYamlGradersLog = logger.New("workflow:compiler_yaml_graders") + +// generateGradersStep emits an always() post-agent step that runs deterministic +// graders. The step executes after secret redaction / summary steps and before +// the unified artifact upload so results are included in the agent artifact. +// +// The step is only emitted when graders are configured (graders: in frontmatter). +func (c *Compiler) generateGradersStep(yaml *strings.Builder, data *WorkflowData) { + if data.Graders == nil || !data.Graders.HasGraders() { + return + } + + compilerYamlGradersLog.Printf("Generating graders step with %d enabled graders", len(data.Graders.EnabledGraderIDs())) + + // Build the manifest JSON that the JS runtime will consume. + manifest := buildGraderManifest(data.Graders) + manifestJSON, err := json.Marshal(manifest) + if err != nil { + compilerYamlGradersLog.Printf("Failed to marshal grader manifest: %v", err) + return + } + manifestB64 := base64.StdEncoding.EncodeToString(manifestJSON) + + // Build execution spec (scripts) separately, base64 encoded for safety. + execSpec := buildGraderExecSpec(data.Graders) + execJSON, err := json.Marshal(execSpec) + if err != nil { + compilerYamlGradersLog.Printf("Failed to marshal grader exec spec: %v", err) + return + } + execB64 := base64.StdEncoding.EncodeToString(execJSON) + + yaml.WriteString(" - name: Run graders\n") + yaml.WriteString(" if: always()\n") + yaml.WriteString(" continue-on-error: true\n") + fmt.Fprintf(yaml, " uses: %s\n", getCachedActionPin("actions/github-script", data)) + yaml.WriteString(" with:\n") + yaml.WriteString(" script: |\n") + yaml.WriteString(" const { setupGlobals } = require('" + SetupActionDestination + "/setup_globals.cjs');\n") + yaml.WriteString(" setupGlobals(core, github, context, exec, io, getOctokit);\n") + yaml.WriteString(" const { main } = require('" + SetupActionDestination + "/trace_graders.cjs');\n") + fmt.Fprintf(yaml, " await main('%s', '%s');\n", manifestB64, execB64) + + compilerYamlGradersLog.Print("Generated graders step") +} + +// graderManifestEntry represents a single grader in the serialized manifest. +// The manifest is an object {version:1, graders:[...]} for stable schema. +type graderManifestEntry struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description,omitempty"` + Source string `json:"source"` // "builtin" or "inline" + Enabled bool `json:"enabled"` + Unit string `json:"unit,omitempty"` + Direction string `json:"direction,omitempty"` + Threshold *float64 `json:"threshold,omitempty"` + Max *float64 `json:"max,omitempty"` + Min *float64 `json:"min,omitempty"` + Digest string `json:"digest,omitempty"` // SHA-256 of inline script + Config map[string]any `json:"config,omitempty"` +} + +// graderManifest is the top-level manifest object written to disk. +type graderManifest struct { + Version int `json:"version"` + Graders []graderManifestEntry `json:"graders"` +} + +// graderExecEntry carries the script body for a custom grader, keyed by ID. +type graderExecEntry struct { + ID string `json:"id"` + Script string `json:"script"` +} + +// buildGraderManifest constructs the manifest for the JS runtime. +func buildGraderManifest(cfg *GradersConfig) *graderManifest { + if cfg == nil { + return &graderManifest{Version: 1} + } + + builtinSet := make(map[string]struct{}, len(BuiltinGraderIDs)) + for _, id := range BuiltinGraderIDs { + builtinSet[id] = struct{}{} + } + + ids := cfg.EnabledGraderIDs() + // Also include disabled graders so the manifest records them + var disabledIDs []string + for id, g := range cfg.Graders { + if g.Enabled != nil && !*g.Enabled { + disabledIDs = append(disabledIDs, id) + } + } + sort.Strings(disabledIDs) + + entries := make([]graderManifestEntry, 0, len(ids)+len(disabledIDs)) + + addEntry := func(id string, enabled bool) { + g := cfg.Graders[id] + source := "builtin" + if _, ok := builtinSet[id]; !ok { + source = "inline" + } + name := g.Name + if name == "" { + name = id + } + entries = append(entries, graderManifestEntry{ + ID: id, + Name: name, + Description: g.Description, + Source: source, + Enabled: enabled, + Unit: g.Unit, + Direction: g.Direction, + Threshold: g.Threshold, + Max: g.Max, + Min: g.Min, + Digest: g.ScriptDigest(), + Config: g.Config, + }) + } + + for _, id := range ids { + addEntry(id, true) + } + for _, id := range disabledIDs { + addEntry(id, false) + } + + return &graderManifest{Version: 1, Graders: entries} +} + +// buildGraderExecSpec builds the execution spec: an array of {id, script} +// entries for custom graders only. This is base64-encoded to avoid JS/YAML injection. +func buildGraderExecSpec(cfg *GradersConfig) []graderExecEntry { + if cfg == nil { + return nil + } + builtinSet := make(map[string]struct{}, len(BuiltinGraderIDs)) + for _, id := range BuiltinGraderIDs { + builtinSet[id] = struct{}{} + } + + var specs []graderExecEntry + for _, id := range cfg.EnabledGraderIDs() { + g := cfg.Graders[id] + if _, ok := builtinSet[id]; !ok && g.Script != "" { + specs = append(specs, graderExecEntry{ID: id, Script: g.Script}) + } + } + return specs +} + +// generateGraderRedactionStep emits a lightweight redaction pass that scans grader +// output files for leaked secrets. Custom grader scripts can evaluate trace data +// that may contain credential-bearing strings. This step runs after the graders step +// and reuses the existing redact_secrets.cjs infrastructure. +func (c *Compiler) generateGraderRedactionStep(yaml *strings.Builder, yamlContent string, data *WorkflowData) { + if data.Graders == nil || !data.Graders.HasGraders() { + return + } + if !data.Graders.HasCustomScripts() { + return + } + + secretReferences := CollectSecretReferences(yamlContent) + c.stepOrderTracker.RecordSecretRedaction("Redact grader outputs") + + yaml.WriteString(" - name: Redact grader outputs\n") + yaml.WriteString(" if: always()\n") + yaml.WriteString(" continue-on-error: true\n") + fmt.Fprintf(yaml, " uses: %s\n", getCachedActionPin("actions/github-script", data)) + yaml.WriteString(" with:\n") + yaml.WriteString(" script: |\n") + yaml.WriteString(" const { setupGlobals } = require('" + SetupActionDestination + "/setup_globals.cjs');\n") + yaml.WriteString(" setupGlobals(core, github, context, exec, io, getOctokit);\n") + yaml.WriteString(" const { redactFilesInDir } = require('" + SetupActionDestination + "/redact_secrets.cjs');\n") + fmt.Fprintf(yaml, " await redactFilesInDir('%s');\n", constants.GradersDir) + + if len(secretReferences) > 0 { + yaml.WriteString(" env:\n") + escapedRefs := make([]string, len(secretReferences)) + for i, ref := range secretReferences { + escapedRefs[i] = escapeSingleQuoteBackslash(ref) + } + fmt.Fprintf(yaml, " GH_AW_SECRET_NAMES: '%s'\n", strings.Join(escapedRefs, ",")) + for _, secretName := range secretReferences { + escapedSecretName := escapeSingleQuoteBackslash(secretName) + fmt.Fprintf(yaml, " SECRET_%s: ${{ secrets.%s }}\n", escapedSecretName, secretName) + } + } +} + +// collectGraderArtifactPaths returns artifact paths for grader output files. +func collectGraderArtifactPaths() []string { + return []string{ + constants.GradersDirSlash + constants.GraderManifestFilename, + constants.GradersDirSlash + constants.GraderResultsFilename, + } +} diff --git a/pkg/workflow/compiler_yaml_post_agent.go b/pkg/workflow/compiler_yaml_post_agent.go index 8b114b9669f..93ed5dfeef2 100644 --- a/pkg/workflow/compiler_yaml_post_agent.go +++ b/pkg/workflow/compiler_yaml_post_agent.go @@ -60,6 +60,11 @@ func (c *Compiler) collectArtifactPaths(data *WorkflowData, engine CodingAgentEn paths = append(paths, constants.TmpGhAwDirSlash+constants.OtlpExportErrorsFilename) } + // Collect grader manifest and results when graders are configured. + if data.Graders != nil && data.Graders.HasGraders() { + paths = append(paths, collectGraderArtifactPaths()...) + } + // Collect safe outputs and agent output paths for the unified artifact. // These were previously uploaded as separate safe-output and agent-output artifacts. if data.SafeOutputs != nil { @@ -192,6 +197,14 @@ func (c *Compiler) generatePostAgentCollectionAndUpload(yaml *strings.Builder, d // Emit all GITHUB_STEP_SUMMARY log-parsing steps. c.generateSummarySteps(yaml, data, engine) + // Run deterministic graders after trace data is available. + c.generateGradersStep(yaml, data) + + // Re-scan grader output files for leaked secrets when custom grader scripts + // are present. Custom scripts evaluate trace data that may contain + // credential-bearing strings written after the initial workspace redaction. + c.generateGraderRedactionStep(yaml, yaml.String(), data) + // Write a minimal agent_output.json placeholder when the engine fails before // producing any safe outputs, so downstream safe_outputs and conclusion jobs // receive a valid (empty) JSON file instead of an ENOENT error. diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index 5fbb778ee7f..52ff34a5808 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -452,6 +452,11 @@ type FrontmatterConfig struct { // engine-config / runs-on overrides. Evals any `json:"evals,omitempty"` + // Graders configures deterministic graders that compute metrics from + // post-agent trace files. Can be {} for zero-config (all built-ins) or a map + // of grader IDs with optional enabled/script overrides. + Graders any `json:"graders,omitempty"` + // ExcludedEnv lists additional environment variable names that must be excluded from // the agent container via AWF's --exclude-env flag. Use this when an env var is set // from a source that the compiler cannot automatically detect as credential-bearing diff --git a/pkg/workflow/graders_config.go b/pkg/workflow/graders_config.go new file mode 100644 index 00000000000..4c3eb2444a3 --- /dev/null +++ b/pkg/workflow/graders_config.go @@ -0,0 +1,424 @@ +// Package workflow - Deterministic graders configuration types and parser. +package workflow + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "regexp" + "sort" + "strings" + "unicode/utf8" + + "github.com/github/gh-aw/pkg/logger" +) + +var gradersConfigLog = logger.New("workflow:graders_config") + +// BuiltinGraderMeta describes a built-in deterministic grader with its default metadata. +type BuiltinGraderMeta struct { + ID string + Name string + Description string + Unit string + Direction string // "higher_is_better" | "lower_is_better" + Threshold *float64 + Max *float64 + Min *float64 +} + +// BuiltinGraderRegistry is the ordered list of all built-in grader definitions. +var BuiltinGraderRegistry = []BuiltinGraderMeta{ + {ID: "tool-success-rate", Name: "Tool Success Rate", Description: "Fraction of tool calls that succeeded", Unit: "ratio", Direction: "higher_is_better", Threshold: new(0.8), Min: new(0.0), Max: new(1.0)}, + {ID: "tool-failure-count", Name: "Tool Failure Count", Description: "Number of tool calls that failed", Unit: "count", Direction: "lower_is_better", Threshold: new(5.0)}, + {ID: "retries", Name: "Retries", Description: "Number of retry events detected in gateway logs", Unit: "count", Direction: "lower_is_better", Threshold: new(10.0)}, + {ID: "loops", Name: "Loops", Description: "Consecutive identical tool calls (same name and arguments)", Unit: "count", Direction: "lower_is_better", Threshold: new(3.0)}, + {ID: "trajectory-efficiency", Name: "Trajectory Efficiency", Description: "Ratio of unique tool names to total tool calls (higher = more diverse usage)", Unit: "ratio", Direction: "higher_is_better", Min: new(0.0), Max: new(1.0)}, + {ID: "execution-step-count", Name: "Execution Step Count", Description: "Total LLM request count", Unit: "count", Direction: "lower_is_better"}, + {ID: "execution-duration", Name: "Execution Duration", Description: "Total execution duration", Unit: "ms", Direction: "lower_is_better"}, + {ID: "context-growth", Name: "Context Growth", Description: "Ratio of total tokens to first-request tokens", Unit: "factor", Direction: "lower_is_better"}, + {ID: "artifact-production", Name: "Artifact Production", Description: "Count of outputs/artifacts produced by the agent", Unit: "count", Direction: "higher_is_better"}, +} + +// BuiltinGraderIDs is the ordered list of built-in grader IDs (derived from registry). +var BuiltinGraderIDs = func() []string { + ids := make([]string, len(BuiltinGraderRegistry)) + for i, m := range BuiltinGraderRegistry { + ids[i] = m.ID + } + return ids +}() + +// builtinGraderMetaByID is a lookup map for BuiltinGraderRegistry. +var builtinGraderMetaByID = func() map[string]*BuiltinGraderMeta { + m := make(map[string]*BuiltinGraderMeta, len(BuiltinGraderRegistry)) + for i := range BuiltinGraderRegistry { + m[BuiltinGraderRegistry[i].ID] = &BuiltinGraderRegistry[i] + } + return m +}() + +// GraderDefinition represents a single grader entry in the graders map. +type GraderDefinition struct { + ID string // grader identifier (must be unique) + Enabled *bool // explicit enable/disable; nil means use default (true for built-ins) + Name string // human-readable name (defaults from registry for built-ins) + Description string // description of the metric + Unit string // e.g. "ratio", "count", "ms", "factor" + Direction string // "higher_is_better" or "lower_is_better" + Threshold *float64 // quality threshold (pass/fail boundary) + Max *float64 // theoretical maximum + Min *float64 // theoretical minimum + Script string // inline JS body for trusted custom graders (built-ins leave empty) + Config map[string]any // arbitrary config passed to grader at runtime +} + +// ScriptDigest returns the SHA-256 hex digest of the script, or "" if no script. +func (g *GraderDefinition) ScriptDigest() string { + if g.Script == "" { + return "" + } + h := sha256.Sum256([]byte(g.Script)) + return hex.EncodeToString(h[:]) +} + +// GradersConfig holds the configuration for deterministic graders declared +// in workflow frontmatter. Graders run as an always() post-agent step in the agent job. +type GradersConfig struct { + // Graders is the map of grader ID to definition. + Graders map[string]*GraderDefinition +} + +// HasGraders returns true when the config contains at least one enabled grader. +func (gc *GradersConfig) HasGraders() bool { + if gc == nil { + return false + } + for _, g := range gc.Graders { + if g.Enabled == nil || *g.Enabled { + return true + } + } + return false +} + +// HasCustomScripts returns true if any enabled grader has a custom script. +func (gc *GradersConfig) HasCustomScripts() bool { + if gc == nil { + return false + } + for _, g := range gc.Graders { + if (g.Enabled == nil || *g.Enabled) && g.Script != "" { + return true + } + } + return false +} + +// EnabledGraderIDs returns the sorted list of enabled grader IDs. +func (gc *GradersConfig) EnabledGraderIDs() []string { + if gc == nil { + return nil + } + enabledSet := make(map[string]struct{}) + for id, g := range gc.Graders { + if g.Enabled == nil || *g.Enabled { + enabledSet[id] = struct{}{} + } + } + // Stable order: built-ins first in canonical order, then custom sorted + var result []string + builtinSet := make(map[string]struct{}, len(BuiltinGraderIDs)) + for _, bid := range BuiltinGraderIDs { + builtinSet[bid] = struct{}{} + if _, ok := enabledSet[bid]; ok { + result = append(result, bid) + } + } + var custom []string + for id := range enabledSet { + if _, ok := builtinSet[id]; !ok { + custom = append(custom, id) + } + } + sort.Strings(custom) + result = append(result, custom...) + return result +} + +// graderIDPattern validates grader IDs: lowercase alphanumeric + hyphens, 1-64 chars. +var graderIDPattern = regexp.MustCompile(`^[a-z][a-z0-9-]{0,63}$`) + +// parseGradersFromFrontmatter extracts and validates the graders configuration from the +// raw frontmatter map. Returns nil when the graders field is absent. +// +// Supported forms: +// +// # Zero-config: all built-ins enabled +// graders: {} +// +// # Selective disable +// graders: +// loops: +// enabled: false +// +// # Built-in with threshold override +// graders: +// tool-success-rate: +// threshold: 0.95 +// +// # Custom inline grader +// graders: +// my-metric: +// script: | +// return { value: trace.toolCalls.length } +// unit: count +// direction: lower_is_better +func (c *Compiler) parseGradersFromFrontmatter(frontmatter map[string]any) (*GradersConfig, error) { + raw, exists := frontmatter["graders"] + if !exists || raw == nil { + return nil, nil + } + + cfg := &GradersConfig{ + Graders: make(map[string]*GraderDefinition), + } + + m, ok := raw.(map[string]any) + if !ok { + return nil, errors.New("graders must be a map of grader IDs to configuration objects (or {} for all built-in defaults). Example:\ngraders:\n tool-success-rate:\n enabled: true") + } + + builtinSet := make(map[string]struct{}, len(BuiltinGraderIDs)) + for _, id := range BuiltinGraderIDs { + builtinSet[id] = struct{}{} + } + + // If empty map {}, populate all built-ins with defaults + if len(m) == 0 { + for _, id := range BuiltinGraderIDs { + meta := builtinGraderMetaByID[id] + cfg.Graders[id] = builtinDefFromMeta(meta) + } + gradersConfigLog.Printf("Parsed graders config: zero-config with %d built-in graders", len(BuiltinGraderIDs)) + return cfg, nil + } + + // Parse explicit entries + seenNormalizedIDs := make(map[string]string, len(m)) + for id, entryRaw := range m { + rawID := id + id = strings.TrimSpace(id) + if existingRawID, exists := seenNormalizedIDs[id]; exists { + return nil, fmt.Errorf("graders has duplicate id %q after normalization. Remove whitespace variants (for example %q and %q)", id, existingRawID, rawID) + } + seenNormalizedIDs[id] = rawID + if !graderIDPattern.MatchString(id) { + return nil, fmt.Errorf("graders has invalid id %q: must match %s. Example:\ngraders:\n my-metric:\n script: \"return { value: trace.toolCalls.length }\"", id, graderIDPattern.String()) + } + + def := &GraderDefinition{ID: id} + + // Apply built-in defaults if this is a built-in + if meta, ok := builtinGraderMetaByID[id]; ok { + def = builtinDefFromMeta(meta) + } + + _, isBuiltin := builtinSet[id] + if entryRaw == nil { + if !isBuiltin { + return nil, fmt.Errorf("graders.%s is not a built-in grader and requires a 'script' field. Built-in graders: %s", id, strings.Join(BuiltinGraderIDs, ", ")) + } + cfg.Graders[id] = def + continue + } + + entry, ok := entryRaw.(map[string]any) + if !ok { + return nil, fmt.Errorf("graders.%s must be a map or null, got %T. Example:\ngraders:\n %s:\n enabled: true", id, entryRaw, id) + } + + if err := parseGraderEntryFields(def, entry, id, isBuiltin); err != nil { + return nil, err + } + + // Custom graders must have a script + if !isBuiltin && def.Script == "" && (def.Enabled == nil || *def.Enabled) { + return nil, fmt.Errorf("graders.%s is not a built-in grader and requires a 'script' field. Built-in graders: %s", id, strings.Join(BuiltinGraderIDs, ", ")) + } + + cfg.Graders[id] = def + } + + // Add missing built-ins as defaults when at least one built-in is explicitly listed + hasAnyBuiltin := false + for id := range cfg.Graders { + if _, ok := builtinSet[id]; ok { + hasAnyBuiltin = true + break + } + } + if hasAnyBuiltin { + for _, id := range BuiltinGraderIDs { + if _, exists := cfg.Graders[id]; !exists { + meta := builtinGraderMetaByID[id] + cfg.Graders[id] = builtinDefFromMeta(meta) + } + } + } + + if err := validateGraders(cfg); err != nil { + return nil, err + } + + enabledCount := len(cfg.EnabledGraderIDs()) + gradersConfigLog.Printf("Parsed %d grader definitions (%d enabled)", len(cfg.Graders), enabledCount) + return cfg, nil +} + +// builtinDefFromMeta creates a GraderDefinition from built-in metadata. +func builtinDefFromMeta(meta *BuiltinGraderMeta) *GraderDefinition { + def := &GraderDefinition{ + ID: meta.ID, + Name: meta.Name, + Description: meta.Description, + Unit: meta.Unit, + Direction: meta.Direction, + } + if meta.Threshold != nil { + v := *meta.Threshold + def.Threshold = &v + } + if meta.Max != nil { + v := *meta.Max + def.Max = &v + } + if meta.Min != nil { + v := *meta.Min + def.Min = &v + } + return def +} + +// parseGraderEntryFields parses individual fields from a grader entry map into the definition. +func parseGraderEntryFields(def *GraderDefinition, entry map[string]any, id string, isBuiltin bool) error { + if v, ok := entry["enabled"]; ok { + b, ok := v.(bool) + if !ok { + return fmt.Errorf("graders.%s.enabled must be a boolean, got %T", id, v) + } + def.Enabled = &b + } + + if v, ok := entry["name"]; ok { + s, ok := v.(string) + if !ok { + return fmt.Errorf("graders.%s.name must be a string, got %T", id, v) + } + def.Name = s + } + if v, ok := entry["description"]; ok { + s, ok := v.(string) + if !ok { + return fmt.Errorf("graders.%s.description must be a string, got %T", id, v) + } + def.Description = s + } + if v, ok := entry["unit"]; ok { + s, ok := v.(string) + if !ok { + return fmt.Errorf("graders.%s.unit must be a string, got %T", id, v) + } + def.Unit = s + } + if v, ok := entry["direction"]; ok { + s, ok := v.(string) + if !ok { + return fmt.Errorf("graders.%s.direction must be a string, got %T", id, v) + } + if s != "higher_is_better" && s != "lower_is_better" { + return fmt.Errorf("graders.%s.direction must be 'higher_is_better' or 'lower_is_better', got %q", id, s) + } + def.Direction = s + } + if err := parseOptionalFloat(entry, "threshold", id, &def.Threshold); err != nil { + return err + } + if err := parseOptionalFloat(entry, "max", id, &def.Max); err != nil { + return err + } + if err := parseOptionalFloat(entry, "min", id, &def.Min); err != nil { + return err + } + + if v, ok := entry["config"]; ok { + m, ok := v.(map[string]any) + if !ok { + return fmt.Errorf("graders.%s.config must be an object, got %T", id, v) + } + def.Config = m + } + + if scriptRaw, ok := entry["script"]; ok { + s, ok := scriptRaw.(string) + if !ok { + return fmt.Errorf("graders.%s.script must be a string, got %T", id, scriptRaw) + } + s = strings.TrimSpace(s) + if s == "" { + return fmt.Errorf("graders.%s.script must be non-empty when specified", id) + } + if isBuiltin { + return fmt.Errorf("graders.%s is a built-in grader and cannot have a custom script", id) + } + scriptCharCount := utf8.RuneCountInString(s) + if scriptCharCount > 4096 { + return fmt.Errorf("graders.%s.script exceeds maximum length of 4096 characters (%d)", id, scriptCharCount) + } + forbiddenPatterns := []string{"require(", "import(", "import ", "fetch(", "eval(", "process.exit", "child_process", "execSync", "spawnSync", "Function("} + for _, p := range forbiddenPatterns { + if strings.Contains(s, p) { + return fmt.Errorf("graders.%s.script contains forbidden pattern %q — inline grader scripts must be pure functions without side effects", id, p) + } + } + def.Script = s + } + + return nil +} + +// parseOptionalFloat parses an optional float64 field from a map. +func parseOptionalFloat(m map[string]any, key string, graderID string, target **float64) error { + v, ok := m[key] + if !ok { + return nil + } + switch n := v.(type) { + case float64: + *target = &n + case int: + f := float64(n) + *target = &f + default: + return fmt.Errorf("graders.%s.%s must be a number, got %T", graderID, key, v) + } + return nil +} + +// validateGraders checks invariants after parsing. +func validateGraders(cfg *GradersConfig) error { + if cfg == nil { + return nil + } + if !cfg.HasGraders() { + return errors.New("graders configuration has no enabled graders. Remove the graders field to disable grading, or set enabled: true on at least one grader") + } + return nil +} + +// ParseGradersFromFrontmatter is a public standalone convenience wrapper. +func ParseGradersFromFrontmatter(frontmatter map[string]any) (*GradersConfig, error) { + var c Compiler + return c.parseGradersFromFrontmatter(frontmatter) +} diff --git a/pkg/workflow/graders_config_test.go b/pkg/workflow/graders_config_test.go new file mode 100644 index 00000000000..ace2ab91d4e --- /dev/null +++ b/pkg/workflow/graders_config_test.go @@ -0,0 +1,455 @@ +package workflow + +import ( + "encoding/json" + "strings" + "testing" +) + +// TestParseGradersFromFrontmatter_Absent verifies nil return when graders absent. +func TestParseGradersFromFrontmatter_Absent(t *testing.T) { + var c Compiler + cfg, err := c.parseGradersFromFrontmatter(map[string]any{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg != nil { + t.Fatal("expected nil config when graders absent") + } +} + +// TestParseGradersFromFrontmatter_Nil verifies nil return when graders is nil. +func TestParseGradersFromFrontmatter_Nil(t *testing.T) { + var c Compiler + cfg, err := c.parseGradersFromFrontmatter(map[string]any{"graders": nil}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg != nil { + t.Fatal("expected nil config when graders is nil") + } +} + +// TestParseGradersFromFrontmatter_ZeroConfig verifies {} enables all built-ins. +func TestParseGradersFromFrontmatter_ZeroConfig(t *testing.T) { + var c Compiler + cfg, err := c.parseGradersFromFrontmatter(map[string]any{"graders": map[string]any{}}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil config") + } + if !cfg.HasGraders() { + t.Fatal("expected HasGraders to be true") + } + ids := cfg.EnabledGraderIDs() + if len(ids) != len(BuiltinGraderIDs) { + t.Fatalf("expected %d enabled graders, got %d", len(BuiltinGraderIDs), len(ids)) + } +} + +// TestParseGradersFromFrontmatter_DisableOne verifies selective disable. +func TestParseGradersFromFrontmatter_DisableOne(t *testing.T) { + var c Compiler + cfg, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "loops": map[string]any{"enabled": false}, + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil config") + } + ids := cfg.EnabledGraderIDs() + for _, id := range ids { + if id == "loops" { + t.Fatal("loops should be disabled") + } + } + // Should have all built-ins minus loops + if len(ids) != len(BuiltinGraderIDs)-1 { + t.Fatalf("expected %d enabled graders, got %d", len(BuiltinGraderIDs)-1, len(ids)) + } +} + +// TestParseGradersFromFrontmatter_CustomGrader verifies custom grader with script. +func TestParseGradersFromFrontmatter_CustomGrader(t *testing.T) { + var c Compiler + cfg, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "my-metric": map[string]any{ + "script": "trace.toolCalls.length", + }, + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil config") + } + g, ok := cfg.Graders["my-metric"] + if !ok { + t.Fatal("expected my-metric grader") + } + if g.Script != "trace.toolCalls.length" { + t.Fatalf("unexpected script: %s", g.Script) + } +} + +// TestParseGradersFromFrontmatter_InvalidType verifies error for wrong type. +func TestParseGradersFromFrontmatter_InvalidType(t *testing.T) { + var c Compiler + _, err := c.parseGradersFromFrontmatter(map[string]any{"graders": "invalid"}) + if err == nil { + t.Fatal("expected error for string graders value") + } +} + +// TestParseGradersFromFrontmatter_ForbiddenScript verifies forbidden patterns in scripts. +func TestParseGradersFromFrontmatter_ForbiddenScript(t *testing.T) { + var c Compiler + forbidden := []string{ + "require('fs')", + "import('os')", + "fetch('http://evil.com')", + "eval('bad')", + "process.exit(1)", + } + for _, script := range forbidden { + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "bad-grader": map[string]any{"script": script}, + }, + }) + if err == nil { + t.Fatalf("expected error for forbidden script: %s", script) + } + if !strings.Contains(err.Error(), "forbidden pattern") { + t.Fatalf("expected forbidden pattern error, got: %v", err) + } + } +} + +// TestParseGradersFromFrontmatter_BuiltinScriptRejected verifies built-in cannot have script. +func TestParseGradersFromFrontmatter_BuiltinScriptRejected(t *testing.T) { + var c Compiler + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "retries": map[string]any{"script": "1 + 1"}, + }, + }) + if err == nil { + t.Fatal("expected error for built-in with script") + } +} + +// TestParseGradersFromFrontmatter_CustomWithoutScript verifies custom requires script. +func TestParseGradersFromFrontmatter_CustomWithoutScript(t *testing.T) { + var c Compiler + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "no-script": map[string]any{}, + }, + }) + if err == nil { + t.Fatal("expected error for custom grader without script") + } +} + +// TestParseGradersFromFrontmatter_CustomNullRejected verifies null custom graders are rejected at parse time. +func TestParseGradersFromFrontmatter_CustomNullRejected(t *testing.T) { + var c Compiler + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "my-custom": nil, + }, + }) + if err == nil { + t.Fatal("expected error for null custom grader") + } + if !strings.Contains(err.Error(), "requires a 'script' field") { + t.Fatalf("expected missing-script error, got: %v", err) + } +} + +// TestParseGradersFromFrontmatter_ScriptLengthUsesCharacters verifies limits align to character count. +func TestParseGradersFromFrontmatter_ScriptLengthUsesCharacters(t *testing.T) { + var c Compiler + validMultibyteScript := "return '" + strings.Repeat("é", 1366) + "'.length" + if _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "unicode-ok": map[string]any{"script": validMultibyteScript}, + }, + }); err != nil { + t.Fatalf("expected multibyte script under 4096 characters to pass, got: %v", err) + } + + tooLongScript := "return '" + strings.Repeat("a", 4097) + "'.length" + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "too-long": map[string]any{"script": tooLongScript}, + }, + }) + if err == nil { + t.Fatal("expected error for script longer than 4096 characters") + } + if !strings.Contains(err.Error(), "maximum length of 4096 characters") { + t.Fatalf("expected script-length error, got: %v", err) + } +} + +// TestParseGradersFromFrontmatter_InvalidID verifies ID validation. +func TestParseGradersFromFrontmatter_InvalidID(t *testing.T) { + var c Compiler + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "UPPER_CASE": map[string]any{"script": "1"}, + }, + }) + if err == nil { + t.Fatal("expected error for invalid ID") + } +} + +// TestParseGradersFromFrontmatter_DuplicateNormalizedID verifies whitespace variants are rejected. +func TestParseGradersFromFrontmatter_DuplicateNormalizedID(t *testing.T) { + var c Compiler + _, err := c.parseGradersFromFrontmatter(map[string]any{ + "graders": map[string]any{ + "retry-test": map[string]any{"script": "return 1"}, + " retry-test": map[string]any{"script": "return 2"}, + }, + }) + if err == nil { + t.Fatal("expected duplicate normalized id error") + } + if !strings.Contains(err.Error(), "duplicate id") { + t.Fatalf("expected duplicate id error, got: %v", err) + } +} + +// TestParseGradersFromFrontmatter_AllDisabledError verifies error when all disabled. +func TestParseGradersFromFrontmatter_AllDisabledError(t *testing.T) { + var c Compiler + graders := map[string]any{} + for _, id := range BuiltinGraderIDs { + graders[id] = map[string]any{"enabled": false} + } + _, err := c.parseGradersFromFrontmatter(map[string]any{"graders": graders}) + if err == nil { + t.Fatal("expected error when all graders disabled") + } +} + +// TestGradersConfig_EnabledGraderIDs_Order verifies stable ordering. +func TestGradersConfig_EnabledGraderIDs_Order(t *testing.T) { + cfg := &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "zebra-metric": {ID: "zebra-metric", Script: "1"}, + "alpha-metric": {ID: "alpha-metric", Script: "1"}, + "retries": {ID: "retries"}, + "tool-success-rate": {ID: "tool-success-rate"}, + }, + } + ids := cfg.EnabledGraderIDs() + // Built-ins first in canonical order, then custom alphabetically + if ids[0] != "tool-success-rate" { + t.Fatalf("expected tool-success-rate first, got %s", ids[0]) + } + if ids[1] != "retries" { + t.Fatalf("expected retries second, got %s", ids[1]) + } + if ids[2] != "alpha-metric" { + t.Fatalf("expected alpha-metric third, got %s", ids[2]) + } + if ids[3] != "zebra-metric" { + t.Fatalf("expected zebra-metric fourth, got %s", ids[3]) + } +} + +// TestBuildGraderManifest verifies manifest serialization. +func TestBuildGraderManifest(t *testing.T) { + enabled := true + disabled := false + cfg := &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "tool-success-rate": {ID: "tool-success-rate", Enabled: &enabled}, + "retries": {ID: "retries", Enabled: &disabled}, + "my-custom": {ID: "my-custom", Script: "trace.toolCalls.length"}, + }, + } + entries := buildGraderManifest(cfg) + if entries == nil { + t.Fatal("expected non-nil manifest") + } + if entries.Version != 1 { + t.Fatalf("expected version 1, got %d", entries.Version) + } + if len(entries.Graders) != 3 { + t.Fatalf("expected 3 entries, got %d", len(entries.Graders)) + } + + // Verify JSON serialization round-trips + data, err := json.Marshal(entries) + if err != nil { + t.Fatalf("json marshal error: %v", err) + } + var decoded graderManifest + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("json unmarshal error: %v", err) + } + if decoded.Version != 1 { + t.Fatalf("expected decoded version 1, got %d", decoded.Version) + } + if len(decoded.Graders) != 3 { + t.Fatalf("expected 3 decoded entries, got %d", len(decoded.Graders)) + } +} + +// TestGenerateGradersStep_Absent verifies no step when graders nil. +func TestGenerateGradersStep_Absent(t *testing.T) { + c := &Compiler{} + var yaml strings.Builder + data := &WorkflowData{Graders: nil} + c.generateGradersStep(&yaml, data) + if yaml.Len() != 0 { + t.Fatal("expected no output when graders nil") + } +} + +// TestGenerateGradersStep_Present verifies step is emitted. +func TestGenerateGradersStep_Present(t *testing.T) { + c := &Compiler{} + initActionPinCacheForTest(c) + var yaml strings.Builder + data := &WorkflowData{ + Graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "retries": {ID: "retries"}, + }, + }, + } + c.generateGradersStep(&yaml, data) + output := yaml.String() + if !strings.Contains(output, "Run graders") { + t.Fatal("expected step name 'Run graders'") + } + if !strings.Contains(output, "if: always()") { + t.Fatal("expected always() condition") + } + if !strings.Contains(output, "trace_graders.cjs") { + t.Fatal("expected trace_graders.cjs require") + } + if !strings.Contains(output, "actions/github-script") { + t.Fatal("expected actions/github-script usage") + } + if !strings.Contains(output, "await main('") { + t.Fatal("expected main invocation with encoded payloads") + } + if strings.Contains(output, "{\"version\"") { + t.Fatal("expected manifest to be encoded, not embedded as raw JSON") + } +} + +// TestGenerateGradersStep_BeforeArtifactUpload verifies ordering. +func TestGenerateGradersStep_BeforeArtifactUpload(t *testing.T) { + c := &Compiler{} + initActionPinCacheForTest(c) + var yaml strings.Builder + + data := &WorkflowData{ + Graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "retries": {ID: "retries"}, + }, + }, + } + + // Simulate the ordering: graders step then artifact upload + c.generateGradersStep(&yaml, data) + yaml.WriteString(" - name: Upload agent artifacts\n") + + output := yaml.String() + graderIdx := strings.Index(output, "Run graders") + uploadIdx := strings.Index(output, "Upload agent artifacts") + if graderIdx < 0 || uploadIdx < 0 { + t.Fatal("expected both steps to be present") + } + if graderIdx >= uploadIdx { + t.Fatal("graders step must come before artifact upload") + } +} + +// TestCollectGraderArtifactPaths verifies paths include manifest and results. +func TestCollectGraderArtifactPaths(t *testing.T) { + paths := collectGraderArtifactPaths() + if len(paths) != 2 { + t.Fatalf("expected 2 paths, got %d", len(paths)) + } + if !strings.Contains(paths[0], "grader_manifest.json") { + t.Fatal("expected grader_manifest.json in paths") + } + if !strings.Contains(paths[1], "grader_results.json") { + t.Fatal("expected grader_results.json in paths") + } +} + +// initActionPinCacheForTest sets up minimal action pin resolution for tests. +func initActionPinCacheForTest(c *Compiler) { + // The Compiler uses getActionPin/getCachedActionPin which resolves from a global + // cache. In tests, we just verify the step generation logic, the pin is tested separately. +} + +// TestCollectGraderArtifactPaths_AgentGradersDir verifies paths use the agent/graders subdirectory. +func TestCollectGraderArtifactPaths_AgentGradersDir(t *testing.T) { + paths := collectGraderArtifactPaths() + for _, p := range paths { + if !strings.Contains(p, "agent/graders/") { + t.Errorf("expected path to contain agent/graders/, got %q", p) + } + } +} + +// TestGenerateGraderRedactionStep_CustomOnly verifies that the redaction step is only emitted +// when custom (non-builtin) grader scripts are present. +func TestGenerateGraderRedactionStep_CustomOnly(t *testing.T) { + c := &Compiler{stepOrderTracker: NewStepOrderTracker()} + var yaml strings.Builder + + // Built-in only — no redaction step + data := &WorkflowData{ + Graders: &GradersConfig{ + Graders: map[string]*GraderDefinition{ + "retries": {ID: "retries"}, + }, + }, + } + c.generateGraderRedactionStep(&yaml, "", data) + if yaml.Len() > 0 { + t.Error("expected no redaction step for built-in-only graders") + } + + // Custom script — should emit redaction step + data.Graders.Graders["my-custom"] = &GraderDefinition{ + ID: "my-custom", + Script: "return {value: 1}", + } + c.generateGraderRedactionStep(&yaml, "", data) + if !strings.Contains(yaml.String(), "Redact grader outputs") { + t.Error("expected redaction step for custom grader script") + } +} + +// TestGradersConfig_FrontmatterConfigField verifies the FrontmatterConfig struct has a Graders field. +func TestGradersConfig_FrontmatterConfigField(t *testing.T) { + fc := FrontmatterConfig{ + Graders: map[string]any{}, + } + if fc.Graders == nil { + t.Error("expected Graders field to be set") + } +} diff --git a/pkg/workflow/graders_experimental_warning_test.go b/pkg/workflow/graders_experimental_warning_test.go new file mode 100644 index 00000000000..540e2fcbb79 --- /dev/null +++ b/pkg/workflow/graders_experimental_warning_test.go @@ -0,0 +1,99 @@ +//go:build integration + +package workflow + +import ( + "bytes" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" +) + +func TestGradersExperimentalWarning(t *testing.T) { + tests := []struct { + name string + content string + expectWarning bool + }{ + { + name: "graders enabled produces experimental warning", + content: `--- +on: workflow_dispatch +engine: copilot +graders: {} +--- + +# Test Workflow +`, + expectWarning: true, + }, + { + name: "no graders does not produce experimental warning", + content: `--- +on: workflow_dispatch +engine: copilot +--- + +# Test Workflow +`, + expectWarning: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := testutil.TempDir(t, "graders-experimental-warning-test") + + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(tt.content), 0644); err != nil { + t.Fatal(err) + } + + oldStderr := os.Stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stderr = w + t.Cleanup(func() { + os.Stderr = oldStderr + }) + + compiler := NewCompiler() + compiler.SetStrictMode(false) + compileErr := compiler.CompileWorkflow(testFile) + + _ = w.Close() + os.Stderr = oldStderr + var buf bytes.Buffer + if _, copyErr := io.Copy(&buf, r); copyErr != nil { + t.Fatal(copyErr) + } + stderrOutput := buf.String() + + if compileErr != nil { + t.Errorf("expected compilation to succeed but it failed: %v", compileErr) + return + } + + expectedMessage := "Using experimental feature: graders" + if tt.expectWarning { + if !strings.Contains(stderrOutput, expectedMessage) { + t.Errorf("expected warning containing %q, got stderr:\n%s", expectedMessage, stderrOutput) + } + if compiler.GetWarningCount() == 0 { + t.Error("expected warning count > 0 but got 0") + } + return + } + + if strings.Contains(stderrOutput, expectedMessage) { + t.Errorf("did not expect warning %q, but got stderr:\n%s", expectedMessage, stderrOutput) + } + }) + } +} diff --git a/pkg/workflow/graders_workflow_integration_test.go b/pkg/workflow/graders_workflow_integration_test.go new file mode 100644 index 00000000000..a4ed7b0ae59 --- /dev/null +++ b/pkg/workflow/graders_workflow_integration_test.go @@ -0,0 +1,158 @@ +//go:build integration + +package workflow + +import ( + "encoding/base64" + "encoding/json" + "os" + "path/filepath" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/testutil" +) + +func TestGradersWorkflowIntegration_SyntaxParserSchema(t *testing.T) { + tmpDir := testutil.TempDir(t, "graders-workflow-integration") + workflowPath := filepath.Join(tmpDir, "workflow.md") + + content := `--- +on: workflow_dispatch +engine: copilot +strict: false +permissions: + contents: read +graders: + retries: null + custom-score: + name: Custom Score + description: Measures custom score + unit: ratio + direction: higher_is_better + threshold: 0.7 + min: 0.0 + max: 1.0 + config: + window: 5 + script: | + return helpers.clamp(trace.toolCalls.length / 10, 0, 1) +experiments: + prompt_style: + variants: [control, candidate] + metric: grader:retries + model_mix: + variants: [baseline, tuned] + metric: graders.custom-score.value +--- + +# Integration test workflow +` + require.NoError(t, os.WriteFile(workflowPath, []byte(content), 0o644)) + + compiler := NewCompiler(WithVersion("dev")) + require.NoError(t, compiler.CompileWorkflow(workflowPath)) + + compiledPath := stringutil.MarkdownToLockFile(workflowPath) + compiled, err := os.ReadFile(compiledPath) + require.NoError(t, err) + yaml := string(compiled) + + assert.Contains(t, yaml, "Run graders") + assert.Contains(t, yaml, "trace_graders.cjs") + assert.Contains(t, yaml, "grader:retries") + assert.Contains(t, yaml, "graders.custom-score.value") + + match := regexp.MustCompile(`await main\('([^']+)', '([^']+)'\);`).FindStringSubmatch(yaml) + require.Len(t, match, 3, "expected encoded manifest and exec spec in generated script") + + manifestJSON, err := base64.StdEncoding.DecodeString(match[1]) + require.NoError(t, err) + + var manifest graderManifest + require.NoError(t, json.Unmarshal(manifestJSON, &manifest)) + require.Equal(t, 1, manifest.Version) + + entriesByID := map[string]graderManifestEntry{} + for _, entry := range manifest.Graders { + entriesByID[entry.ID] = entry + } + + retries, ok := entriesByID["retries"] + require.True(t, ok, "expected retries grader in manifest") + assert.Equal(t, "builtin", retries.Source) + assert.True(t, retries.Enabled) + + custom, ok := entriesByID["custom-score"] + require.True(t, ok, "expected custom grader in manifest") + assert.Equal(t, "inline", custom.Source) + assert.Equal(t, "Custom Score", custom.Name) + assert.Equal(t, "ratio", custom.Unit) + assert.Equal(t, "higher_is_better", custom.Direction) + require.NotNil(t, custom.Threshold) + assert.InDelta(t, 0.7, *custom.Threshold, 0.000001) + require.NotNil(t, custom.Config) + assert.Equal(t, float64(5), custom.Config["window"]) + + execJSON, err := base64.StdEncoding.DecodeString(match[2]) + require.NoError(t, err) + + var execEntries []graderExecEntry + require.NoError(t, json.Unmarshal(execJSON, &execEntries)) + require.Len(t, execEntries, 1) + assert.Equal(t, "custom-score", execEntries[0].ID) + assert.Contains(t, execEntries[0].Script, "helpers.clamp") +} + +func TestGradersWorkflowIntegration_SchemaRejectsUnknownGraderField(t *testing.T) { + tmpDir := testutil.TempDir(t, "graders-schema-rejects-unknown-field") + workflowPath := filepath.Join(tmpDir, "workflow.md") + + content := `--- +on: workflow_dispatch +engine: copilot +graders: + retries: + unsupported: true +--- + +# Integration test workflow +` + require.NoError(t, os.WriteFile(workflowPath, []byte(content), 0o644)) + + compiler := NewCompiler(WithVersion("dev")) + err := compiler.CompileWorkflow(workflowPath) + require.Error(t, err) + assert.Contains(t, err.Error(), "graders") + if !regexp.MustCompile(`additionalProperties|unknown field|Unknown property`).MatchString(err.Error()) { + t.Fatalf("expected schema/parser unknown-field error, got: %v", err) + } +} + +func TestGradersWorkflowIntegration_ExperimentsMetricRequiresDeclaredGrader(t *testing.T) { + tmpDir := testutil.TempDir(t, "experiments-grader-metric-reference") + workflowPath := filepath.Join(tmpDir, "workflow.md") + + content := `--- +on: workflow_dispatch +engine: copilot +strict: false +experiments: + prompt_style: + variants: [control, candidate] + metric: grader:loops +--- + +# Integration test workflow +` + require.NoError(t, os.WriteFile(workflowPath, []byte(content), 0o644)) + + compiler := NewCompiler(WithVersion("dev")) + err := compiler.CompileWorkflow(workflowPath) + require.Error(t, err) + assert.Contains(t, err.Error(), `references grader "loops" but no graders are declared`) +} diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index 8bf44a15177..f033f4d2cba 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -208,6 +208,7 @@ type WorkflowData struct { ContainerPinMappings map[string]string // container-pin redirect table from aw.json container_pins: maps source image → replacement image GHES bool // select action versions compatible with GitHub Enterprise Server Evals *EvalsConfig // BinEval evaluation configuration parsed from frontmatter evals field + Graders *GradersConfig // Deterministic graders configuration parsed from frontmatter graders field ExcludedEnv []string // additional env var names to exclude from agent container via AWF --exclude-env (from frontmatter excluded-env field) }