From 5a5183fa738cc9111258fcb2f638df8b1185b005 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Fri, 12 Jun 2026 14:21:54 +0530 Subject: [PATCH 1/5] Expand agent quality eval module --- docs/AGENT_EVALS.md | 70 ++++- internal/agent/system_prompt.go | 30 ++ internal/agent/system_prompt_test.go | 69 +++++ internal/agenteval/agenteval_test.go | 4 +- internal/agenteval/run.go | 192 +++++++++++++ internal/agenteval/run_test.go | 267 ++++++++++++++++++ .../testdata/fixtures/zero-mini/bin/zero.js | 6 + .../zero-mini/cmd/zero-release/main.go | 9 + .../zero-mini/cmd/zero-release/main_test.go | 9 + .../zero-mini/docs/NPM_WRAPPER_SMOKE.md | 10 + .../fixtures/zero-mini/docs/RELEASE_SMOKE.md | 4 + .../zero-mini/docs/STREAM_JSON_PROTOCOL.md | 11 + .../testdata/fixtures/zero-mini/go.mod | 3 + .../zero-mini/internal/agent/denial_test.go | 17 ++ .../zero-mini/internal/agent/system_prompt.md | 4 + .../zero-mini/internal/config/validate.go | 18 ++ .../internal/config/validate_test.go | 10 + .../fixtures/zero-mini/internal/mcp/config.go | 9 + .../zero-mini/internal/mcp/config_test.go | 10 + .../internal/modelregistry/catalog.go | 13 + .../zero-mini/internal/npmwrapper/wrapper.go | 5 + .../internal/npmwrapper/wrapper_test.go | 10 + .../zero-mini/internal/release/release.go | 8 + .../internal/release/release_test.go | 9 + .../internal/selfverify/selfverify.go | 5 + .../internal/selfverify/selfverify_test.go | 11 + .../zero-mini/internal/verify/verify.go | 13 + .../zero-mini/internal/verify/verify_test.go | 10 + internal/agenteval/testdata/sample_suite.json | 133 +++++++-- internal/cli/agent_eval.go | 192 +++++++++++-- internal/cli/agent_eval_test.go | 179 +++++++++++- 31 files changed, 1285 insertions(+), 55 deletions(-) create mode 100644 internal/agenteval/run.go create mode 100644 internal/agenteval/run_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/bin/zero.js create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/docs/NPM_WRAPPER_SMOKE.md create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/docs/RELEASE_SMOKE.md create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/docs/STREAM_JSON_PROTOCOL.md create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/go.mod create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/agent/denial_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/agent/system_prompt.md create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/modelregistry/catalog.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/release/release.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/release/release_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify.go create mode 100644 internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go diff --git a/docs/AGENT_EVALS.md b/docs/AGENT_EVALS.md index 2d6754a70..a84008227 100644 --- a/docs/AGENT_EVALS.md +++ b/docs/AGENT_EVALS.md @@ -5,13 +5,15 @@ without calling a live model. They describe a task, the files the agent is expected to change, the commands that should verify the result, and the scoring rules an offline harness can apply to a captured run. -These fixtures are intentionally contract-shaped. They do not prove provider -quality or live model execution by themselves; they give tests and future CLI -work a stable sample suite to parse, validate, and score against saved outputs. +These fixtures are intentionally local-first. They do not prove provider quality +or live model execution by themselves; they give tests and future CLI work a +stable sample suite to validate, run against copied workspaces, and score from +saved outputs. ## Suite Format -Sample suites live under `internal/agenteval/testdata/`. +Sample suites live under `internal/agenteval/testdata/`. Tiny fixture +workspaces live under `internal/agenteval/testdata/fixtures/`. Each suite JSON file contains: @@ -35,20 +37,47 @@ by `verificationCommands[].id`, and changed files are compared against `expectedChangedFiles`. Extra fields should not be added to suite JSON unless the loader and tests are updated in the same PR. -## Run Locally +## Modes -Validate and summarize a suite through the CLI: +`zero eval` defaults to validate mode. It parses the suite, rejects schema or +contract errors, and reports the number of tasks and checks. It does not copy +fixtures, invoke an agent, or execute verification commands. ```bash go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json ``` -For JSON output: +Use JSON output when another local tool needs the validation summary: ```bash go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json --json ``` +`zero eval run` scores one local workspace. It does not invoke the agent yet; +point it at a Git worktree where a fixture has already been copied and a task +has already been attempted. The runner executes each `verificationCommands` +entry, collects changed files with `git status --porcelain`, and emits the +task-success report contract below. When `--workspace` is omitted, the current +directory is used. + +```bash +go run ./cmd/zero eval run \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --workspace /tmp/zero-eval-workspace +``` + +Persist the report for comparison between prompt or model changes: + +```bash +go run ./cmd/zero eval run \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --workspace /tmp/zero-eval-workspace \ + --report-dir /tmp/zero-eval-report \ + --json +``` + Run the package tests when changing the suite schema or scorer: ```bash @@ -71,12 +100,27 @@ The `internal/agenteval` tests load every JSON file under `internal/agenteval/testdata/` and reject missing task IDs, empty verification commands, and malformed changed-file expectations. +## Report JSON + +Scored reports use contract `zero.agenteval.report.v1`. + +- `suiteId` and `taskId`: identify the suite and selected task. +- `status`: overall `pass`, `fail`, `blocked`, or `error`. +- `ok`: true only when every result passes. +- `summary`: total result counts by status. +- `changedFiles`: normalized files collected from the workspace. +- `results`: one result per verification command, plus `changed_files`. +- `error`: task-selection or report-level error, when present. + +Command results include the command ID, display name, command argv, status, +exit code, stdout, stderr, and an optional message. The changed-files result +includes expected, actual, missing, and unexpected files. + ## Score Interpretation Scores are offline quality signals, not pass/fail release gates by default. The -current `zero eval` command validates and summarizes suite files only; the -statuses below are produced by the `internal/agenteval.Score` library API when a -future harness supplies captured command results and changed files. +statuses below are produced when a harness supplies captured command results and +changed files. - `pass`: every verification command exited successfully and the changed files matched `expectedChangedFiles`. @@ -86,5 +130,7 @@ future harness supplies captured command results and changed files. - `error`: the suite, task ID, command ID, or captured input could not be interpreted. -Prefer comparing results between runs of the same suite revision. Do not compare -results across suites unless the task mix and scoring contract are unchanged. +Real task-success measurement comes from the combination of prompt, fixture, +verification commands, and changed-file expectations. Prefer comparing results +between runs of the same suite revision. Do not compare results across suites +unless the task mix and scoring contract are unchanged. diff --git a/internal/agent/system_prompt.go b/internal/agent/system_prompt.go index 4add02a43..20903988e 100644 --- a/internal/agent/system_prompt.go +++ b/internal/agent/system_prompt.go @@ -9,6 +9,7 @@ import ( "unicode/utf8" "github.com/Gitlawb/zero/internal/repomap" + "github.com/Gitlawb/zero/internal/workspaceseed" ) // coreSystemPrompt is the de-branded coding-craft instruction set: identity, @@ -42,6 +43,11 @@ const maxProjectContextBytes = 8 << 10 // 8 KiB // remain stable across normal agent turns. const maxRepoMapContextBytes = 4 << 10 // 4 KiB +const ( + workspaceSeedMaxLines = 12 + workspaceSeedWidth = 100 +) + // buildSystemPrompt assembles the full system prompt for a run: the core // coding-craft instructions, dynamic workspace context (cwd, git branch, project // guidelines), and the safety confirmation policy. It is built once per run so @@ -61,6 +67,9 @@ func buildSystemPrompt(options Options) string { if session := sessionRuntimeContext(options); session != "" { sections = append(sections, session) } + if seed := workspaceSeedContext(options.Cwd); seed != "" { + sections = append(sections, seed) + } if ws := workspaceContext(options.Cwd); ws != "" { sections = append(sections, ws) } @@ -137,6 +146,27 @@ func workspaceContext(cwd string) string { return b.String() } +func workspaceSeedContext(cwd string) string { + cwd = strings.TrimSpace(cwd) + if cwd == "" { + return "" + } + seed, err := workspaceseed.BuildFromWorkspace(cwd, workspaceseed.GitInfo{ + Branch: gitBranchForPrompt(cwd), + }) + if err != nil { + return "" + } + rendered := strings.TrimSpace(workspaceseed.Render(seed, workspaceseed.RenderOptions{ + MaxLines: workspaceSeedMaxLines, + Width: workspaceSeedWidth, + })) + if rendered == "" { + return "" + } + return "\n" + rendered + "\n" +} + func repoMapContext(cwd string) string { // repomap.Scan is best-effort supplemental context for the prompt. If it // fails, omit the repo map instead of failing the agent run; successful scans diff --git a/internal/agent/system_prompt_test.go b/internal/agent/system_prompt_test.go index d93e57b51..472bde7ff 100644 --- a/internal/agent/system_prompt_test.go +++ b/internal/agent/system_prompt_test.go @@ -1,6 +1,8 @@ package agent import ( + "os" + "path/filepath" "strings" "testing" ) @@ -24,3 +26,70 @@ func TestCoreSystemPromptIncludesCodingQualityRules(t *testing.T) { } } } + +func TestBuildSystemPromptIncludesWorkspaceSeedFromCwd(t *testing.T) { + cwd := t.TempDir() + writeSystemPromptTestFile(t, cwd, "go.mod", "module example.test/zero\n") + writeSystemPromptTestFile(t, cwd, "AGENTS.md", "Use Go commands.\n") + writeSystemPromptTestFile(t, cwd, "cmd/zero/main.go", "package main\n") + writeSystemPromptTestFile(t, cwd, "internal/agent/loop.go", "package agent\n") + writeSystemPromptTestFile(t, cwd, "node_modules/pkg/index.js", "ignored") + writeSystemPromptTestFile(t, cwd, filepath.Join(".git", "HEAD"), "ref: refs/heads/feature/seed\n") + + prompt := buildSystemPrompt(Options{Cwd: cwd}) + + for _, want := range []string{ + "", + "Workspace context seed", + "cwd: " + filepath.Base(cwd), + "git: feature/seed", + "layout: AGENTS.md, cmd/, go.mod, internal/", + "project files: go.mod, AGENTS.md", + "memory hints: AGENTS.md", + "", + } { + if !strings.Contains(prompt, want) { + t.Fatalf("expected workspace seed to include %q, got:\n%s", want, prompt) + } + } + seed := systemPromptTestBlock(t, prompt, "", "") + if strings.Contains(seed, cwd) { + t.Fatalf("workspace seed should use safe cwd label, not absolute path %q, got:\n%s", cwd, seed) + } + if strings.Contains(prompt, "node_modules") { + t.Fatalf("workspace seed should inherit workspace skip rules, got:\n%s", prompt) + } +} + +func TestBuildSystemPromptOmitsWorkspaceSeedWithoutCwd(t *testing.T) { + prompt := buildSystemPrompt(Options{}) + + if strings.Contains(prompt, "") || strings.Contains(prompt, "Workspace context seed") { + t.Fatalf("workspace seed should be absent without cwd, got:\n%s", prompt) + } +} + +func writeSystemPromptTestFile(t *testing.T, root, rel, contents string) { + t.Helper() + path := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } +} + +func systemPromptTestBlock(t *testing.T, prompt, start, end string) string { + t.Helper() + startIndex := strings.Index(prompt, start) + if startIndex < 0 { + t.Fatalf("missing block start %q", start) + } + afterStart := prompt[startIndex+len(start):] + body, _, ok := strings.Cut(afterStart, end) + if !ok { + t.Fatalf("missing block end %q", end) + } + return body +} diff --git a/internal/agenteval/agenteval_test.go b/internal/agenteval/agenteval_test.go index 676960f4c..d6e267dfc 100644 --- a/internal/agenteval/agenteval_test.go +++ b/internal/agenteval/agenteval_test.go @@ -90,8 +90,8 @@ func TestSampleSuiteLoads(t *testing.T) { if err != nil { t.Fatalf("sample suite should load: %v", err) } - if len(suite.Tasks) != 3 { - t.Fatalf("sample suite tasks = %d, want 3", len(suite.Tasks)) + if len(suite.Tasks) < 5 || len(suite.Tasks) > 8 { + t.Fatalf("sample suite tasks = %d, want 5-8", len(suite.Tasks)) } for _, task := range suite.Tasks { if len(task.ExpectedChangedFiles) == 0 { diff --git a/internal/agenteval/run.go b/internal/agenteval/run.go new file mode 100644 index 000000000..ecfe05eb7 --- /dev/null +++ b/internal/agenteval/run.go @@ -0,0 +1,192 @@ +package agenteval + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +type RunInput struct { + TaskID string + WorkspacePath string +} + +type CommandRunner func(context.Context, string, Command) CommandResult + +type ChangedFilesFunc func(context.Context, string) ([]string, error) + +type Runner struct { + RunCommand CommandRunner + ChangedFiles ChangedFilesFunc + + runGit func(context.Context, string, ...string) ([]byte, error) +} + +func (runner Runner) Run(ctx context.Context, suite Suite, input RunInput) Report { + if ctx == nil { + ctx = context.Background() + } + task, err := selectTask(suite, input.TaskID) + if err != nil { + return Score(suite, ScoreInput{TaskID: input.TaskID}) + } + workspace, err := verifyWorkspace(input.WorkspacePath) + if err != nil { + return runner.blocked(suite, task.ID, fmt.Sprintf("workspace setup failed: %v", err), nil) + } + if err := ctx.Err(); err != nil { + return runner.blocked(suite, task.ID, err.Error(), nil) + } + + results := make([]CommandResult, 0, len(task.VerificationCommands)) + for _, command := range task.VerificationCommands { + if err := ctx.Err(); err != nil { + return runner.blocked(suite, task.ID, err.Error(), results) + } + result := runner.runCommand(ctx, workspace, command) + if result.ID == "" { + result.ID = command.ID + } + results = append(results, result) + } + + files, err := runner.changedFiles(ctx, workspace) + if err != nil { + reason := fmt.Sprintf("changed files collection failed: %v", err) + return runner.blocked(suite, task.ID, reason, results) + } + + return Score(suite, ScoreInput{ + TaskID: task.ID, + CommandResults: results, + ChangedFiles: files, + }) +} + +func (runner Runner) blocked(suite Suite, taskID string, reason string, results []CommandResult) Report { + return Score(suite, ScoreInput{ + TaskID: taskID, + CommandResults: results, + Blocked: true, + BlockReason: reason, + }) +} + +func (runner Runner) runCommand(ctx context.Context, workspace string, command Command) CommandResult { + if runner.RunCommand != nil { + return runner.RunCommand(ctx, workspace, command) + } + return execCommand(ctx, workspace, command) +} + +func (runner Runner) changedFiles(ctx context.Context, workspace string) ([]string, error) { + if runner.ChangedFiles != nil { + return runner.ChangedFiles(ctx, workspace) + } + runGit := runner.runGit + if runGit == nil { + runGit = defaultRunGit + } + output, err := runGit(ctx, workspace, "status", "--porcelain", "--untracked-files=all") + if err != nil { + return nil, err + } + return parseGitStatusPorcelain(output), nil +} + +func execCommand(ctx context.Context, workspace string, command Command) CommandResult { + result := CommandResult{ID: command.ID, ExitCode: -1} + if emptyCommand(command.Command) { + result.Error = "command must not be empty" + return result + } + args := trimCommand(command.Command) + cmd := exec.CommandContext(ctx, args[0], args[1:]...) + cmd.Dir = workspace + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + result.Stdout = stdout.String() + result.Stderr = stderr.String() + if err == nil { + result.ExitCode = 0 + return result + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + result.ExitCode = exitErr.ExitCode() + return result + } + if ctxErr := ctx.Err(); ctxErr != nil { + result.Error = ctxErr.Error() + return result + } + result.Error = err.Error() + return result +} + +func defaultRunGit(ctx context.Context, workspace string, args ...string) ([]byte, error) { + allArgs := append([]string{"-C", workspace}, args...) + cmd := exec.CommandContext(ctx, "git", allArgs...) + output, err := cmd.Output() + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, err + } + return output, nil +} + +func parseGitStatusPorcelain(output []byte) []string { + lines := strings.Split(string(output), "\n") + files := make([]string, 0, len(lines)) + for _, line := range lines { + if len(line) < 4 { + continue + } + file := strings.TrimSpace(line[3:]) + if arrow := strings.LastIndex(file, " -> "); arrow >= 0 { + file = file[arrow+4:] + } + files = append(files, file) + } + return normalizeFiles(files) +} + +func verifyWorkspace(workspace string) (string, error) { + workspace = strings.TrimSpace(workspace) + if workspace == "" { + return "", errors.New("workspace path is required") + } + clean, err := filepath.Abs(workspace) + if err != nil { + return "", err + } + info, err := os.Stat(clean) + if err != nil { + return "", err + } + if !info.IsDir() { + return "", errors.New("workspace path must be a directory") + } + return clean, nil +} + +func trimCommand(command []string) []string { + trimmed := make([]string, 0, len(command)) + for _, part := range command { + part = strings.TrimSpace(part) + if part != "" { + trimmed = append(trimmed, part) + } + } + return trimmed +} diff --git a/internal/agenteval/run_test.go b/internal/agenteval/run_test.go new file mode 100644 index 000000000..6eb0ea816 --- /dev/null +++ b/internal/agenteval/run_test.go @@ -0,0 +1,267 @@ +package agenteval + +import ( + "context" + "errors" + "os" + "path/filepath" + "reflect" + "testing" +) + +func TestRunnerMapsPassingAndFailingCommandResults(t *testing.T) { + workspace := t.TempDir() + runner := Runner{ + RunCommand: func(_ context.Context, gotWorkspace string, command Command) CommandResult { + if gotWorkspace != workspace { + t.Fatalf("workspace = %q, want %q", gotWorkspace, workspace) + } + switch command.ID { + case "pass": + return CommandResult{ID: command.ID, ExitCode: 0, Stdout: "ok"} + case "fail": + return CommandResult{ID: command.ID, ExitCode: 2, Stderr: "nope"} + default: + t.Fatalf("unexpected command %q", command.ID) + } + return CommandResult{} + }, + ChangedFiles: func(context.Context, string) ([]string, error) { + return []string{"internal/reader/a.go"}, nil + }, + } + + report := runner.Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "two-commands", + WorkspacePath: workspace, + }) + + if report.Status != StatusFail || report.OK { + t.Fatalf("status = %s ok = %v, want fail false", report.Status, report.OK) + } + if got := report.Results[0]; got.ID != "pass" || got.Status != StatusPass || got.Stdout != "ok" { + t.Fatalf("passing command result = %#v", got) + } + if got := report.Results[1]; got.ID != "fail" || got.Status != StatusFail || got.Stderr != "nope" { + t.Fatalf("failing command result = %#v", got) + } + if report.Summary.Failed != 1 || report.Summary.Passed != 2 { + t.Fatalf("summary = %#v, want one failed command and two passing checks", report.Summary) + } +} + +func TestRunnerBlocksWhenWorkspaceSetupFails(t *testing.T) { + missing := filepath.Join(t.TempDir(), "missing") + calledCommand := false + calledChangedFiles := false + runner := Runner{ + RunCommand: func(context.Context, string, Command) CommandResult { + calledCommand = true + return CommandResult{} + }, + ChangedFiles: func(context.Context, string) ([]string, error) { + calledChangedFiles = true + return nil, nil + }, + } + + report := runner.Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "single", + WorkspacePath: missing, + }) + + if report.Status != StatusBlocked || report.OK { + t.Fatalf("status = %s ok = %v, want blocked false", report.Status, report.OK) + } + if calledCommand || calledChangedFiles { + t.Fatalf("blocked workspace should not execute commands or changed-file collection") + } + if report.Summary.Blocked != 2 { + t.Fatalf("summary = %#v, want both checks blocked", report.Summary) + } + if got := report.Results[0].Message; got == "" { + t.Fatal("blocked result should include a message") + } +} + +func TestRunnerCollectsChangedFilesFromInjectedFunc(t *testing.T) { + workspace := t.TempDir() + runner := Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + ChangedFiles: func(_ context.Context, gotWorkspace string) ([]string, error) { + if gotWorkspace != workspace { + t.Fatalf("workspace = %q, want %q", gotWorkspace, workspace) + } + return []string{"z.go", "internal/reader/a.go", "internal/reader/a/../b.go"}, nil + }, + } + + report := runner.Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "single", + WorkspacePath: workspace, + }) + + want := []string{"internal/reader/a.go", "internal/reader/b.go", "z.go"} + if !reflect.DeepEqual(report.ChangedFiles, want) { + t.Fatalf("changed files = %#v, want %#v", report.ChangedFiles, want) + } + changed := report.Results[1] + if !reflect.DeepEqual(changed.UnexpectedFiles, []string{"z.go"}) { + t.Fatalf("unexpected files = %#v, want z.go", changed.UnexpectedFiles) + } +} + +func TestRunnerBlocksWhenChangedFileCollectionFails(t *testing.T) { + workspace := t.TempDir() + runner := Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + ChangedFiles: func(context.Context, string) ([]string, error) { + return nil, errors.New("git unavailable") + }, + } + + report := runner.Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "single", + WorkspacePath: workspace, + }) + + if report.Status != StatusBlocked || report.Summary.Blocked != 2 { + t.Fatalf("report = %#v, want blocked command and changed-file checks", report) + } + if got := report.Results[1].Message; got == "" { + t.Fatal("changed-file block should include a message") + } +} + +func TestRunnerSelectsRequestedTask(t *testing.T) { + workspace := t.TempDir() + var executed []string + runner := Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + executed = append(executed, command.ID) + return CommandResult{ID: command.ID, ExitCode: 0} + }, + ChangedFiles: func(context.Context, string) ([]string, error) { + return []string{"cmd/zero/main.go"}, nil + }, + } + + report := runner.Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "other", + WorkspacePath: workspace, + }) + + if report.TaskID != "other" || report.Status != StatusPass { + t.Fatalf("report task/status = %q/%s, want other/pass", report.TaskID, report.Status) + } + if !reflect.DeepEqual(executed, []string{"other-test"}) { + t.Fatalf("executed commands = %#v, want only selected task command", executed) + } +} + +func TestRunnerDoesNotRequireWorkspaceForTaskSelectionErrors(t *testing.T) { + runner := Runner{} + + report := runner.Run(context.Background(), runnerSuite(), RunInput{}) + + if report.Status != StatusError { + t.Fatalf("status = %s, want task selection error", report.Status) + } + if report.Error == "" { + t.Fatal("task selection error should be reported") + } +} + +func TestDefaultChangedFilesParsesGitStatusPorcelain(t *testing.T) { + workspace := t.TempDir() + runner := Runner{ + RunCommand: func(context.Context, string, Command) CommandResult { + t.Fatal("command should not be needed") + return CommandResult{} + }, + runGit: func(_ context.Context, gotWorkspace string, args ...string) ([]byte, error) { + if gotWorkspace != workspace { + t.Fatalf("workspace = %q, want %q", gotWorkspace, workspace) + } + if !reflect.DeepEqual(args, []string{"status", "--porcelain", "--untracked-files=all"}) { + t.Fatalf("git args = %#v", args) + } + return []byte(" M internal/reader/a.go\nR old.go -> internal/reader/b.go\n?? z.go\n"), nil + }, + } + + files, err := runner.changedFiles(context.Background(), workspace) + if err != nil { + t.Fatalf("changedFiles returned error: %v", err) + } + + want := []string{"internal/reader/a.go", "internal/reader/b.go", "z.go"} + if !reflect.DeepEqual(files, want) { + t.Fatalf("files = %#v, want %#v", files, want) + } +} + +func TestRunnerBlocksWhenWorkspacePathIsAFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "file.txt") + if err := os.WriteFile(path, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("write file workspace: %v", err) + } + + report := (Runner{}).Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "single", + WorkspacePath: path, + }) + + if report.Status != StatusBlocked { + t.Fatalf("status = %s, want blocked", report.Status) + } +} + +func runnerSuite() Suite { + return Suite{ + ID: "runner-suite", + Name: "Runner suite", + Tasks: []Task{ + { + ID: "single", + Name: "Single command", + Prompt: "Change reader.", + WorkspaceFixture: "fixtures/reader", + VerificationCommands: []Command{{ + ID: "test", + Name: "Tests", + Command: []string{"go", "test", "./..."}, + }}, + ExpectedChangedFiles: []string{"internal/reader/a.go", "internal/reader/b.go"}, + }, + { + ID: "two-commands", + Name: "Two commands", + Prompt: "Run two commands.", + WorkspaceFixture: "fixtures/reader", + VerificationCommands: []Command{ + {ID: "pass", Name: "Passing", Command: []string{"go", "test", "./..."}}, + {ID: "fail", Name: "Failing", Command: []string{"go", "test", "./internal/nope"}}, + }, + ExpectedChangedFiles: []string{"internal/reader/a.go"}, + }, + { + ID: "other", + Name: "Other task", + Prompt: "Change CLI.", + WorkspaceFixture: "fixtures/cli", + VerificationCommands: []Command{{ + ID: "other-test", + Name: "Other tests", + Command: []string{"go", "test", "./internal/cli"}, + }}, + ExpectedChangedFiles: []string{"cmd/zero/main.go"}, + }, + }, + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/bin/zero.js b/internal/agenteval/testdata/fixtures/zero-mini/bin/zero.js new file mode 100644 index 000000000..4904875a9 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/bin/zero.js @@ -0,0 +1,6 @@ +#!/usr/bin/env node + +const { spawnSync } = require("node:child_process"); + +const result = spawnSync("zero", process.argv.slice(2), { stdio: "inherit" }); +process.exit(result.status ?? 1); diff --git a/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go new file mode 100644 index 000000000..3cb77af02 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go @@ -0,0 +1,9 @@ +package main + +import "github.com/Gitlawb/zero-fixture/internal/release" + +func smokeTarget(alreadyBuilt bool) string { + return release.SmokeTarget(alreadyBuilt) +} + +func main() {} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go new file mode 100644 index 000000000..50afd64e9 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go @@ -0,0 +1,9 @@ +package main + +import "testing" + +func TestSmokeTargetUsesLocalBinary(t *testing.T) { + if got := smokeTarget(true); got != "local-binary" { + t.Fatalf("smokeTarget(true) = %q, want local-binary", got) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/docs/NPM_WRAPPER_SMOKE.md b/internal/agenteval/testdata/fixtures/zero-mini/docs/NPM_WRAPPER_SMOKE.md new file mode 100644 index 000000000..aaab0dc61 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/docs/NPM_WRAPPER_SMOKE.md @@ -0,0 +1,10 @@ +# NPM Wrapper Smoke + +Use Go commands for release validation: + +```bash +go run ./cmd/zero-release build +go run ./cmd/zero-release smoke +``` + +Use Node only when directly exercising `bin/zero.js`. diff --git a/internal/agenteval/testdata/fixtures/zero-mini/docs/RELEASE_SMOKE.md b/internal/agenteval/testdata/fixtures/zero-mini/docs/RELEASE_SMOKE.md new file mode 100644 index 000000000..552e60fbf --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/docs/RELEASE_SMOKE.md @@ -0,0 +1,4 @@ +# Release Smoke + +The release smoke command validates that the local release binary starts and +prints version information. diff --git a/internal/agenteval/testdata/fixtures/zero-mini/docs/STREAM_JSON_PROTOCOL.md b/internal/agenteval/testdata/fixtures/zero-mini/docs/STREAM_JSON_PROTOCOL.md new file mode 100644 index 000000000..4f7db11d3 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/docs/STREAM_JSON_PROTOCOL.md @@ -0,0 +1,11 @@ +# Stream JSON Protocol + +The fixture documents stream events for local verification commands. + +`zero verify --json` emits newline-delimited JSON events. Each event has a +`type` field and a stable task-local payload. + +Known event types: + +- `check`: one verification check completed. +- `summary`: all verification checks completed. diff --git a/internal/agenteval/testdata/fixtures/zero-mini/go.mod b/internal/agenteval/testdata/fixtures/zero-mini/go.mod new file mode 100644 index 000000000..52c964450 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/go.mod @@ -0,0 +1,3 @@ +module github.com/Gitlawb/zero-fixture + +go 1.22 diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/agent/denial_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/agent/denial_test.go new file mode 100644 index 000000000..926af0e73 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/agent/denial_test.go @@ -0,0 +1,17 @@ +package agent + +import ( + "os" + "strings" + "testing" +) + +func TestPromptMentionsDeniedOperations(t *testing.T) { + data, err := os.ReadFile("system_prompt.md") + if err != nil { + t.Fatalf("read prompt: %v", err) + } + if !strings.Contains(string(data), "denied") && !strings.Contains(string(data), "blocked") { + t.Fatalf("prompt should describe denied operations:\n%s", data) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/agent/system_prompt.md b/internal/agenteval/testdata/fixtures/zero-mini/internal/agent/system_prompt.md new file mode 100644 index 000000000..f6488e7a5 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/agent/system_prompt.md @@ -0,0 +1,4 @@ +# Local Agent Prompt + +When a file operation is denied, explain that the operation was blocked and ask +the user how they want to proceed. diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go new file mode 100644 index 000000000..cf371a2c2 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go @@ -0,0 +1,18 @@ +package config + +import "errors" + +type Config struct { + DefaultProvider string + Providers map[string]string +} + +func Validate(cfg Config) error { + if cfg.DefaultProvider == "" { + return errors.New("default provider is required") + } + if len(cfg.Providers) == 0 { + return errors.New("providers are required") + } + return nil +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go new file mode 100644 index 000000000..79af54fae --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go @@ -0,0 +1,10 @@ +package config + +import "testing" + +func TestValidateRequiresDefaultProvider(t *testing.T) { + err := Validate(Config{Providers: map[string]string{"local": "fixture"}}) + if err == nil { + t.Fatal("Validate returned nil, want missing default provider error") + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config.go new file mode 100644 index 000000000..c0041772d --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config.go @@ -0,0 +1,9 @@ +package mcp + +func ServerNames(servers map[string]string) []string { + names := make([]string, 0, len(servers)) + for name := range servers { + names = append(names, name) + } + return names +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config_test.go new file mode 100644 index 000000000..ad492fedb --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/mcp/config_test.go @@ -0,0 +1,10 @@ +package mcp + +import "testing" + +func TestServerNamesIncludesConfiguredServers(t *testing.T) { + names := ServerNames(map[string]string{"docs": "stdio"}) + if len(names) != 1 || names[0] != "docs" { + t.Fatalf("names = %#v, want docs", names) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/modelregistry/catalog.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/modelregistry/catalog.go new file mode 100644 index 000000000..0f5368bbc --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/modelregistry/catalog.go @@ -0,0 +1,13 @@ +package modelregistry + +type Provider struct { + Name string + Aliases []string +} + +func DefaultProviders() map[string]Provider { + return map[string]Provider{ + "openai": {Name: "OpenAI", Aliases: []string{"gpt-4.1"}}, + "local": {Name: "Local", Aliases: []string{"fixture-small"}}, + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper.go new file mode 100644 index 000000000..d336a9596 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper.go @@ -0,0 +1,5 @@ +package npmwrapper + +func DirectCommand() []string { + return []string{"node", "bin/zero.js", "--version"} +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper_test.go new file mode 100644 index 000000000..3c9538edd --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/npmwrapper/wrapper_test.go @@ -0,0 +1,10 @@ +package npmwrapper + +import "testing" + +func TestDirectCommandUsesNodeWrapper(t *testing.T) { + command := DirectCommand() + if len(command) < 2 || command[0] != "node" || command[1] != "bin/zero.js" { + t.Fatalf("command = %#v, want direct node wrapper", command) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/release/release.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/release/release.go new file mode 100644 index 000000000..2b0cb17b0 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/release/release.go @@ -0,0 +1,8 @@ +package release + +func SmokeTarget(alreadyBuilt bool) string { + if alreadyBuilt { + return "local-binary" + } + return "build-then-smoke" +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/release/release_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/release/release_test.go new file mode 100644 index 000000000..8829f50b4 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/release/release_test.go @@ -0,0 +1,9 @@ +package release + +import "testing" + +func TestSmokeTargetNamesLocalBinary(t *testing.T) { + if got := SmokeTarget(true); got != "local-binary" { + t.Fatalf("SmokeTarget(true) = %q, want local-binary", got) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify.go new file mode 100644 index 000000000..5de6365a5 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify.go @@ -0,0 +1,5 @@ +package selfverify + +func Checks() []string { + return []string{"version", "fixtures"} +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go new file mode 100644 index 000000000..8195a76a0 --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go @@ -0,0 +1,11 @@ +package selfverify + +import "testing" + +func TestChecksAreLocal(t *testing.T) { + for _, check := range Checks() { + if check == "" { + t.Fatal("check names must not be empty") + } + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify.go new file mode 100644 index 000000000..6539bc86b --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify.go @@ -0,0 +1,13 @@ +package verify + +type Event struct { + Type string + Name string +} + +func Events() []Event { + return []Event{ + {Type: "check", Name: "config"}, + {Type: "summary", Name: "verify"}, + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go new file mode 100644 index 000000000..ab361f50e --- /dev/null +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go @@ -0,0 +1,10 @@ +package verify + +import "testing" + +func TestEventsIncludeSummary(t *testing.T) { + events := Events() + if len(events) == 0 || events[len(events)-1].Type != "summary" { + t.Fatalf("events = %#v, want trailing summary", events) + } +} diff --git a/internal/agenteval/testdata/sample_suite.json b/internal/agenteval/testdata/sample_suite.json index 2dddbcba7..e0fcc5d53 100644 --- a/internal/agenteval/testdata/sample_suite.json +++ b/internal/agenteval/testdata/sample_suite.json @@ -1,21 +1,21 @@ { - "id": "zero-offline-agent-sample", - "name": "zero-offline-agent-sample", - "description": "Small offline fixture suite for exercising Zero coding-agent scoring contracts without live provider calls.", + "id": "zero-local-agent-quality", + "name": "zero-local-agent-quality", + "description": "Clean-room local fixture suite for exercising Zero coding-agent task success without live provider calls.", "tasks": [ { - "id": "verify-json-contract-doc", - "name": "Document verify JSON contract behavior", - "description": "Docs-only task for verify and selfverify JSON contracts.", - "prompt": "Update the stream-json protocol documentation so maintainers can see how `zero verify --json` reports verification events. Keep the wording honest and do not change runtime code.", - "workspaceFixture": "fixtures/zero-repo", + "id": "document-stream-json-verify-events", + "name": "Document stream JSON verify events", + "description": "Docs-only task covering the verify and self-verify event contract.", + "prompt": "Update the stream-json protocol docs so maintainers can see how `zero verify --json` and `zero selfverify --json` report start, check, summary, and error events. Keep the wording accurate, local-first, and do not change runtime code.", + "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "docs/STREAM_JSON_PROTOCOL.md" ], "verificationCommands": [ { - "id": "go-test-verify-contracts", - "name": "Verify contract tests", + "id": "verify-contract-tests", + "name": "Verify JSON contract tests", "command": [ "go", "test", @@ -26,17 +26,17 @@ ] }, { - "id": "npm-wrapper-smoke-fixture", - "name": "Tighten npm wrapper smoke guidance", - "description": "Docs-only task for npm wrapper validation guidance.", - "prompt": "Improve the npm wrapper smoke checklist so a maintainer can distinguish Go release checks from direct Node wrapper checks. Do not add Node build dependencies.", - "workspaceFixture": "fixtures/zero-repo", + "id": "separate-go-release-from-node-wrapper-smoke", + "name": "Separate Go release checks from Node wrapper checks", + "description": "Docs task that keeps the npm wrapper guidance honest and dependency-free.", + "prompt": "Improve the npm wrapper smoke checklist so a maintainer can distinguish Go release checks from direct `bin/zero.js` wrapper checks. Keep Node usage limited to the wrapper itself and do not add Bun, TypeScript, or package build dependencies.", + "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "docs/NPM_WRAPPER_SMOKE.md" ], "verificationCommands": [ { - "id": "go-test-npm-release", + "id": "npm-wrapper-release-tests", "name": "NPM wrapper and release tests", "command": [ "go", @@ -48,17 +48,17 @@ ] }, { - "id": "provider-catalog-validation-test", - "name": "Add provider catalog validation coverage", - "description": "Focused test task for model registry catalog stability.", - "prompt": "Add focused tests for provider catalog defaults so model aliases and provider display names remain stable. Keep the change inside the model registry package.", - "workspaceFixture": "fixtures/zero-repo", + "id": "add-provider-catalog-stability-coverage", + "name": "Add provider catalog stability coverage", + "description": "Focused test task for model registry aliases and provider names.", + "prompt": "Add focused tests for provider catalog defaults so common model aliases and provider display names remain stable. Keep the change inside the model registry package.", + "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "internal/modelregistry/catalog_test.go" ], "verificationCommands": [ { - "id": "go-test-modelregistry", + "id": "modelregistry-tests", "name": "Model registry tests", "command": [ "go", @@ -67,6 +67,95 @@ ] } ] + }, + { + "id": "tighten-config-validation-errors", + "name": "Tighten config validation errors", + "description": "Small Go task for deterministic config validation behavior.", + "prompt": "Make config validation return actionable errors when the default provider is missing from the provider map, and cover the behavior with a table test. Do not change config file locations or environment variable names.", + "workspaceFixture": "fixtures/zero-mini", + "expectedChangedFiles": [ + "internal/config/validate.go", + "internal/config/validate_test.go" + ], + "verificationCommands": [ + { + "id": "config-tests", + "name": "Config package tests", + "command": [ + "go", + "test", + "./internal/config" + ] + } + ] + }, + { + "id": "preserve-mcp-server-order", + "name": "Preserve MCP server order", + "description": "Regression task for deterministic MCP configuration output.", + "prompt": "Add coverage showing MCP server names are emitted in deterministic sorted order when rendering config summaries. Keep the production change, if any, inside the MCP config package.", + "workspaceFixture": "fixtures/zero-mini", + "expectedChangedFiles": [ + "internal/mcp/config.go", + "internal/mcp/config_test.go" + ], + "verificationCommands": [ + { + "id": "mcp-tests", + "name": "MCP config tests", + "command": [ + "go", + "test", + "./internal/mcp" + ] + } + ] + }, + { + "id": "clarify-agent-permission-denial-copy", + "name": "Clarify agent permission denial copy", + "description": "Prompt and test task for local permission-denial messaging.", + "prompt": "Clarify the local agent system prompt so denied file operations explain the blocked path and the next safe action. Add a focused test that checks the denial guidance without depending on a live model.", + "workspaceFixture": "fixtures/zero-mini", + "expectedChangedFiles": [ + "internal/agent/system_prompt.md", + "internal/agent/denial_test.go" + ], + "verificationCommands": [ + { + "id": "agent-prompt-tests", + "name": "Agent prompt tests", + "command": [ + "go", + "test", + "./internal/agent" + ] + } + ] + }, + { + "id": "add-release-smoke-dry-run-note", + "name": "Add release smoke dry-run note", + "description": "Release-facing task that ties docs to local smoke behavior.", + "prompt": "Update the release smoke guidance and tests so maintainers can tell when a smoke run is using an already-built local binary. Keep the checks local and deterministic.", + "workspaceFixture": "fixtures/zero-mini", + "expectedChangedFiles": [ + "cmd/zero-release/main_test.go", + "docs/RELEASE_SMOKE.md" + ], + "verificationCommands": [ + { + "id": "release-command-tests", + "name": "Release command tests", + "command": [ + "go", + "test", + "./cmd/zero-release", + "./internal/release" + ] + } + ] } ] } diff --git a/internal/cli/agent_eval.go b/internal/cli/agent_eval.go index 39da9cd05..b54dfc398 100644 --- a/internal/cli/agent_eval.go +++ b/internal/cli/agent_eval.go @@ -2,30 +2,40 @@ package cli import ( "context" + "encoding/json" "fmt" "io" + "os" + "path/filepath" "strings" "github.com/Gitlawb/zero/internal/agenteval" ) type agentEvalOptions struct { - SuitePath string `json:"suite_path"` - JSON bool `json:"json"` + Mode string `json:"mode"` + SuitePath string `json:"suite_path"` + TaskID string `json:"task_id,omitempty"` + WorkspacePath string `json:"workspace_path,omitempty"` + ReportDir string `json:"report_dir,omitempty"` + JSON bool `json:"json"` } type agentEvalReport struct { - Suite string `json:"suite"` - Name string `json:"name,omitempty"` - Status string `json:"status,omitempty"` - OK bool `json:"ok"` - Tasks int `json:"tasks"` - Checks int `json:"checks"` - Total int `json:"total"` - Passed int `json:"passed"` - Failed int `json:"failed"` - Errors int `json:"errors"` - Failures []agentEvalFailure `json:"failures,omitempty"` + Suite string `json:"suite"` + Name string `json:"name,omitempty"` + TaskID string `json:"task_id,omitempty"` + Status string `json:"status,omitempty"` + OK bool `json:"ok"` + Tasks int `json:"tasks"` + Checks int `json:"checks"` + Total int `json:"total"` + Passed int `json:"passed"` + Failed int `json:"failed"` + Blocked int `json:"blocked"` + Errors int `json:"errors"` + ReportPath string `json:"report_path,omitempty"` + Failures []agentEvalFailure `json:"failures,omitempty"` } type agentEvalFailure struct { @@ -48,6 +58,12 @@ func runAgentEvalCommand(args []string, stdout io.Writer, stderr io.Writer, deps if err != nil { return writeExecUsageError(stderr, err.Error()) } + if options.ReportDir != "" { + report.ReportPath = filepath.Join(options.ReportDir, "agent-eval-report.json") + if err := writeAgentEvalReportFile(options.ReportDir, report); err != nil { + return writeAppError(stderr, "failed to write eval report: "+err.Error(), exitCrash) + } + } if options.JSON { if err := writePrettyJSON(stdout, report); err != nil { return exitCrash @@ -62,7 +78,14 @@ func runAgentEvalCommand(args []string, stdout io.Writer, stderr io.Writer, deps } func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { - options := agentEvalOptions{} + options := agentEvalOptions{Mode: "validate"} + if len(args) > 0 { + switch args[0] { + case "run", "validate": + options.Mode = args[0] + args = args[1:] + } + } for index := 0; index < len(args); index++ { arg := args[index] switch { @@ -79,6 +102,63 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { index = next case strings.HasPrefix(arg, "--suite="): options.SuitePath = strings.TrimSpace(strings.TrimPrefix(arg, "--suite=")) + case arg == "--task": + if options.Mode != "run" { + return options, false, execUsageError{"--task is only valid for eval run"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.TaskID = strings.TrimSpace(value) + index = next + case strings.HasPrefix(arg, "--task="): + if options.Mode != "run" { + return options, false, execUsageError{"--task is only valid for eval run"} + } + value, err := requiredInlineFlagValue(arg, "--task") + if err != nil { + return options, false, err + } + options.TaskID = strings.TrimSpace(value) + case arg == "--workspace": + if options.Mode != "run" { + return options, false, execUsageError{"--workspace is only valid for eval run"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.WorkspacePath = strings.TrimSpace(value) + index = next + case strings.HasPrefix(arg, "--workspace="): + if options.Mode != "run" { + return options, false, execUsageError{"--workspace is only valid for eval run"} + } + value, err := requiredInlineFlagValue(arg, "--workspace") + if err != nil { + return options, false, err + } + options.WorkspacePath = strings.TrimSpace(value) + case arg == "--report-dir": + if options.Mode != "run" { + return options, false, execUsageError{"--report-dir is only valid for eval run"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.ReportDir = strings.TrimSpace(value) + index = next + case strings.HasPrefix(arg, "--report-dir="): + if options.Mode != "run" { + return options, false, execUsageError{"--report-dir is only valid for eval run"} + } + value, err := requiredInlineFlagValue(arg, "--report-dir") + if err != nil { + return options, false, err + } + options.ReportDir = strings.TrimSpace(value) case strings.HasPrefix(arg, "-"): return options, false, execUsageError{fmt.Sprintf("unknown eval flag %q", arg)} default: @@ -88,6 +168,9 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { if options.SuitePath == "" { return options, false, execUsageError{"--suite requires a path"} } + if options.Mode == "run" && options.WorkspacePath == "" { + options.WorkspacePath = "." + } return options, false, nil } @@ -99,10 +182,13 @@ func formatAgentEvalReport(report agentEvalReport) string { if report.Name != "" { lines = append(lines, "name: "+report.Name) } + if report.TaskID != "" { + lines = append(lines, "task: "+report.TaskID) + } if report.Tasks > 0 || report.Checks > 0 { lines = append(lines, fmt.Sprintf("summary: %d tasks, %d checks", report.Tasks, report.Checks)) } else { - lines = append(lines, fmt.Sprintf("summary: %d total, %d passed, %d failed, %d errors", report.Total, report.Passed, report.Failed, report.Errors)) + lines = append(lines, fmt.Sprintf("summary: %d total, %d passed, %d failed, %d blocked, %d errors", report.Total, report.Passed, report.Failed, report.Blocked, report.Errors)) } status := strings.TrimSpace(report.Status) if status == "" { @@ -113,6 +199,12 @@ func formatAgentEvalReport(report agentEvalReport) string { } } lines = append(lines, "status: "+status) + if report.ReportPath != "" { + lines = append(lines, "report: "+report.ReportPath) + } + if len(report.Failures) > 0 { + lines = append(lines, "failures:") + } for _, failure := range report.Failures { detail := strings.TrimSpace(failure.ID) message := strings.TrimSpace(failure.Message) @@ -130,22 +222,33 @@ func formatAgentEvalReport(report agentEvalReport) string { func writeAgentEvalHelp(w io.Writer) error { _, err := fmt.Fprint(w, `Usage: zero eval --suite [flags] + zero eval run --suite [flags] -Validates offline agent eval suites for maintainers. +Validates offline agent eval suites and runs local scoring when a runner is wired in. Flags: --suite Eval suite JSON file + --task Run one task (eval run only) + --workspace Workspace path for local scoring (eval run only, default ".") + --report-dir Write agent-eval-report.json (eval run only) --json Print JSON output -h, --help Show this help `) return err } -func defaultRunAgentEval(_ context.Context, options agentEvalOptions) (agentEvalReport, error) { +func defaultRunAgentEval(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { suite, err := agenteval.LoadSuite(options.SuitePath) if err != nil { return agentEvalReport{}, err } + if options.Mode == "run" { + report := (agenteval.Runner{}).Run(ctx, suite, agenteval.RunInput{ + TaskID: options.TaskID, + WorkspacePath: options.WorkspacePath, + }) + return agentEvalReportFromRunner(suite, report), nil + } checks := 0 for _, task := range suite.Tasks { // Every task has N verification commands plus one changed-file expectation @@ -161,3 +264,58 @@ func defaultRunAgentEval(_ context.Context, options agentEvalOptions) (agentEval Checks: checks, }, nil } + +func agentEvalReportFromRunner(suite agenteval.Suite, report agenteval.Report) agentEvalReport { + converted := agentEvalReport{ + Suite: report.SuiteID, + Name: suite.Name, + TaskID: report.TaskID, + Status: string(report.Status), + OK: report.OK, + Total: report.Summary.Total, + Passed: report.Summary.Passed, + Failed: report.Summary.Failed, + Blocked: report.Summary.Blocked, + Errors: report.Summary.Errors, + } + if converted.Suite == "" { + converted.Suite = suite.ID + } + if report.Error != "" { + converted.Failures = append(converted.Failures, agentEvalFailure{ + ID: "task", + Message: report.Error, + }) + } + for _, result := range report.Results { + if result.Status == agenteval.StatusPass { + continue + } + converted.Failures = append(converted.Failures, agentEvalFailure{ + ID: result.ID, + Message: agentEvalResultMessage(result), + }) + } + return converted +} + +func agentEvalResultMessage(result agenteval.Result) string { + for _, value := range []string{result.Message, result.Stderr, string(result.Status)} { + value = strings.TrimSpace(value) + if value != "" { + return value + } + } + return "" +} + +func writeAgentEvalReportFile(reportDir string, report agentEvalReport) error { + if err := os.MkdirAll(reportDir, 0o755); err != nil { + return err + } + data, err := json.MarshalIndent(report, "", " ") + if err != nil { + return err + } + return os.WriteFile(report.ReportPath, append(data, '\n'), 0o644) +} diff --git a/internal/cli/agent_eval_test.go b/internal/cli/agent_eval_test.go index 94a767048..dd94a7679 100644 --- a/internal/cli/agent_eval_test.go +++ b/internal/cli/agent_eval_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -62,7 +63,7 @@ func TestRunEvalJSONMode(t *testing.T) { exitCode := runWithDeps([]string{"eval", "--suite", "evals/context.yaml", "--json"}, &stdout, &stderr, appDeps{ runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { - if options.SuitePath != "evals/context.yaml" || !options.JSON { + if options.Mode != "validate" || options.SuitePath != "evals/context.yaml" || !options.JSON { t.Fatalf("unexpected eval options: %#v", options) } return report, nil @@ -91,13 +92,187 @@ func TestRunEvalJSONMode(t *testing.T) { t.Fatalf("expected JSON key %q in %s", key, stdout.String()) } } - for _, key := range []string{"tasks", "checks", "failed", "errors"} { + for _, key := range []string{"tasks", "checks", "failed", "blocked", "errors"} { if string(raw[key]) != "0" { t.Fatalf("expected JSON key %q to be zero, got %s", key, string(raw[key])) } } } +func TestRunEvalRunJSONModePassesRunnerOptions(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{ + "eval", "run", + "--suite", "evals/context.json", + "--task", "edit-reader", + "--workspace", "D:\\work\\zero-fixture", + "--json", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Mode != "run" || options.SuitePath != "evals/context.json" || options.TaskID != "edit-reader" || options.WorkspacePath != "D:\\work\\zero-fixture" || !options.JSON { + t.Fatalf("unexpected eval run options: %#v", options) + } + return agentEvalReport{ + Suite: "quality-context", + TaskID: "edit-reader", + Status: "pass", + OK: true, + Total: 2, + Passed: 2, + }, nil + }, + }) + + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } + var decoded agentEvalReport + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("decode eval run JSON: %v\n%s", err, stdout.String()) + } + if decoded.TaskID != "edit-reader" || decoded.Status != "pass" || decoded.Blocked != 0 { + t.Fatalf("unexpected eval run JSON: %#v", decoded) + } +} + +func TestRunEvalRunFailureTextShowsSummaryAndFailures(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{"eval", "run", "--suite=evals/context.json", "--task=edit-reader"}, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Mode != "run" || options.TaskID != "edit-reader" || options.WorkspacePath != "." { + t.Fatalf("unexpected eval run options: %#v", options) + } + return agentEvalReport{ + Suite: "quality-context", + TaskID: "edit-reader", + Status: "fail", + OK: false, + Total: 3, + Passed: 1, + Failed: 1, + Blocked: 1, + Failures: []agentEvalFailure{{ + ID: "test", + Message: "go test ./... exited 1", + }}, + }, nil + }, + }) + + if exitCode != exitProvider { + t.Fatalf("expected provider-style failure exit %d, got %d", exitProvider, exitCode) + } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } + for _, want := range []string{ + "suite: quality-context", + "task: edit-reader", + "status: fail", + "summary: 3 total, 1 passed, 1 failed, 1 blocked, 0 errors", + "failures:", + "test - go test ./... exited 1", + } { + if !strings.Contains(stdout.String(), want) { + t.Fatalf("expected output to contain %q, got:\n%s", want, stdout.String()) + } + } +} + +func TestRunEvalRunReportDirWritesJSONReport(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + reportDir := t.TempDir() + + exitCode := runWithDeps([]string{"eval", "run", "--suite", "evals/context.json", "--report-dir", reportDir}, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.ReportDir != reportDir { + t.Fatalf("unexpected report dir: %#v", options) + } + return agentEvalReport{ + Suite: "quality-context", + TaskID: "edit-reader", + Status: "pass", + OK: true, + Total: 1, + Passed: 1, + }, nil + }, + }) + + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } + reportPath := filepath.Join(reportDir, "agent-eval-report.json") + data, err := os.ReadFile(reportPath) + if err != nil { + t.Fatalf("read report: %v", err) + } + var decoded agentEvalReport + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("decode report: %v\n%s", err, string(data)) + } + if decoded.ReportPath != reportPath || decoded.Suite != "quality-context" { + t.Fatalf("unexpected written report: %#v", decoded) + } + if !strings.Contains(stdout.String(), "report: "+reportPath) { + t.Fatalf("expected text output to mention report path, got:\n%s", stdout.String()) + } +} + +func TestRunEvalRunDefaultRunnerUsesAgentEvalRunner(t *testing.T) { + workspace := t.TempDir() + if output, err := exec.Command("git", "-C", workspace, "init").CombinedOutput(); err != nil { + t.Fatalf("git init: %v\n%s", err, string(output)) + } + if err := os.WriteFile(filepath.Join(workspace, "expected.txt"), []byte("changed"), 0o600); err != nil { + t.Fatalf("write expected file: %v", err) + } + suitePath := filepath.Join(t.TempDir(), "suite.json") + if err := os.WriteFile(suitePath, []byte(`{ + "id": "runner-cli", + "name": "Runner CLI", + "tasks": [{ + "id": "local-score", + "name": "Local score", + "prompt": "Touch the expected file.", + "workspaceFixture": "fixtures/runner", + "verificationCommands": [ + {"id": "go-version", "name": "Go version", "command": ["go", "version"]} + ], + "expectedChangedFiles": ["expected.txt"] + }] + }`), 0o600); err != nil { + t.Fatalf("write suite: %v", err) + } + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{"eval", "run", "--suite", suitePath, "--workspace", workspace}, &stdout, &stderr, appDeps{}) + + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s\n%s", exitSuccess, exitCode, stderr.String(), stdout.String()) + } + for _, want := range []string{ + "suite: runner-cli", + "name: Runner CLI", + "task: local-score", + "summary: 2 total, 2 passed, 0 failed, 0 blocked, 0 errors", + "status: pass", + } { + if !strings.Contains(stdout.String(), want) { + t.Fatalf("expected output to contain %q, got:\n%s", want, stdout.String()) + } + } +} + func TestRunEvalDefaultRunnerLoadsSuite(t *testing.T) { path := filepath.Join(t.TempDir(), "suite.json") if err := os.WriteFile(path, []byte(`{ From 97fcc38f73c981cb6c33a10fc531b2060066d9c7 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Fri, 12 Jun 2026 14:50:45 +0530 Subject: [PATCH 2/5] Address agent eval fixture review feedback --- .../zero-mini/cmd/zero-release/main.go | 32 +++++++++++++++++-- .../zero-mini/cmd/zero-release/main_test.go | 26 ++++++++++++++- .../zero-mini/internal/config/validate.go | 8 ++++- .../internal/config/validate_test.go | 25 +++++++++++++-- .../internal/selfverify/selfverify_test.go | 13 +++++--- .../zero-mini/internal/verify/verify_test.go | 8 +++-- 6 files changed, 98 insertions(+), 14 deletions(-) diff --git a/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go index 3cb77af02..f03d0a9bd 100644 --- a/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go +++ b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main.go @@ -1,9 +1,37 @@ package main -import "github.com/Gitlawb/zero-fixture/internal/release" +import ( + "errors" + "fmt" + "io" + "os" + + "github.com/Gitlawb/zero-fixture/internal/release" +) func smokeTarget(alreadyBuilt bool) string { return release.SmokeTarget(alreadyBuilt) } -func main() {} +func run(args []string, stdout io.Writer) error { + if len(args) == 0 { + return errors.New("command required: build or smoke") + } + switch args[0] { + case "build": + _, err := fmt.Fprintln(stdout, smokeTarget(false)) + return err + case "smoke": + _, err := fmt.Fprintln(stdout, smokeTarget(true)) + return err + default: + return fmt.Errorf("unknown command %q", args[0]) + } +} + +func main() { + if err := run(os.Args[1:], os.Stdout); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go index 50afd64e9..82f831ad8 100644 --- a/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go +++ b/internal/agenteval/testdata/fixtures/zero-mini/cmd/zero-release/main_test.go @@ -1,9 +1,33 @@ package main -import "testing" +import ( + "bytes" + "strings" + "testing" +) func TestSmokeTargetUsesLocalBinary(t *testing.T) { if got := smokeTarget(true); got != "local-binary" { t.Fatalf("smokeTarget(true) = %q, want local-binary", got) } } + +func TestRunSmokeUsesReleasePath(t *testing.T) { + var stdout bytes.Buffer + if err := run([]string{"smoke"}, &stdout); err != nil { + t.Fatalf("run smoke: %v", err) + } + if got := strings.TrimSpace(stdout.String()); got != "local-binary" { + t.Fatalf("run smoke output = %q, want local-binary", got) + } +} + +func TestRunBuildUsesReleasePath(t *testing.T) { + var stdout bytes.Buffer + if err := run([]string{"build"}, &stdout); err != nil { + t.Fatalf("run build: %v", err) + } + if got := strings.TrimSpace(stdout.String()); got != "build-then-smoke" { + t.Fatalf("run build output = %q, want build-then-smoke", got) + } +} diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go index cf371a2c2..11ee02c0c 100644 --- a/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate.go @@ -1,6 +1,9 @@ package config -import "errors" +import ( + "errors" + "fmt" +) type Config struct { DefaultProvider string @@ -14,5 +17,8 @@ func Validate(cfg Config) error { if len(cfg.Providers) == 0 { return errors.New("providers are required") } + if _, ok := cfg.Providers[cfg.DefaultProvider]; !ok { + return fmt.Errorf("default provider %q is not configured", cfg.DefaultProvider) + } return nil } diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go index 79af54fae..4e7f1bc8d 100644 --- a/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/config/validate_test.go @@ -3,8 +3,27 @@ package config import "testing" func TestValidateRequiresDefaultProvider(t *testing.T) { - err := Validate(Config{Providers: map[string]string{"local": "fixture"}}) - if err == nil { - t.Fatal("Validate returned nil, want missing default provider error") + tests := []struct { + name string + cfg Config + }{ + { + name: "empty default", + cfg: Config{Providers: map[string]string{"local": "fixture"}}, + }, + { + name: "unknown default", + cfg: Config{ + DefaultProvider: "missing", + Providers: map[string]string{"local": "fixture"}, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := Validate(test.cfg); err == nil { + t.Fatal("Validate returned nil, want provider validation error") + } + }) } } diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go index 8195a76a0..dd99a0e0c 100644 --- a/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/selfverify/selfverify_test.go @@ -1,11 +1,14 @@ package selfverify -import "testing" +import ( + "reflect" + "testing" +) func TestChecksAreLocal(t *testing.T) { - for _, check := range Checks() { - if check == "" { - t.Fatal("check names must not be empty") - } + got := Checks() + want := []string{"version", "fixtures"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("Checks() = %#v, want %#v", got, want) } } diff --git a/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go b/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go index ab361f50e..66d388dc2 100644 --- a/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go +++ b/internal/agenteval/testdata/fixtures/zero-mini/internal/verify/verify_test.go @@ -4,7 +4,11 @@ import "testing" func TestEventsIncludeSummary(t *testing.T) { events := Events() - if len(events) == 0 || events[len(events)-1].Type != "summary" { - t.Fatalf("events = %#v, want trailing summary", events) + if len(events) == 0 { + t.Fatalf("events = %#v, want non-empty", events) + } + last := events[len(events)-1] + if last.Type != "summary" || last.Name != "verify" { + t.Fatalf("last event = %#v, want Type %q and Name %q", last, "summary", "verify") } } From fbfffb69e2b4b4335ceb6d685ef84397253934c7 Mon Sep 17 00:00:00 2001 From: gnanam1990 Date: Fri, 12 Jun 2026 21:14:43 +0530 Subject: [PATCH 3/5] agent eval: require --workspace for eval run; bound + cancel verification commands (review #178) Addresses the two runtime-boundary findings: 1. eval run no longer defaults --workspace to '.', which silently ran the suite's verification commands (go test/git) against the real working tree. It is now required; omitting it is a usage error. Docs and help text updated. 2. Wire cancellation + timeouts: runAgentEvalCommand runs under signalContext() so Ctrl+C/SIGTERM cancels in-flight commands, and the runner now applies a per-command timeout (default 10m, overridable via RunInput.CommandTimeout) so a hung verification command can't run unbounded. Tests: require-workspace usage error, cancellable-context wiring, and a per-command-deadline regression. --- docs/AGENT_EVALS.md | 5 ++-- internal/agenteval/run.go | 22 +++++++++++++++-- internal/agenteval/run_test.go | 28 +++++++++++++++++++++ internal/cli/agent_eval.go | 13 +++++++--- internal/cli/agent_eval_test.go | 44 ++++++++++++++++++++++++++++++--- 5 files changed, 102 insertions(+), 10 deletions(-) diff --git a/docs/AGENT_EVALS.md b/docs/AGENT_EVALS.md index a84008227..1cc546625 100644 --- a/docs/AGENT_EVALS.md +++ b/docs/AGENT_EVALS.md @@ -57,8 +57,9 @@ go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json --j point it at a Git worktree where a fixture has already been copied and a task has already been attempted. The runner executes each `verificationCommands` entry, collects changed files with `git status --porcelain`, and emits the -task-success report contract below. When `--workspace` is omitted, the current -directory is used. +task-success report contract below. `--workspace` is required: it must point at +the prepared fixture worktree, never the current directory, so the suite's +verification commands (`go test`, `git`, …) don't run against your real repo. ```bash go run ./cmd/zero eval run \ diff --git a/internal/agenteval/run.go b/internal/agenteval/run.go index ecfe05eb7..713e0773f 100644 --- a/internal/agenteval/run.go +++ b/internal/agenteval/run.go @@ -9,11 +9,20 @@ import ( "os/exec" "path/filepath" "strings" + "time" ) +// defaultCommandTimeout bounds a single verification command so a hung command +// (e.g. a test that waits on input) cannot run unbounded. Generous enough for +// real build/test commands; overridable per run via RunInput.CommandTimeout. +const defaultCommandTimeout = 10 * time.Minute + type RunInput struct { TaskID string WorkspacePath string + // CommandTimeout bounds each verification command. Non-positive applies + // defaultCommandTimeout. + CommandTimeout time.Duration } type CommandRunner func(context.Context, string, Command) CommandResult @@ -43,12 +52,16 @@ func (runner Runner) Run(ctx context.Context, suite Suite, input RunInput) Repor return runner.blocked(suite, task.ID, err.Error(), nil) } + timeout := input.CommandTimeout + if timeout <= 0 { + timeout = defaultCommandTimeout + } results := make([]CommandResult, 0, len(task.VerificationCommands)) for _, command := range task.VerificationCommands { if err := ctx.Err(); err != nil { return runner.blocked(suite, task.ID, err.Error(), results) } - result := runner.runCommand(ctx, workspace, command) + result := runner.runCommand(ctx, workspace, command, timeout) if result.ID == "" { result.ID = command.ID } @@ -77,7 +90,12 @@ func (runner Runner) blocked(suite Suite, taskID string, reason string, results }) } -func (runner Runner) runCommand(ctx context.Context, workspace string, command Command) CommandResult { +func (runner Runner) runCommand(ctx context.Context, workspace string, command Command, timeout time.Duration) CommandResult { + if timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } if runner.RunCommand != nil { return runner.RunCommand(ctx, workspace, command) } diff --git a/internal/agenteval/run_test.go b/internal/agenteval/run_test.go index 6eb0ea816..3985437c2 100644 --- a/internal/agenteval/run_test.go +++ b/internal/agenteval/run_test.go @@ -50,6 +50,34 @@ func TestRunnerMapsPassingAndFailingCommandResults(t *testing.T) { } } +func TestRunnerBoundsCommandsWithDefaultTimeout(t *testing.T) { + workspace := t.TempDir() + var hadDeadline bool + runner := Runner{ + RunCommand: func(ctx context.Context, _ string, command Command) CommandResult { + _, hadDeadline = ctx.Deadline() + return CommandResult{ID: command.ID, ExitCode: 0} + }, + ChangedFiles: func(context.Context, string) ([]string, error) { + return []string{"internal/reader/a.go"}, nil + }, + } + + // No CommandTimeout set: a per-command deadline must still apply by default so a + // hung verification command cannot run unbounded. + report := runner.Run(context.Background(), runnerSuite(), RunInput{ + TaskID: "two-commands", + WorkspacePath: workspace, + }) + + if report.Status == StatusError { + t.Fatalf("unexpected error report: %#v", report) + } + if !hadDeadline { + t.Fatal("verification commands must run under a per-command deadline by default") + } +} + func TestRunnerBlocksWhenWorkspaceSetupFails(t *testing.T) { missing := filepath.Join(t.TempDir(), "missing") calledCommand := false diff --git a/internal/cli/agent_eval.go b/internal/cli/agent_eval.go index b54dfc398..fdb6e02d1 100644 --- a/internal/cli/agent_eval.go +++ b/internal/cli/agent_eval.go @@ -54,7 +54,11 @@ func runAgentEvalCommand(args []string, stdout io.Writer, stderr io.Writer, deps } return exitSuccess } - report, err := deps.runAgentEval(context.Background(), options) + // Run under a signal-aware context so Ctrl+C / SIGTERM cancels in-flight + // verification commands (which honor context cancellation) instead of leaking them. + ctx, stop := signalContext() + defer stop() + report, err := deps.runAgentEval(ctx, options) if err != nil { return writeExecUsageError(stderr, err.Error()) } @@ -169,7 +173,10 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { return options, false, execUsageError{"--suite requires a path"} } if options.Mode == "run" && options.WorkspacePath == "" { - options.WorkspacePath = "." + // Require an explicit workspace: defaulting to "." would run the suite's + // verification commands (go test/git) against the real working tree instead + // of a staged fixture. + return options, false, execUsageError{"--workspace requires a path for eval run"} } return options, false, nil } @@ -229,7 +236,7 @@ Validates offline agent eval suites and runs local scoring when a runner is wire Flags: --suite Eval suite JSON file --task Run one task (eval run only) - --workspace Workspace path for local scoring (eval run only, default ".") + --workspace Workspace path for local scoring (eval run only, required) --report-dir Write agent-eval-report.json (eval run only) --json Print JSON output -h, --help Show this help diff --git a/internal/cli/agent_eval_test.go b/internal/cli/agent_eval_test.go index dd94a7679..82e9c45aa 100644 --- a/internal/cli/agent_eval_test.go +++ b/internal/cli/agent_eval_test.go @@ -140,13 +140,51 @@ func TestRunEvalRunJSONModePassesRunnerOptions(t *testing.T) { } } +func TestRunEvalRunRequiresExplicitWorkspace(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{"eval", "run", "--suite", "evals/context.json", "--task", "edit-reader"}, &stdout, &stderr, appDeps{ + runAgentEval: func(context.Context, agentEvalOptions) (agentEvalReport, error) { + t.Fatal("eval run must not execute without an explicit --workspace (would run against the real working tree)") + return agentEvalReport{}, nil + }, + }) + + if exitCode != exitUsage { + t.Fatalf("expected usage exit %d, got %d", exitUsage, exitCode) + } + if !strings.Contains(stderr.String(), "--workspace") { + t.Fatalf("expected a --workspace required error, got %q", stderr.String()) + } +} + +func TestRunEvalRunsUnderCancellableContext(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + cancellable := false + + runWithDeps([]string{"eval", "--suite", "evals/context.json"}, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, _ agentEvalOptions) (agentEvalReport, error) { + // signal.NotifyContext yields a cancellable context (non-nil Done); + // context.Background().Done() is nil. + cancellable = ctx.Done() != nil + return agentEvalReport{Suite: "quality-context", OK: true}, nil + }, + }) + + if !cancellable { + t.Fatal("eval must run under a cancellable signal context, not context.Background()") + } +} + func TestRunEvalRunFailureTextShowsSummaryAndFailures(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer - exitCode := runWithDeps([]string{"eval", "run", "--suite=evals/context.json", "--task=edit-reader"}, &stdout, &stderr, appDeps{ + exitCode := runWithDeps([]string{"eval", "run", "--suite=evals/context.json", "--task=edit-reader", "--workspace", "fixture-dir"}, &stdout, &stderr, appDeps{ runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { - if options.Mode != "run" || options.TaskID != "edit-reader" || options.WorkspacePath != "." { + if options.Mode != "run" || options.TaskID != "edit-reader" || options.WorkspacePath != "fixture-dir" { t.Fatalf("unexpected eval run options: %#v", options) } return agentEvalReport{ @@ -191,7 +229,7 @@ func TestRunEvalRunReportDirWritesJSONReport(t *testing.T) { var stderr bytes.Buffer reportDir := t.TempDir() - exitCode := runWithDeps([]string{"eval", "run", "--suite", "evals/context.json", "--report-dir", reportDir}, &stdout, &stderr, appDeps{ + exitCode := runWithDeps([]string{"eval", "run", "--suite", "evals/context.json", "--workspace", "fixture-dir", "--report-dir", reportDir}, &stdout, &stderr, appDeps{ runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { if options.ReportDir != reportDir { t.Fatalf("unexpected report dir: %#v", options) From e8d68d108187a32cc068fc49c9de58934db876a6 Mon Sep 17 00:00:00 2001 From: Vasanth T <148849890+Vasanthdev2004@users.noreply.github.com> Date: Sat, 13 Jun 2026 10:33:57 +0530 Subject: [PATCH 4/5] Add agent eval benchmark harness (#180) * Add agent eval benchmark harness * Upgrade agent eval quality signals * Address agent eval benchmark review feedback Blocking review items: - Bound each bench task with a --timeout flag + BenchmarkInput.Timeout, threading a per-task context; report a clear timeout error instead of "exited with code -1". - Cap captured agent stdout/stderr per stream (default 1 MiB, configurable) and flag truncation via AgentRunResult.Truncated, surfaced in the CLI report (text note + JSON). - Show scored pass/fail/blocked/error tallies for bench/run text output (formatAgentEvalReport now keys the validate-style summary on Checks). - Clean up materialized temp workspaces on copy/git-baseline failure instead of leaking them. - Cover unknown-task-id, materialization-failure, per-task-timeout, and timeout-cancellation paths with tests. Lows/nits: - Reject symlink/non-regular fixture entries instead of silently dropping them. - Check copyFixtureFile's target.Close()/source.Close(); fix cleanup-defer errcheck in benchmark.go and cli. - Surface the work root when --keep-workspaces is set so kept dirs are findable. - Route work-root creation failures to the crash exit code, not the usage code. - Avoid re-expanding injected placeholder values via strings.NewReplacer. - Document that committing agent changes defeats changed-files scoring; preserve executable bits; add work-root and truncation tests. * Make timeout-cancellation test strict Use a generous (2s) per-task timeout so the agent is always reached, then hard-assert that it observed cancellation and the task is blocked, instead of gating those assertions behind agentReached (which let a pre-agent timeout regression pass silently). --------- Co-authored-by: gnanam1990 --- docs/AGENT_EVALS.md | 188 ++++++- ...2026-06-12-agent-eval-benchmark-harness.md | 304 +++++++++++ .../2026-06-12-agent-eval-d-level-upgrades.md | 181 ++++++ internal/agenteval/agent_command.go | 135 +++++ internal/agenteval/agent_command_test.go | 240 ++++++++ internal/agenteval/agenteval_test.go | 126 ++++- internal/agenteval/benchmark.go | 219 ++++++++ internal/agenteval/benchmark_test.go | 513 ++++++++++++++++++ internal/agenteval/context_checks.go | 105 ++++ internal/agenteval/context_checks_test.go | 50 ++ internal/agenteval/materialize.go | 194 +++++++ internal/agenteval/materialize_test.go | 220 ++++++++ internal/agenteval/run.go | 20 +- internal/agenteval/score.go | 125 ++++- internal/agenteval/suite.go | 78 ++- internal/agenteval/testdata/sample_suite.json | 205 ++++++- internal/agenteval/trace.go | 85 +++ internal/agenteval/trace_test.go | 31 ++ internal/cli/agent_eval.go | 395 ++++++++++++-- internal/cli/agent_eval_test.go | 488 +++++++++++++++++ 20 files changed, 3802 insertions(+), 100 deletions(-) create mode 100644 docs/superpowers/plans/2026-06-12-agent-eval-benchmark-harness.md create mode 100644 docs/superpowers/plans/2026-06-12-agent-eval-d-level-upgrades.md create mode 100644 internal/agenteval/agent_command.go create mode 100644 internal/agenteval/agent_command_test.go create mode 100644 internal/agenteval/benchmark.go create mode 100644 internal/agenteval/benchmark_test.go create mode 100644 internal/agenteval/context_checks.go create mode 100644 internal/agenteval/context_checks_test.go create mode 100644 internal/agenteval/materialize.go create mode 100644 internal/agenteval/materialize_test.go create mode 100644 internal/agenteval/trace.go create mode 100644 internal/agenteval/trace_test.go diff --git a/docs/AGENT_EVALS.md b/docs/AGENT_EVALS.md index 1cc546625..d685dd66e 100644 --- a/docs/AGENT_EVALS.md +++ b/docs/AGENT_EVALS.md @@ -6,9 +6,10 @@ expected to change, the commands that should verify the result, and the scoring rules an offline harness can apply to a captured run. These fixtures are intentionally local-first. They do not prove provider quality -or live model execution by themselves; they give tests and future CLI work a +or live model execution by themselves; they give tests and CLI workflows a stable sample suite to validate, run against copied workspaces, and score from -saved outputs. +saved outputs. The eval harness is local and offline-testable. It only makes +live model calls when the supplied agent command does. ## Suite Format @@ -26,22 +27,48 @@ Task fields used by the sample suite: - `id`: stable task identifier for filters and reports. - `name` and `description`: short task metadata. +- `tags`: stable category labels such as `docs`, `go`, or `wrapper`. +- `difficulty`: a coarse task size such as `easy`, `medium`, or `hard`. - `prompt`: the user request to give an agent. - `workspaceFixture`: the fixture workspace to copy before running the task. - `expectedChangedFiles`: files that should change for a complete solution. +- `forbiddenChangedFiles`: files that must not change during the task. +- `requiredTraceEvents`: JSONL agent events that must appear in benchmark + stdout. +- `contextChecks`: required and forbidden files checked in the materialized + workspace before verification commands run. - `verificationCommands`: commands a maintainer or harness can run after the agent output is applied. -The current scoring contract is deliberately small: command results are matched -by `verificationCommands[].id`, and changed files are compared against -`expectedChangedFiles`. Extra fields should not be added to suite JSON unless -the loader and tests are updated in the same PR. +The scoring contract matches command results by `verificationCommands[].id`, +compares changed files against `expectedChangedFiles`, rejects any +`forbiddenChangedFiles`, checks the materialized workspace with `contextChecks`, +and can require agent trace events during benchmark runs. The loader rejects +unknown JSON fields so suite changes fail fast. + +Example richer task rubric: + +```json +{ + "tags": ["docs", "jsonl"], + "difficulty": "easy", + "forbiddenChangedFiles": ["go.mod", "package.json"], + "requiredTraceEvents": ["tool:read_file", "verify:go-test"], + "contextChecks": { + "requiredFiles": ["docs/STREAM_JSON_PROTOCOL.md"], + "forbiddenFiles": ["node_modules/cache.txt"] + } +} +``` ## Modes -`zero eval` defaults to validate mode. It parses the suite, rejects schema or -contract errors, and reports the number of tasks and checks. It does not copy -fixtures, invoke an agent, or execute verification commands. +### Validate Mode + +`zero eval` defaults to `validate` mode. In validate mode, the command performs +schema and contract checks only: it parses the suite, rejects invalid task +definitions, and reports the number of tasks and checks. It does not copy +fixtures, invoke an agent, score a workspace, or execute verification commands. ```bash go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json @@ -53,13 +80,16 @@ Use JSON output when another local tool needs the validation summary: go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json --json ``` -`zero eval run` scores one local workspace. It does not invoke the agent yet; -point it at a Git worktree where a fixture has already been copied and a task -has already been attempted. The runner executes each `verificationCommands` -entry, collects changed files with `git status --porcelain`, and emits the -task-success report contract below. `--workspace` is required: it must point at -the prepared fixture worktree, never the current directory, so the suite's -verification commands (`go test`, `git`, …) don't run against your real repo. +### Run Mode + +`zero eval run` scores one already-mutated Git workspace. It does not copy +fixtures or invoke an agent; point it at a Git worktree where a fixture has +already been copied, initialized, and attempted by an agent or deterministic +local script. The runner executes each `verificationCommands` entry, collects +changed files with `git status --porcelain`, and emits the task-success report +contract below. `--workspace` is required: it must point at the prepared fixture +worktree, never the current directory, so the suite's verification commands +(`go test`, `git`, ...) don't run against your real repo. ```bash go run ./cmd/zero eval run \ @@ -79,6 +109,113 @@ go run ./cmd/zero eval run \ --json ``` +### Bench Mode + +`zero eval bench` runs the full benchmark harness for one task or a suite. Bench +mode copies each task fixture into `--work-root`, initializes a clean Git +baseline, runs the supplied `--agent-command` in that workspace, then scores the +result with the same scorer used by run mode. + +Agent commands are passed as argv, without shell interpolation. The harness +expands these placeholders in each argument: + +- `{workspace}`: copied task workspace path. +- `{prompt}`: task prompt from the suite. +- `{task_id}`: selected task ID. +- `{model}`: current model ID for model-matrix benchmark runs. + +Use `--model ` more than once, or `--models a,b,c`, to run the same task +matrix across several models. When no model is supplied, the harness preserves +the previous single-run behavior and `{model}` expands to an empty string. + +Example using a real local agent command: + +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --work-root /tmp/zero-evals \ + --agent-command zero exec --cwd {workspace} {prompt} +``` + +Example with model selection: + +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --work-root /tmp/zero-evals \ + --model gpt-5 \ + --agent-command zero exec --model {model} --cwd {workspace} {prompt} +``` + +Include `{task_id}` when the agent wrapper needs stable per-task logging, +branching, or fixture-specific behavior: + +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --work-root /tmp/zero-evals \ + --agent-command zero-agent-wrapper --task {task_id} --workspace {workspace} --prompt {prompt} +``` + +The same wrapper can emit JSONL trace events to stdout for future trace scoring: + +```json +{"type":"tool","name":"read_file"} +{"event":"verify","name":"go-test"} +``` + +Those events are required by adding keys such as `tool:read_file` and +`verify:go-test` to `requiredTraceEvents`. + +For deterministic offline testing, point `--agent-command` at a local script +that edits the copied workspace without calling a model: + +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --work-root /tmp/zero-evals \ + --agent-command ./scripts/fake-agent --workspace {workspace} --task {task_id} --prompt {prompt} +``` + +Bound a benchmark run with `--timeout` (a Go duration) so a wedged or +interactive agent cannot block the harness forever. The timeout applies per +task and cancels materialization, the agent process, and scoring: + +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --work-root /tmp/zero-evals \ + --timeout 5m \ + --agent-command ./scripts/fake-agent --workspace {workspace} --prompt {prompt} +``` + +Use `--report-dir` to persist the CLI report artifact. The file is always named +`agent-eval-report.json`; with bench mode it records the suite status, task +counts, pass/fail totals, failures, and the nested benchmark report with each +task/model run. Use `--keep-workspaces` when you also need to inspect the +materialized workspaces after the run: + +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task add-npm-wrapper-argv-helper \ + --work-root /tmp/zero-evals \ + --keep-workspaces \ + --report-dir /tmp/zero-eval-report \ + --json \ + --agent-command ./scripts/fake-agent --workspace {workspace} --task {task_id} --prompt {prompt} +``` + +**Scoring caveat:** changed-file scoring inspects the workspace with +`git status --porcelain` against the baseline commit. An agent that *commits* +its own changes (or otherwise leaves a clean working tree) defeats this check — +the committed edits no longer appear as changed files, so `expectedChangedFiles` +will not match. Agents under bench should leave their edits uncommitted. + Run the package tests when changing the suite schema or scorer: ```bash @@ -110,12 +247,15 @@ Scored reports use contract `zero.agenteval.report.v1`. - `ok`: true only when every result passes. - `summary`: total result counts by status. - `changedFiles`: normalized files collected from the workspace. -- `results`: one result per verification command, plus `changed_files`. +- `results`: one result per verification command, plus configured + `changed_files`, `forbidden_changed_files`, `context_checks`, and + `trace_events` checks. - `error`: task-selection or report-level error, when present. Command results include the command ID, display name, command argv, status, -exit code, stdout, stderr, and an optional message. The changed-files result -includes expected, actual, missing, and unexpected files. +exit code, stdout, stderr, and an optional message. File-based results include +expected, actual, missing, and unexpected files. Trace results include expected, +actual, and missing event keys. ## Score Interpretation @@ -123,10 +263,12 @@ Scores are offline quality signals, not pass/fail release gates by default. The statuses below are produced when a harness supplies captured command results and changed files. -- `pass`: every verification command exited successfully and the changed files - matched `expectedChangedFiles`. -- `fail`: at least one command failed or changed files were missing or - unexpected. +- `pass`: every verification command exited successfully, changed files matched + `expectedChangedFiles`, forbidden files stayed untouched, configured context + checks passed, and required trace events were present. +- `fail`: at least one command failed, changed files were missing or + unexpected, a forbidden file changed, a context file check failed, or a + required trace event was missing. - `blocked`: the harness could not run the task or collect the expected inputs. - `error`: the suite, task ID, command ID, or captured input could not be interpreted. diff --git a/docs/superpowers/plans/2026-06-12-agent-eval-benchmark-harness.md b/docs/superpowers/plans/2026-06-12-agent-eval-benchmark-harness.md new file mode 100644 index 000000000..6a6fbc34c --- /dev/null +++ b/docs/superpowers/plans/2026-06-12-agent-eval-benchmark-harness.md @@ -0,0 +1,304 @@ +# Agent Eval Benchmark Harness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Build a local, offline-testable benchmark harness that copies eval fixtures, runs an agent command per task, scores the resulting workspace, and reports aggregate results. + +**Architecture:** Keep `internal/agenteval.Runner` as the scoring primitive. Add a `Harness` layer that materializes fixture workspaces into a work directory, initializes a clean Git baseline, invokes an injectable `AgentRunner`, then calls `Runner.Run`. The CLI exposes this as `zero eval bench`, while `zero eval run` remains the lower-level scorer for an already-mutated workspace. + +**Tech Stack:** Go 1.24 standard library, existing `internal/agenteval`, existing `internal/cli`, Git CLI for workspace baselines. + +--- + +## File Structure + +- Create `internal/agenteval/materialize.go`: fixture path resolution, directory copy, Git init/baseline helpers. +- Create `internal/agenteval/materialize_test.go`: fixture copy and Git baseline tests. +- Create `internal/agenteval/agent_command.go`: command-template based agent runner with `{prompt}`, `{workspace}`, `{task_id}` placeholders. +- Create `internal/agenteval/agent_command_test.go`: placeholder substitution, cwd, exit/error behavior. +- Create `internal/agenteval/benchmark.go`: benchmark orchestration and aggregate report. +- Create `internal/agenteval/benchmark_test.go`: end-to-end harness with fake agent runner and sample fixtures. +- Modify `internal/cli/agent_eval.go`: parse `zero eval bench` and wire `Harness`. +- Modify `internal/cli/agent_eval_test.go`: bench parsing, JSON/text output, report writing. +- Modify `docs/AGENT_EVALS.md`: document `bench` mode and real local command examples. + +## Task 1: Fixture Materializer + +**Files:** +- Create: `internal/agenteval/materialize.go` +- Create: `internal/agenteval/materialize_test.go` + +- [ ] **Step 1: Write failing tests** + +```go +func TestMaterializeTaskCopiesFixtureAndInitializesGit(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + task := suite.Tasks[0] + workRoot := t.TempDir() + + workspace, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: workRoot}) + if err != nil { + t.Fatalf("MaterializeTask: %v", err) + } + if _, err := os.Stat(filepath.Join(workspace.Path, "go.mod")); err != nil { + t.Fatalf("fixture was not copied: %v", err) + } + if output, err := exec.Command("git", "-C", workspace.Path, "status", "--porcelain").CombinedOutput(); err != nil || strings.TrimSpace(string(output)) != "" { + t.Fatalf("workspace baseline is dirty: err=%v output=%s", err, output) + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/agenteval -run TestMaterializeTask -count=1` +Expected: FAIL because `Materializer` is undefined. + +- [ ] **Step 3: Implement minimal materializer** + +Create a `Materializer` with: +- `MaterializeTask(ctx, suitePath string, task Task, input MaterializeInput) (Workspace, error)` +- `MaterializeInput{WorkRoot string}` +- `Workspace{Path string, TaskID string, FixturePath string}` +- Resolve relative `task.WorkspaceFixture` from `filepath.Dir(suitePath)`. +- Copy directories recursively using stdlib only. +- Skip `.git` while copying. +- Run `git init`, `git add .`, and `git commit -m "baseline"` with local user config/env. +- Return clear errors for missing fixture, absolute fixture escapes, and empty work root. + +- [ ] **Step 4: Verify** + +Run: +```bash +go test ./internal/agenteval -run 'TestMaterializeTask|TestMaterializer' -count=1 +``` + +## Task 2: Agent Command Runner + +**Files:** +- Create: `internal/agenteval/agent_command.go` +- Create: `internal/agenteval/agent_command_test.go` + +- [ ] **Step 1: Write failing tests** + +```go +func TestCommandAgentRunnerExpandsPlaceholders(t *testing.T) { + dir := t.TempDir() + script := filepath.Join(dir, "agent.bat") + if err := os.WriteFile(script, []byte("@echo %1|%2|%3> out.txt\r\n"), 0o755); err != nil { + t.Fatal(err) + } + runner := CommandAgentRunner{Command: []string{script, "{task_id}", "{workspace}", "{prompt}"}} + result := runner.Run(context.Background(), AgentRunInput{TaskID: "task-a", WorkspacePath: dir, Prompt: "fix bug"}) + if result.ExitCode != 0 || result.Error != "" { + t.Fatalf("Run = %#v", result) + } + data, err := os.ReadFile(filepath.Join(dir, "out.txt")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), "task-a") || !strings.Contains(string(data), dir) || !strings.Contains(string(data), "fix bug") { + t.Fatalf("placeholders not expanded: %q", data) + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/agenteval -run TestCommandAgentRunner -count=1` +Expected: FAIL because `CommandAgentRunner` is undefined. + +- [ ] **Step 3: Implement runner** + +Add: +- `type AgentRunInput struct { TaskID, Prompt, WorkspacePath string }` +- `type AgentRunResult struct { ExitCode int; Stdout, Stderr, Error string }` +- `type AgentRunner interface { Run(context.Context, AgentRunInput) AgentRunResult }` +- `type CommandAgentRunner struct { Command []string }` +- `Run` executes without shell interpolation, with `cmd.Dir = WorkspacePath`. +- Replace `{prompt}`, `{workspace}`, `{task_id}` in every arg. +- Empty command returns `ExitCode:-1` and `Error:"agent command is required"`. + +- [ ] **Step 4: Verify** + +Run: `go test ./internal/agenteval -run TestCommandAgentRunner -count=1` + +## Task 3: Benchmark Harness + +**Files:** +- Create: `internal/agenteval/benchmark.go` +- Create: `internal/agenteval/benchmark_test.go` + +- [ ] **Step 1: Write failing tests** + +```go +func TestHarnessRunsTaskFromFixtureAndScoresResult(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + harness := Harness{ + Materializer: Materializer{}, + Agent: agentRunnerFunc(func(ctx context.Context, input AgentRunInput) AgentRunResult { + target := filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + err := os.WriteFile(target, []byte("updated"), 0o644) + if err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{}, + } + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + }) + if report.Summary.TotalTasks != 1 || report.Summary.PassedTasks != 1 || !report.OK { + t.Fatalf("report = %#v", report) + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/agenteval -run TestHarness -count=1` +Expected: FAIL because `Harness` is undefined. + +- [ ] **Step 3: Implement harness** + +Add: +- `BenchmarkInput{TaskID, WorkRoot string; KeepWorkspaces bool}` +- `BenchmarkReport{Contract, SuiteID, OK, Summary, Tasks}` +- `BenchmarkTaskReport{TaskID, WorkspacePath, FixturePath, Agent AgentRunResult, Report Report}` +- `BenchmarkSummary{TotalTasks, PassedTasks, FailedTasks, BlockedTasks, ErrorTasks int}` +- `Harness{Materializer Materializer; Agent AgentRunner; Runner Runner}` +- If no `Agent`, produce blocked task reports with message `agent command is required`. +- Select one task when `TaskID` set, otherwise all tasks. +- For each task: materialize, run agent, if agent fails mark blocked; otherwise score with `Runner.Run`. +- Remove each materialized task workspace after scoring unless `BenchmarkInput.KeepWorkspaces` is true. + +- [ ] **Step 4: Verify** + +Run: `go test ./internal/agenteval -run 'TestHarness|TestBenchmark' -count=1` + +## Task 4: CLI `zero eval bench` + +**Files:** +- Modify: `internal/cli/agent_eval.go` +- Modify: `internal/cli/agent_eval_test.go` + +- [ ] **Step 1: Write failing tests** + +```go +func TestRunEvalBenchJSONModePassesHarnessOptions(t *testing.T) { + var stdout, stderr bytes.Buffer + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", "evals/context.json", + "--task", "edit-reader", + "--work-root", "D:\\tmp\\zero-evals", + "--agent-command", "zero", "exec", "{prompt}", + "--json", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Mode != "bench" || options.TaskID != "edit-reader" || options.WorkRoot == "" || len(options.AgentCommand) != 3 { + t.Fatalf("unexpected bench options: %#v", options) + } + return agentEvalReport{Suite: "quality-context", Status: "pass", OK: true, Total: 1, Passed: 1}, nil + }, + }) + if exitCode != exitSuccess { + t.Fatalf("exit=%d stderr=%s", exitCode, stderr.String()) + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cli -run TestRunEvalBench -count=1` +Expected: FAIL because bench mode is not parsed. + +- [ ] **Step 3: Implement CLI** + +Add parser support: +- Modes: `validate`, `run`, `bench` +- Bench flags: `--work-root `, `--agent-command `, `--keep-workspaces` +- `--task`, `--report-dir`, `--json` work in bench mode. +- `--workspace` remains run-only. +- Default work root for bench: temp dir with prefix `zero-eval-`. +- Default `AgentCommand` empty -> harness report blocked rather than usage error. +- Convert benchmark aggregate to existing CLI report shape. + +- [ ] **Step 4: Verify** + +Run: +```bash +go test ./internal/cli -run 'TestRunEvalBench|TestRunEvalRun' -count=1 +``` + +## Task 5: Docs and Manual Examples + +**Files:** +- Modify: `docs/AGENT_EVALS.md` + +- [ ] **Step 1: Document modes** + +Add a `bench` subsection: +- `validate`: schema only +- `run`: score an already-mutated worktree +- `bench`: copy fixture, run agent command, score result + +- [ ] **Step 2: Include examples** + +Document: +```bash +go run ./cmd/zero eval bench \ + --suite internal/agenteval/testdata/sample_suite.json \ + --task document-stream-json-verify-events \ + --work-root /tmp/zero-evals \ + --agent-command zero exec --cwd {workspace} {prompt} +``` + +Also document a deterministic fake-agent example for local testing. + +- [ ] **Step 3: Verify** + +Run: +```bash +go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json +go test ./... +``` + +## Task 6: Integration Validation + +**Files:** +- Modify as needed only in files above. + +- [ ] **Step 1: Run focused validation** + +```bash +go test -count=1 ./internal/agenteval ./internal/cli +``` + +- [ ] **Step 2: Run fixture validation** + +```bash +go test ./... +``` + +from `internal/agenteval/testdata/fixtures/zero-mini`. + +- [ ] **Step 3: Run full repo validation** + +```bash +gofmt -l internal/agenteval internal/cli +git diff --check +go test ./... +go run ./cmd/zero-release build +go run ./cmd/zero-release smoke +``` diff --git a/docs/superpowers/plans/2026-06-12-agent-eval-d-level-upgrades.md b/docs/superpowers/plans/2026-06-12-agent-eval-d-level-upgrades.md new file mode 100644 index 000000000..af6399419 --- /dev/null +++ b/docs/superpowers/plans/2026-06-12-agent-eval-d-level-upgrades.md @@ -0,0 +1,181 @@ +# Agent Eval D-Level Upgrades Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Upgrade the local agent eval harness from a smoke benchmark into a quality signal that can compare models, score traces, catch context mistakes, and preserve regression artifacts. + +**Architecture:** Keep the existing `internal/agenteval` harness as the core. Add small schema fields to `Task`, score them through `Score` and `Harness.Run`, then expose model matrix and full benchmark report data through `zero eval bench`. Avoid provider calls or UI work. + +**Tech Stack:** Go 1.24, standard library JSON/process/git helpers, existing `go test ./...` validation. + +--- + +### Task 1: Suite Schema And Rubric Checks + +**Files:** +- Modify: `internal/agenteval/suite.go` +- Modify: `internal/agenteval/score.go` +- Test: `internal/agenteval/agenteval_test.go` + +- [ ] **Step 1: Write failing schema/rubric tests** + +Add tests that load and normalize `tags`, `difficulty`, and `forbiddenChangedFiles`, reject malformed forbidden paths, and fail scoring when a forbidden file is touched. + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/agenteval -run "LoadSuite|Validate|Forbidden" -count=1` + +- [ ] **Step 3: Implement minimal schema and scoring** + +Add these fields: + +```go +Tags []string `json:"tags,omitempty"` +Difficulty string `json:"difficulty,omitempty"` +ForbiddenChangedFiles []string `json:"forbiddenChangedFiles,omitempty"` +``` + +Normalize file lists, validate duplicate/malformed entries, and add a `forbidden_changed_files` result. + +- [ ] **Step 4: Verify green** + +Run: `go test ./internal/agenteval -count=1` + +### Task 2: Agent Trace Scoring + +**Files:** +- Create: `internal/agenteval/trace.go` +- Test: `internal/agenteval/trace_test.go` +- Modify: `internal/agenteval/benchmark.go` + +- [ ] **Step 1: Write failing trace tests** + +Cover JSONL events such as: + +```json +{"type":"tool","name":"read_file"} +{"event":"verify","name":"go-test"} +``` + +Require deterministic event keys like `tool:read_file` and `verify:go-test`. + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/agenteval -run Trace -count=1` + +- [ ] **Step 3: Implement parser and benchmark scoring** + +Parse agent stdout line by line, ignore non-JSON noise, and append a `trace` result when a task declares `requiredTraceEvents`. + +- [ ] **Step 4: Verify green** + +Run: `go test ./internal/agenteval -count=1` + +### Task 3: Context Quality Checks + +**Files:** +- Create: `internal/agenteval/context_checks.go` +- Test: `internal/agenteval/context_checks_test.go` +- Modify: `internal/agenteval/benchmark.go` + +- [ ] **Step 1: Write failing context tests** + +Cover a task that declares: + +```json +"contextChecks": { + "requiredFiles": ["docs/STREAM_JSON_PROTOCOL.md"], + "forbiddenFiles": ["node_modules/cache.txt"] +} +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/agenteval -run Context -count=1` + +- [ ] **Step 3: Implement fixture/workspace context checks** + +Check required files exist under the materialized workspace and forbidden files do not. Append a `context` result before summary finalization. + +- [ ] **Step 4: Verify green** + +Run: `go test ./internal/agenteval -count=1` + +### Task 4: Model Matrix Benchmarking + +**Files:** +- Modify: `internal/agenteval/agent_command.go` +- Modify: `internal/agenteval/benchmark.go` +- Test: `internal/agenteval/agent_command_test.go` +- Test: `internal/agenteval/benchmark_test.go` + +- [ ] **Step 1: Write failing model-matrix tests** + +Assert `{model}` expands in agent command argv and `BenchmarkInput{Models: []string{"a","b"}}` runs every task for each model. + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/agenteval -run "Model|Harness" -count=1` + +- [ ] **Step 3: Implement model propagation** + +Add `Model` to `AgentRunInput` and `BenchmarkTaskReport`; add `Models []string` to `BenchmarkInput`; loop task/model pairs deterministically. + +- [ ] **Step 4: Verify green** + +Run: `go test ./internal/agenteval -count=1` + +### Task 5: CLI Flags And Regression Artifacts + +**Files:** +- Modify: `internal/cli/agent_eval.go` +- Test: `internal/cli/agent_eval_test.go` + +- [ ] **Step 1: Write failing CLI tests** + +Cover repeated `--model`, comma-separated `--models`, help text, and report-dir JSON containing nested benchmark detail. + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/cli -run Eval -count=1` + +- [ ] **Step 3: Implement CLI wiring** + +Pass models into `agenteval.BenchmarkInput`, include benchmark detail in `agentEvalReport`, and prefix failure IDs with model when present. + +- [ ] **Step 4: Verify green** + +Run: `go test ./internal/cli -count=1` + +### Task 6: Suite Expansion And Docs + +**Files:** +- Modify: `internal/agenteval/testdata/sample_suite.json` +- Modify: `docs/AGENT_EVALS.md` + +- [ ] **Step 1: Write failing suite expectation** + +Update the sample suite test to require richer coverage count and metadata fields. + +- [ ] **Step 2: Run tests to verify failure** + +Run: `go test ./internal/agenteval -run SampleSuite -count=1` + +- [ ] **Step 3: Expand the fixture suite and docs** + +Add more tasks using the existing `zero-mini` fixture, plus examples for `--model`, `{model}`, `requiredTraceEvents`, `contextChecks`, and report artifacts. + +- [ ] **Step 4: Verify green** + +Run: `go test ./internal/agenteval ./internal/cli -count=1` + +### Final Verification + +- [ ] Run `gofmt -l internal/agenteval internal/cli` +- [ ] Run `git diff --check` +- [ ] Run `go vet ./...` +- [ ] Run `go test ./...` +- [ ] Run `go run ./cmd/zero eval --suite internal/agenteval/testdata/sample_suite.json` +- [ ] Run `go run ./cmd/zero eval bench --suite internal/agenteval/testdata/sample_suite.json --task document-stream-json-verify-events --model test-model --json --agent-command powershell -NoProfile -Command "& { param(`$ws) Set-Content -LiteralPath (Join-Path `$ws 'docs/STREAM_JSON_PROTOCOL.md') -Value updated; Write-Output '{\"type\":\"tool\",\"name\":\"read_file\"}' }" "{workspace}"` +- [ ] Run `go run ./cmd/zero-release build` +- [ ] Commit and push to `feat/agent-eval-harness` diff --git a/internal/agenteval/agent_command.go b/internal/agenteval/agent_command.go new file mode 100644 index 000000000..7eae6a490 --- /dev/null +++ b/internal/agenteval/agent_command.go @@ -0,0 +1,135 @@ +package agenteval + +import ( + "bytes" + "context" + "errors" + "os/exec" + "strings" +) + +type AgentRunInput struct { + TaskID string + Prompt string + WorkspacePath string + Model string +} + +type AgentRunResult struct { + ExitCode int `json:"exitCode"` + Stdout string `json:"stdout,omitempty"` + Stderr string `json:"stderr,omitempty"` + Error string `json:"error,omitempty"` + // Truncated is set when captured stdout/stderr exceeded the runner's + // OutputLimit and some output was dropped. + Truncated bool `json:"truncated,omitempty"` +} + +type AgentRunner interface { + Run(context.Context, AgentRunInput) AgentRunResult +} + +type AgentRunnerFunc func(context.Context, AgentRunInput) AgentRunResult + +func (fn AgentRunnerFunc) Run(ctx context.Context, input AgentRunInput) AgentRunResult { + return fn(ctx, input) +} + +// defaultAgentOutputLimit caps captured stdout/stderr per stream so a chatty or +// runaway agent cannot exhaust memory or bloat the benchmark report. +const defaultAgentOutputLimit = 1 << 20 // 1 MiB per stream + +type CommandAgentRunner struct { + Command []string + // OutputLimit caps captured stdout/stderr per stream in bytes. Zero applies + // defaultAgentOutputLimit; a negative value disables the cap. + OutputLimit int +} + +func (runner CommandAgentRunner) Run(ctx context.Context, input AgentRunInput) AgentRunResult { + result := AgentRunResult{ExitCode: -1} + if len(runner.Command) == 0 || strings.TrimSpace(runner.Command[0]) == "" { + result.Error = "agent command is required" + return result + } + if ctx == nil { + ctx = context.Background() + } + limit := runner.OutputLimit + if limit == 0 { + limit = defaultAgentOutputLimit + } + command := expandAgentCommand(runner.Command, input) + cmd := exec.CommandContext(ctx, command[0], command[1:]...) + cmd.Dir = input.WorkspacePath + stdout := &capWriter{limit: limit} + stderr := &capWriter{limit: limit} + cmd.Stdout = stdout + cmd.Stderr = stderr + + err := cmd.Run() + result.Stdout = stdout.buf.String() + result.Stderr = stderr.buf.String() + result.Truncated = stdout.truncated || stderr.truncated + if err == nil { + result.ExitCode = 0 + return result + } + // A canceled or timed-out context kills the process, surfacing as a signal + // exit; report the context error explicitly instead of "exited with code -1". + if ctxErr := ctx.Err(); ctxErr != nil { + result.Error = ctxErr.Error() + return result + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + result.ExitCode = exitErr.ExitCode() + return result + } + result.Error = err.Error() + return result +} + +func expandAgentCommand(command []string, input AgentRunInput) []string { + // strings.NewReplacer performs a single left-to-right pass and never + // re-scans replaced text, so a placeholder value that itself contains + // "{workspace}"/"{task_id}"/"{model}" is not re-expanded. + replacer := strings.NewReplacer( + "{prompt}", input.Prompt, + "{workspace}", input.WorkspacePath, + "{task_id}", input.TaskID, + "{model}", input.Model, + ) + expanded := make([]string, len(command)) + for i, arg := range command { + expanded[i] = replacer.Replace(arg) + } + return expanded +} + +// capWriter buffers writes up to limit bytes and records whether any data was +// dropped. A non-positive limit means unbounded. +type capWriter struct { + buf bytes.Buffer + limit int + truncated bool +} + +func (w *capWriter) Write(p []byte) (int, error) { + if w.limit <= 0 { + return w.buf.Write(p) + } + remaining := w.limit - w.buf.Len() + if remaining <= 0 { + w.truncated = true + return len(p), nil + } + if len(p) > remaining { + if _, err := w.buf.Write(p[:remaining]); err != nil { + return 0, err + } + w.truncated = true + return len(p), nil + } + return w.buf.Write(p) +} diff --git a/internal/agenteval/agent_command_test.go b/internal/agenteval/agent_command_test.go new file mode 100644 index 000000000..714ff1736 --- /dev/null +++ b/internal/agenteval/agent_command_test.go @@ -0,0 +1,240 @@ +package agenteval + +import ( + "context" + "os" + "path/filepath" + "reflect" + "strconv" + "strings" + "testing" + "time" +) + +func TestAgentCommandRunnerExpandsPlaceholdersAndUsesWorkspaceDir(t *testing.T) { + workspace := t.TempDir() + runner := CommandAgentRunner{Command: helperCommand( + "record", + filepath.Join(workspace, "args.txt"), + "{task_id}", + "{workspace}", + "{prompt}", + "{model}", + )} + + result := runner.Run(context.Background(), AgentRunInput{ + TaskID: "task-a", + WorkspacePath: workspace, + Prompt: "fix bug", + Model: "gpt-5", + }) + + if result.ExitCode != 0 || result.Error != "" { + t.Fatalf("Run = %#v", result) + } + data, err := os.ReadFile(filepath.Join(workspace, "args.txt")) + if err != nil { + t.Fatal(err) + } + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + want := []string{"cwd=" + workspace, "task-a", workspace, "fix bug", "gpt-5"} + if !reflect.DeepEqual(lines, want) { + t.Fatalf("recorded args = %#v, want %#v", lines, want) + } +} + +func TestCommandAgentRunnerCapturesStdoutStderrAndExitCode(t *testing.T) { + workspace := t.TempDir() + runner := CommandAgentRunner{Command: helperCommand("exit", "7")} + + result := runner.Run(context.Background(), AgentRunInput{WorkspacePath: workspace}) + + if result.ExitCode != 7 { + t.Fatalf("ExitCode = %d, want 7; result=%#v", result.ExitCode, result) + } + if result.Error != "" { + t.Fatalf("Error = %q, want empty for non-zero exit", result.Error) + } + if result.Stdout != "agent stdout\n" { + t.Fatalf("Stdout = %q", result.Stdout) + } + if result.Stderr != "agent stderr\n" { + t.Fatalf("Stderr = %q", result.Stderr) + } +} + +func TestCommandAgentRunnerTruncatesOversizedOutput(t *testing.T) { + runner := CommandAgentRunner{ + Command: helperCommand("spew", "5000"), + OutputLimit: 1000, + } + + result := runner.Run(context.Background(), AgentRunInput{WorkspacePath: t.TempDir()}) + + if result.ExitCode != 0 { + t.Fatalf("ExitCode = %d, want 0; result=%#v", result.ExitCode, result) + } + if !result.Truncated { + t.Fatalf("expected Truncated=true; stdout len=%d", len(result.Stdout)) + } + if len(result.Stdout) != 1000 { + t.Fatalf("captured stdout len = %d, want 1000", len(result.Stdout)) + } +} + +func TestCommandAgentRunnerTimesOutHangingAgent(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + runner := CommandAgentRunner{Command: helperCommand("sleep")} + + start := time.Now() + result := runner.Run(ctx, AgentRunInput{WorkspacePath: t.TempDir()}) + + if elapsed := time.Since(start); elapsed > 10*time.Second { + t.Fatalf("runner did not honor timeout; elapsed=%s", elapsed) + } + if result.ExitCode != -1 { + t.Fatalf("ExitCode = %d, want -1; result=%#v", result.ExitCode, result) + } + if result.Error == "" { + t.Fatalf("expected a timeout error; result=%#v", result) + } +} + +func TestExpandAgentCommandDoesNotReexpandInjectedPlaceholders(t *testing.T) { + got := expandAgentCommand( + []string{"agent", "{prompt}", "{task_id}"}, + AgentRunInput{Prompt: "edit {workspace} now", WorkspacePath: "/ws", TaskID: "t1"}, + ) + + want := []string{"agent", "edit {workspace} now", "t1"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("expandAgentCommand = %#v, want %#v", got, want) + } +} + +func TestCommandAgentRunnerEmptyCommandReturnsError(t *testing.T) { + tests := []struct { + name string + command []string + }{ + {name: "nil", command: nil}, + {name: "empty executable", command: []string{""}}, + {name: "blank executable", command: []string{" "}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := (CommandAgentRunner{Command: tt.command}).Run(context.Background(), AgentRunInput{}) + + if result.ExitCode != -1 { + t.Fatalf("ExitCode = %d, want -1", result.ExitCode) + } + if result.Error != "agent command is required" { + t.Fatalf("Error = %q", result.Error) + } + }) + } +} + +func TestCommandAgentRunnerReportsContextErrors(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + result := (CommandAgentRunner{Command: helperCommand("record", filepath.Join(t.TempDir(), "out.txt"))}). + Run(ctx, AgentRunInput{WorkspacePath: t.TempDir()}) + + if result.ExitCode != -1 { + t.Fatalf("ExitCode = %d, want -1", result.ExitCode) + } + if result.Error == "" { + t.Fatalf("Error is empty; result=%#v", result) + } +} + +func TestCommandAgentRunnerReportsSpawnErrors(t *testing.T) { + result := (CommandAgentRunner{Command: []string{filepath.Join(t.TempDir(), "missing-agent")}}). + Run(context.Background(), AgentRunInput{WorkspacePath: t.TempDir()}) + + if result.ExitCode != -1 { + t.Fatalf("ExitCode = %d, want -1", result.ExitCode) + } + if result.Error == "" { + t.Fatalf("Error is empty; result=%#v", result) + } +} + +func TestAgentRunnerFuncRunCallsFunction(t *testing.T) { + called := false + runner := AgentRunnerFunc(func(_ context.Context, input AgentRunInput) AgentRunResult { + called = true + if input.TaskID != "task-a" { + t.Fatalf("TaskID = %q", input.TaskID) + } + return AgentRunResult{ExitCode: 3, Stdout: "ok"} + }) + + result := runner.Run(context.Background(), AgentRunInput{TaskID: "task-a"}) + + if !called { + t.Fatal("function was not called") + } + if result.ExitCode != 3 || result.Stdout != "ok" { + t.Fatalf("Run = %#v", result) + } +} + +func TestCommandAgentRunnerHelperProcess(t *testing.T) { + args := os.Args + for len(args) > 0 && args[0] != "--" { + args = args[1:] + } + if len(args) == 0 { + return + } + if len(args) < 2 { + os.Exit(2) + } + switch args[1] { + case "record": + if len(args) < 3 { + os.Exit(2) + } + cwd, err := os.Getwd() + if err != nil { + os.Exit(2) + } + lines := append([]string{"cwd=" + cwd}, args[3:]...) + if err := os.WriteFile(args[2], []byte(strings.Join(lines, "\n")), 0o600); err != nil { + os.Exit(2) + } + case "exit": + os.Stdout.WriteString("agent stdout\n") + os.Stderr.WriteString("agent stderr\n") + os.Exit(7) + case "spew": + if len(args) < 3 { + os.Exit(2) + } + count, err := strconv.Atoi(args[2]) + if err != nil { + os.Exit(2) + } + os.Stdout.Write([]byte(strings.Repeat("a", count))) + case "sleep": + time.Sleep(30 * time.Second) + default: + os.Exit(2) + } + os.Exit(0) +} + +func helperCommand(args ...string) []string { + command := []string{ + os.Args[0], + "-test.run=TestCommandAgentRunnerHelperProcess", + "--", + } + command = append(command, args...) + return command +} diff --git a/internal/agenteval/agenteval_test.go b/internal/agenteval/agenteval_test.go index d6e267dfc..acdbc56af 100644 --- a/internal/agenteval/agenteval_test.go +++ b/internal/agenteval/agenteval_test.go @@ -18,12 +18,20 @@ func TestLoadSuiteParsesJSONAndNormalizesDeterministicFields(t *testing.T) { "id": "edit-reader", "name": "Edit reader", "description": "Keep prompt context tight.", + "tags": ["repo-map", "tool-use"], + "difficulty": "medium", "prompt": "Update the reader.", "workspaceFixture": "fixtures/reader", + "requiredTraceEvents": ["tool:read_file", "tool:apply_patch"], + "contextChecks": { + "requiredFiles": ["internal/reader/a/../b.go"], + "forbiddenFiles": ["node_modules/cache.txt"] + }, "verificationCommands": [ {"id": "test", "name": "Tests", "command": ["go", "test", "./..."]} ], - "expectedChangedFiles": ["internal/reader/a/../b.go", "internal/reader/a.go"] + "expectedChangedFiles": ["internal/reader/a/../b.go", "internal/reader/a.go"], + "forbiddenChangedFiles": ["docs/generated.log"] }] }`) @@ -42,6 +50,25 @@ func TestLoadSuiteParsesJSONAndNormalizesDeterministicFields(t *testing.T) { if got := strings.Join(suite.Tasks[0].VerificationCommands[0].Command, " "); got != "go test ./..." { t.Fatalf("command = %q, want go test ./...", got) } + task := suite.Tasks[0] + if !reflect.DeepEqual(task.Tags, []string{"repo-map", "tool-use"}) { + t.Fatalf("tags = %#v", task.Tags) + } + if task.Difficulty != "medium" { + t.Fatalf("difficulty = %q", task.Difficulty) + } + if !reflect.DeepEqual(task.RequiredTraceEvents, []string{"tool:apply_patch", "tool:read_file"}) { + t.Fatalf("required trace events = %#v", task.RequiredTraceEvents) + } + if !reflect.DeepEqual(task.ContextChecks.RequiredFiles, []string{"internal/reader/b.go"}) { + t.Fatalf("context required files = %#v", task.ContextChecks.RequiredFiles) + } + if !reflect.DeepEqual(task.ContextChecks.ForbiddenFiles, []string{"node_modules/cache.txt"}) { + t.Fatalf("context forbidden files = %#v", task.ContextChecks.ForbiddenFiles) + } + if !reflect.DeepEqual(task.ForbiddenChangedFiles, []string{"docs/generated.log"}) { + t.Fatalf("forbidden changed files = %#v", task.ForbiddenChangedFiles) + } } func TestLoadSuiteRejectsTrailingJSON(t *testing.T) { @@ -90,16 +117,35 @@ func TestSampleSuiteLoads(t *testing.T) { if err != nil { t.Fatalf("sample suite should load: %v", err) } - if len(suite.Tasks) < 5 || len(suite.Tasks) > 8 { - t.Fatalf("sample suite tasks = %d, want 5-8", len(suite.Tasks)) + if len(suite.Tasks) < 8 || len(suite.Tasks) > 12 { + t.Fatalf("sample suite tasks = %d, want 8-12", len(suite.Tasks)) } + traceTasks := 0 for _, task := range suite.Tasks { + if len(task.Tags) == 0 { + t.Fatalf("sample task %q has no tags", task.ID) + } + if task.Difficulty == "" { + t.Fatalf("sample task %q has no difficulty", task.ID) + } if len(task.ExpectedChangedFiles) == 0 { t.Fatalf("sample task %q has no expected changed files", task.ID) } + if len(task.ForbiddenChangedFiles) == 0 { + t.Fatalf("sample task %q has no forbidden changed files", task.ID) + } + if len(task.ContextChecks.RequiredFiles) == 0 { + t.Fatalf("sample task %q has no required context files", task.ID) + } if len(task.VerificationCommands) == 0 { t.Fatalf("sample task %q has no verification commands", task.ID) } + if len(task.RequiredTraceEvents) > 0 { + traceTasks++ + } + } + if traceTasks < 4 { + t.Fatalf("sample suite trace tasks = %d, want at least 4", traceTasks) } } @@ -168,6 +214,45 @@ func TestValidateRejectsDuplicateNormalizedExpectedChangedFiles(t *testing.T) { } } +func TestValidateRejectsMalformedQualityFileLists(t *testing.T) { + err := Suite{ + ID: "suite", + Name: "Suite", + Tasks: []Task{{ + ID: "task", + Name: "Task", + Prompt: "Do it", + WorkspaceFixture: "fixtures/task", + ExpectedChangedFiles: []string{"internal/reader/a.go"}, + ForbiddenChangedFiles: []string{"../outside.go", "logs/../logs/run.txt", "logs/run.txt"}, + ContextChecks: ContextChecks{ + RequiredFiles: []string{"/tmp/outside.go"}, + ForbiddenFiles: []string{"docs/ok.md", "docs/./ok.md"}, + }, + VerificationCommands: []Command{{ + ID: "test", + Name: "Tests", + Command: []string{"go", "test", "./..."}, + }}, + }}, + }.Validate() + + if err == nil { + t.Fatal("Validate returned nil, want malformed quality file list errors") + } + message := err.Error() + for _, want := range []string{ + "forbiddenChangedFiles[0] must be a relative workspace path", + "forbiddenChangedFiles[2] duplicates forbiddenChangedFiles[1]", + "contextChecks.requiredFiles[0] must be a relative workspace path", + "contextChecks.forbiddenFiles[1] duplicates contextChecks.forbiddenFiles[0]", + } { + if !strings.Contains(message, want) { + t.Fatalf("expected validation error %q in:\n%s", want, message) + } + } +} + func TestValidateReportsUsefulErrors(t *testing.T) { err := Suite{ ID: "suite", @@ -272,6 +357,30 @@ func TestScoreFailsForCommandFailureAndChangedFileMismatch(t *testing.T) { } } +func TestScoreFailsWhenForbiddenFilesChange(t *testing.T) { + suite := sampleSuite() + suite.Tasks[0].ForbiddenChangedFiles = []string{"internal/reader/private.go", "docs/generated.log"} + + report := Score(suite, ScoreInput{ + TaskID: "edit-reader", + CommandResults: []CommandResult{ + {ID: "test", ExitCode: 0}, + }, + ChangedFiles: []string{"internal/reader/a.go", "internal/reader/b.go", "internal/reader/private.go"}, + }) + + if report.OK || report.Status != StatusFail { + t.Fatalf("expected forbidden-file failure, got %#v", report) + } + result := findResultByID(t, report.Results, "forbidden_changed_files") + if result.Status != StatusFail { + t.Fatalf("forbidden result = %#v", result) + } + if !reflect.DeepEqual(result.UnexpectedFiles, []string{"internal/reader/private.go"}) { + t.Fatalf("forbidden touched files = %#v", result.UnexpectedFiles) + } +} + func TestScoreErrorsWhenExpectedCommandResultIsMissing(t *testing.T) { report := Score(sampleSuite(), ScoreInput{ TaskID: "edit-reader", @@ -447,6 +556,17 @@ func TestReportJSONIsStable(t *testing.T) { } } +func findResultByID(t *testing.T, results []Result, id string) Result { + t.Helper() + for _, result := range results { + if result.ID == id { + return result + } + } + t.Fatalf("missing result %q in %#v", id, results) + return Result{} +} + func writeSuite(t *testing.T, content string) string { t.Helper() path := filepath.Join(t.TempDir(), "suite.json") diff --git a/internal/agenteval/benchmark.go b/internal/agenteval/benchmark.go new file mode 100644 index 000000000..0bf3e0a7b --- /dev/null +++ b/internal/agenteval/benchmark.go @@ -0,0 +1,219 @@ +package agenteval + +import ( + "context" + "fmt" + "os" + "strings" + "time" +) + +type BenchmarkInput struct { + TaskID string + WorkRoot string + Models []string + KeepWorkspaces bool + // Timeout bounds each task's materialization, agent run, and scoring. A + // non-positive value leaves the task unbounded. + Timeout time.Duration +} + +type BenchmarkReport struct { + Contract string `json:"contract"` + SuiteID string `json:"suiteId"` + OK bool `json:"ok"` + Summary BenchmarkSummary `json:"summary"` + Tasks []BenchmarkTaskReport `json:"tasks"` +} + +type BenchmarkTaskReport struct { + TaskID string `json:"taskId"` + Model string `json:"model,omitempty"` + WorkspacePath string `json:"workspacePath"` + FixturePath string `json:"fixturePath"` + Agent AgentRunResult `json:"agent"` + Report Report `json:"report"` +} + +type BenchmarkSummary struct { + TotalTasks int `json:"totalTasks"` + PassedTasks int `json:"passedTasks"` + FailedTasks int `json:"failedTasks"` + BlockedTasks int `json:"blockedTasks"` + ErrorTasks int `json:"errorTasks"` +} + +type Harness struct { + Materializer Materializer + Agent AgentRunner + Runner Runner +} + +func (harness Harness) Run(ctx context.Context, suitePath string, suite Suite, input BenchmarkInput) BenchmarkReport { + if ctx == nil { + ctx = context.Background() + } + report := BenchmarkReport{ + Contract: ReportContractVersion, + SuiteID: suite.ID, + } + tasks, err := selectBenchmarkTasks(suite, input.TaskID) + if err != nil { + taskID := input.TaskID + report.Tasks = append(report.Tasks, BenchmarkTaskReport{ + TaskID: taskID, + Report: Report{ + Contract: ReportContractVersion, + SuiteID: suite.ID, + TaskID: taskID, + Status: StatusError, + OK: false, + Summary: Summary{Total: 1, Errors: 1}, + Error: err.Error(), + Results: []Result{{ + ID: "task", + Name: "Task selection", + Kind: ResultChangedFiles, + Status: StatusError, + Message: err.Error(), + }}, + }, + }) + report.finishSummary() + return report + } + + for _, task := range tasks { + for _, model := range benchmarkModels(input.Models) { + report.Tasks = append(report.Tasks, harness.runTask(ctx, suitePath, suite, task, model, input)) + } + } + report.finishSummary() + return report +} + +func (harness Harness) runTask(ctx context.Context, suitePath string, suite Suite, task Task, model string, input BenchmarkInput) BenchmarkTaskReport { + if input.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, input.Timeout) + defer cancel() + } + taskReport := BenchmarkTaskReport{TaskID: task.ID, Model: model} + if harness.Agent == nil { + taskReport.Agent = AgentRunResult{ExitCode: -1, Error: "agent command is required"} + taskReport.Report = Score(suite, ScoreInput{ + TaskID: task.ID, + Blocked: true, + BlockReason: taskReport.Agent.Error, + }) + return taskReport + } + + workspace, err := harness.Materializer.MaterializeTask(ctx, suitePath, task, MaterializeInput{ + WorkRoot: input.WorkRoot, + }) + if err != nil { + taskReport.Report = errorReport(suite.ID, task.ID, fmt.Sprintf("workspace materialization failed: %v", err)) + return taskReport + } + taskReport.WorkspacePath = workspace.Path + taskReport.FixturePath = workspace.FixturePath + if !input.KeepWorkspaces { + defer func() { _ = os.RemoveAll(workspace.Path) }() + } + + agentResult := harness.Agent.Run(ctx, AgentRunInput{ + TaskID: task.ID, + Model: model, + Prompt: task.Prompt, + WorkspacePath: workspace.Path, + }) + taskReport.Agent = agentResult + if agentResult.Error != "" || agentResult.ExitCode != 0 { + reason := firstNonEmpty(agentResult.Error, strings.TrimSpace(agentResult.Stderr), fmt.Sprintf("agent exited with code %d", agentResult.ExitCode)) + taskReport.Report = Score(suite, ScoreInput{ + TaskID: task.ID, + Blocked: true, + BlockReason: reason, + }) + return taskReport + } + + taskReport.Report = harness.Runner.Run(ctx, suite, RunInput{ + TaskID: task.ID, + WorkspacePath: workspace.Path, + TraceStdout: agentResult.Stdout, + }) + return taskReport +} + +func (report *BenchmarkReport) finishSummary() { + report.Summary = BenchmarkSummary{TotalTasks: len(report.Tasks)} + for _, task := range report.Tasks { + switch { + case task.Report.OK: + report.Summary.PassedTasks++ + case task.Report.Status == StatusBlocked: + report.Summary.BlockedTasks++ + case task.Report.Status == StatusError: + report.Summary.ErrorTasks++ + default: + report.Summary.FailedTasks++ + } + } + report.OK = report.Summary.TotalTasks > 0 && + report.Summary.FailedTasks == 0 && + report.Summary.BlockedTasks == 0 && + report.Summary.ErrorTasks == 0 +} + +func selectBenchmarkTasks(suite Suite, taskID string) ([]Task, error) { + if taskID == "" { + tasks := make([]Task, 0, len(suite.Tasks)) + for _, task := range suite.Tasks { + tasks = append(tasks, normalizeTask(task)) + } + return tasks, nil + } + task, err := selectTask(suite, taskID) + if err != nil { + return nil, err + } + return []Task{task}, nil +} + +func benchmarkModels(models []string) []string { + normalized := make([]string, 0, len(models)) + seen := map[string]bool{} + for _, model := range models { + model = strings.TrimSpace(model) + if model == "" || seen[model] { + continue + } + seen[model] = true + normalized = append(normalized, model) + } + if len(normalized) == 0 { + return []string{""} + } + return normalized +} + +func errorReport(suiteID string, taskID string, message string) Report { + return Report{ + Contract: ReportContractVersion, + SuiteID: suiteID, + TaskID: taskID, + Status: StatusError, + OK: false, + Summary: Summary{Total: 1, Errors: 1}, + Error: message, + Results: []Result{{ + ID: "benchmark", + Name: "Benchmark harness", + Kind: ResultChangedFiles, + Status: StatusError, + Message: message, + }}, + } +} diff --git a/internal/agenteval/benchmark_test.go b/internal/agenteval/benchmark_test.go new file mode 100644 index 000000000..39032a412 --- /dev/null +++ b/internal/agenteval/benchmark_test.go @@ -0,0 +1,513 @@ +package agenteval + +import ( + "context" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + "time" +) + +func TestHarnessRunsSelectedTaskFromFixtureAndScoresResult(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(_ context.Context, input AgentRunInput) AgentRunResult { + if input.TaskID != "document-stream-json-verify-events" { + t.Fatalf("agent TaskID = %q", input.TaskID) + } + if !strings.Contains(input.Prompt, "stream-json protocol docs") { + t.Fatalf("agent Prompt = %q", input.Prompt) + } + target := filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + if err := os.WriteFile(target, []byte("updated"), 0o644); err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + }, + } + + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + }) + + if !report.OK { + t.Fatalf("OK = false; report=%#v", report) + } + if report.Contract != ReportContractVersion { + t.Fatalf("Contract = %q", report.Contract) + } + if report.SuiteID != suite.ID { + t.Fatalf("SuiteID = %q", report.SuiteID) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 1, PassedTasks: 1}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if len(report.Tasks) != 1 { + t.Fatalf("Tasks len = %d", len(report.Tasks)) + } + taskReport := report.Tasks[0] + if taskReport.TaskID != "document-stream-json-verify-events" { + t.Fatalf("TaskID = %q", taskReport.TaskID) + } + if taskReport.WorkspacePath == "" || taskReport.FixturePath == "" { + t.Fatalf("workspace fields were not populated: %#v", taskReport) + } + if taskReport.Agent.ExitCode != 0 || taskReport.Agent.Error != "" { + t.Fatalf("Agent = %#v", taskReport.Agent) + } + if taskReport.Report.Status != StatusPass || !taskReport.Report.OK { + t.Fatalf("Report = %#v", taskReport.Report) + } +} + +func TestHarnessRunsAllTasksWhenTaskIDIsEmpty(t *testing.T) { + suite := Suite{ + ID: "suite-a", + Name: "Suite A", + Tasks: []Task{ + { + ID: "task-a", + Name: "Task A", + Prompt: "change a", + WorkspaceFixture: "fixtures/zero-mini", + ExpectedChangedFiles: []string{"docs/STREAM_JSON_PROTOCOL.md"}, + VerificationCommands: []Command{{ID: "verify-a", Name: "Verify A", Command: []string{"go", "test", "./..."}}}, + }, + { + ID: "task-b", + Name: "Task B", + Prompt: "change b", + WorkspaceFixture: "fixtures/zero-mini", + ExpectedChangedFiles: []string{"docs/NPM_WRAPPER_SMOKE.md"}, + VerificationCommands: []Command{{ID: "verify-b", Name: "Verify B", Command: []string{"go", "test", "./..."}}}, + }, + }, + } + calls := []string{} + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(_ context.Context, input AgentRunInput) AgentRunResult { + calls = append(calls, input.TaskID) + var target string + switch input.TaskID { + case "task-a": + target = filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + case "task-b": + target = filepath.Join(input.WorkspacePath, "docs", "NPM_WRAPPER_SMOKE.md") + default: + t.Fatalf("unexpected task %q", input.TaskID) + } + if err := os.WriteFile(target, []byte(input.TaskID), 0o644); err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + }, + } + + report := harness.Run(context.Background(), filepath.Join("testdata", "sample_suite.json"), suite, BenchmarkInput{ + WorkRoot: t.TempDir(), + }) + + if !report.OK { + t.Fatalf("OK = false; report=%#v", report) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 2, PassedTasks: 2}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if strings.Join(calls, ",") != "task-a,task-b" { + t.Fatalf("agent calls = %#v", calls) + } +} + +func TestHarnessRunsEachSelectedTaskForEachModel(t *testing.T) { + suite := Suite{ + ID: "suite-a", + Name: "Suite A", + Tasks: []Task{{ + ID: "task-a", + Name: "Task A", + Prompt: "change a", + WorkspaceFixture: "fixtures/zero-mini", + ExpectedChangedFiles: []string{"docs/STREAM_JSON_PROTOCOL.md"}, + VerificationCommands: []Command{{ID: "verify-a", Name: "Verify A", Command: []string{"go", "test", "./..."}}}, + }}, + } + calls := []string{} + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(_ context.Context, input AgentRunInput) AgentRunResult { + calls = append(calls, input.Model) + if input.Model == "" { + t.Fatal("agent model was empty") + } + target := filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + if err := os.WriteFile(target, []byte(input.Model), 0o644); err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + }, + } + + report := harness.Run(context.Background(), filepath.Join("testdata", "sample_suite.json"), suite, BenchmarkInput{ + WorkRoot: t.TempDir(), + Models: []string{"model-a", "model-b"}, + }) + + if !report.OK { + t.Fatalf("OK = false; report=%#v", report) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 2, PassedTasks: 2}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if strings.Join(calls, ",") != "model-a,model-b" { + t.Fatalf("agent model calls = %#v", calls) + } + if report.Tasks[0].Model != "model-a" || report.Tasks[1].Model != "model-b" { + t.Fatalf("task report models = %#v, %#v", report.Tasks[0].Model, report.Tasks[1].Model) + } +} + +func TestHarnessScoresTraceAndContextChecks(t *testing.T) { + suite := Suite{ + ID: "suite-a", + Name: "Suite A", + Tasks: []Task{{ + ID: "task-a", + Name: "Task A", + Prompt: "change a", + WorkspaceFixture: "fixtures/zero-mini", + ExpectedChangedFiles: []string{"docs/STREAM_JSON_PROTOCOL.md"}, + RequiredTraceEvents: []string{"tool:apply_patch", "tool:read_file"}, + ContextChecks: ContextChecks{ + RequiredFiles: []string{"docs/STREAM_JSON_PROTOCOL.md"}, + ForbiddenFiles: []string{"tmp/leak.txt"}, + }, + VerificationCommands: []Command{{ID: "verify-a", Name: "Verify A", Command: []string{"go", "test", "./..."}}}, + }}, + } + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(_ context.Context, input AgentRunInput) AgentRunResult { + target := filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + if err := os.WriteFile(target, []byte("updated"), 0o644); err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0, Stdout: "{\"type\":\"tool\",\"name\":\"read_file\"}\n"} + }), + Runner: Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + }, + } + + report := harness.Run(context.Background(), filepath.Join("testdata", "sample_suite.json"), suite, BenchmarkInput{ + WorkRoot: t.TempDir(), + }) + + if report.OK || report.Summary.FailedTasks != 1 { + t.Fatalf("expected trace failure, got %#v", report) + } + trace := findResultByID(t, report.Tasks[0].Report.Results, "trace_events") + if trace.Status != StatusFail || !reflect.DeepEqual(trace.MissingEvents, []string{"tool:apply_patch"}) { + t.Fatalf("trace result = %#v", trace) + } + context := findResultByID(t, report.Tasks[0].Report.Results, "context_checks") + if context.Status != StatusPass { + t.Fatalf("context result = %#v", context) + } +} + +func TestHarnessBlocksSelectedTasksWhenAgentIsNil(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + harness := Harness{Materializer: Materializer{}, Runner: Runner{}} + + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + }) + + if report.OK { + t.Fatalf("OK = true; report=%#v", report) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 1, BlockedTasks: 1}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if report.Tasks[0].Agent.Error != "agent command is required" { + t.Fatalf("Agent.Error = %q", report.Tasks[0].Agent.Error) + } + if report.Tasks[0].Report.Status != StatusBlocked { + t.Fatalf("Report.Status = %q", report.Tasks[0].Report.Status) + } +} + +func TestHarnessBlocksWhenAgentRunFails(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(context.Context, AgentRunInput) AgentRunResult { + return AgentRunResult{ExitCode: 2, Stderr: "nope"} + }), + Runner: Runner{ + RunCommand: func(context.Context, string, Command) CommandResult { + t.Fatal("runner should not score after a failed agent run") + return CommandResult{} + }, + }, + } + + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + }) + + if report.OK { + t.Fatalf("OK = true; report=%#v", report) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 1, BlockedTasks: 1}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if report.Tasks[0].Report.Status != StatusBlocked { + t.Fatalf("Report.Status = %q", report.Tasks[0].Report.Status) + } +} + +func TestHarnessReportsErrorForUnknownTaskID(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(context.Context, AgentRunInput) AgentRunResult { + t.Fatal("agent should not run for an unknown task id") + return AgentRunResult{} + }), + Runner: Runner{}, + } + + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "no-such-task", + WorkRoot: t.TempDir(), + }) + + if report.OK { + t.Fatalf("OK = true; report=%#v", report) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 1, ErrorTasks: 1}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if len(report.Tasks) != 1 || report.Tasks[0].TaskID != "no-such-task" { + t.Fatalf("Tasks = %#v", report.Tasks) + } + if report.Tasks[0].Report.Status != StatusError { + t.Fatalf("Report.Status = %q", report.Tasks[0].Report.Status) + } +} + +func TestHarnessReportsErrorWhenMaterializationFails(t *testing.T) { + suite := Suite{ + ID: "suite-mat", + Name: "Suite Mat", + Tasks: []Task{{ + ID: "missing-fixture", + Name: "Missing fixture", + Prompt: "do work", + WorkspaceFixture: "fixtures/does-not-exist", + ExpectedChangedFiles: []string{"x.txt"}, + VerificationCommands: []Command{{ID: "v", Name: "V", Command: []string{"go", "version"}}}, + }}, + } + agentCalled := false + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(context.Context, AgentRunInput) AgentRunResult { + agentCalled = true + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{RunCommand: func(context.Context, string, Command) CommandResult { + t.Fatal("runner should not score when materialization fails") + return CommandResult{} + }}, + } + + report := harness.Run(context.Background(), filepath.Join("testdata", "sample_suite.json"), suite, BenchmarkInput{ + WorkRoot: t.TempDir(), + }) + + if report.OK { + t.Fatalf("OK = true; report=%#v", report) + } + if report.Summary != (BenchmarkSummary{TotalTasks: 1, ErrorTasks: 1}) { + t.Fatalf("Summary = %#v", report.Summary) + } + if report.Tasks[0].Report.Status != StatusError { + t.Fatalf("Report.Status = %q", report.Tasks[0].Report.Status) + } + if !strings.Contains(report.Tasks[0].Report.Error, "materialization failed") { + t.Fatalf("Report.Error = %q", report.Tasks[0].Report.Error) + } + if agentCalled { + t.Fatal("agent should not run when materialization fails") + } +} + +func TestHarnessAppliesPerTaskTimeout(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + var hadDeadline bool + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(ctx context.Context, input AgentRunInput) AgentRunResult { + _, hadDeadline = ctx.Deadline() + target := filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + if err := os.WriteFile(target, []byte("updated"), 0o644); err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + }, + } + + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + Timeout: 30 * time.Second, + }) + + if !report.OK { + t.Fatalf("OK = false; report=%#v", report) + } + if !hadDeadline { + t.Fatal("expected agent context to carry a deadline when Timeout is set") + } +} + +func TestHarnessTimeoutCancelsBlockedAgent(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + agentReached := false + sawCancel := false + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(ctx context.Context, _ AgentRunInput) AgentRunResult { + agentReached = true + <-ctx.Done() + sawCancel = ctx.Err() != nil + return AgentRunResult{ExitCode: -1, Error: ctx.Err().Error()} + }), + Runner: Runner{RunCommand: func(context.Context, string, Command) CommandResult { + t.Fatal("runner should not score after a timed-out agent run") + return CommandResult{} + }}, + } + + // The timeout is far longer than materialization of the small fixture, so + // the agent is always reached; it then blocks until the deadline fires. + report := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + Timeout: 2 * time.Second, + }) + + if !agentReached { + t.Fatal("agent was never reached; the timeout fired before the agent ran") + } + if !sawCancel { + t.Fatal("agent ran but never observed context cancellation") + } + if report.OK { + t.Fatalf("OK = true; expected the timeout to fail the task; report=%#v", report) + } + if report.Tasks[0].Report.Status != StatusBlocked { + t.Fatalf("Report.Status = %q, want blocked after agent timeout", report.Tasks[0].Report.Status) + } +} + +func TestHarnessRemovesWorkspacesByDefaultAndKeepsWhenRequested(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + harness := Harness{ + Materializer: Materializer{}, + Agent: AgentRunnerFunc(func(_ context.Context, input AgentRunInput) AgentRunResult { + target := filepath.Join(input.WorkspacePath, "docs", "STREAM_JSON_PROTOCOL.md") + if err := os.WriteFile(target, []byte("updated"), 0o644); err != nil { + return AgentRunResult{ExitCode: -1, Error: err.Error()} + } + return AgentRunResult{ExitCode: 0} + }), + Runner: Runner{ + RunCommand: func(_ context.Context, _ string, command Command) CommandResult { + return CommandResult{ID: command.ID, ExitCode: 0} + }, + }, + } + + removed := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + }) + if !removed.OK { + t.Fatalf("expected passing report, got %#v", removed) + } + if _, err := os.Stat(removed.Tasks[0].WorkspacePath); !os.IsNotExist(err) { + t.Fatalf("default run should remove workspace, stat err=%v", err) + } + + kept := harness.Run(context.Background(), suitePath, suite, BenchmarkInput{ + TaskID: "document-stream-json-verify-events", + WorkRoot: t.TempDir(), + KeepWorkspaces: true, + }) + if !kept.OK { + t.Fatalf("expected passing report, got %#v", kept) + } + if _, err := os.Stat(kept.Tasks[0].WorkspacePath); err != nil { + t.Fatalf("keep-workspaces should preserve workspace: %v", err) + } +} diff --git a/internal/agenteval/context_checks.go b/internal/agenteval/context_checks.go new file mode 100644 index 000000000..3d207061d --- /dev/null +++ b/internal/agenteval/context_checks.go @@ -0,0 +1,105 @@ +package agenteval + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +type ContextChecks struct { + RequiredFiles []string `json:"requiredFiles,omitempty"` + ForbiddenFiles []string `json:"forbiddenFiles,omitempty"` +} + +type ContextCheckResult struct { + MissingRequiredFiles []string + PresentForbiddenFiles []string +} + +func (result ContextCheckResult) OK() bool { + return len(result.MissingRequiredFiles) == 0 && len(result.PresentForbiddenFiles) == 0 +} + +func (checks ContextChecks) CheckWorkspace(workspace string) (ContextCheckResult, error) { + workspace = strings.TrimSpace(workspace) + if workspace == "" { + return ContextCheckResult{}, errors.New("workspace path is required") + } + required, requiredProblems := normalizeContextCheckFiles("requiredFiles", checks.RequiredFiles) + forbidden, forbiddenProblems := normalizeContextCheckFiles("forbiddenFiles", checks.ForbiddenFiles) + if len(requiredProblems) > 0 || len(forbiddenProblems) > 0 { + return ContextCheckResult{}, ValidationError{Problems: append(requiredProblems, forbiddenProblems...)} + } + + workspacePath, err := filepath.Abs(workspace) + if err != nil { + return ContextCheckResult{}, fmt.Errorf("resolve workspace path: %w", err) + } + info, err := os.Stat(workspacePath) + if err != nil { + return ContextCheckResult{}, fmt.Errorf("workspace: %w", err) + } + if !info.IsDir() { + return ContextCheckResult{}, fmt.Errorf("workspace must be a directory: %s", workspacePath) + } + + result := ContextCheckResult{} + for _, file := range required { + exists, err := workspacePathExists(workspacePath, file) + if err != nil { + return ContextCheckResult{}, err + } + if !exists { + result.MissingRequiredFiles = append(result.MissingRequiredFiles, file) + } + } + for _, file := range forbidden { + exists, err := workspacePathExists(workspacePath, file) + if err != nil { + return ContextCheckResult{}, err + } + if exists { + result.PresentForbiddenFiles = append(result.PresentForbiddenFiles, file) + } + } + return result, nil +} + +func normalizeContextCheckFiles(field string, files []string) ([]string, []string) { + normalized := make([]string, 0, len(files)) + problems := []string{} + seen := map[string]bool{} + for index, file := range files { + clean, ok := normalizeEvalPath(file) + if !ok { + problems = append(problems, fmt.Sprintf("%s[%d] must be a relative workspace path", field, index)) + continue + } + if clean == "" { + problems = append(problems, fmt.Sprintf("%s[%d] must not be empty", field, index)) + continue + } + if seen[clean] { + continue + } + seen[clean] = true + normalized = append(normalized, clean) + } + sort.Strings(normalized) + return normalized, problems +} + +func workspacePathExists(workspace string, file string) (bool, error) { + path := filepath.Join(workspace, filepath.FromSlash(file)) + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return false, fmt.Errorf("stat workspace path %q: %w", file, err) +} diff --git a/internal/agenteval/context_checks_test.go b/internal/agenteval/context_checks_test.go new file mode 100644 index 000000000..205da5bff --- /dev/null +++ b/internal/agenteval/context_checks_test.go @@ -0,0 +1,50 @@ +package agenteval + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +func TestEvaluateContextChecksReportsMissingAndForbiddenFiles(t *testing.T) { + workspace := t.TempDir() + writeContextFile(t, workspace, "docs/present.md") + writeContextFile(t, workspace, "logs/leak.txt") + + got, err := (ContextChecks{ + RequiredFiles: []string{"docs/present.md", "docs/missing.md"}, + ForbiddenFiles: []string{"logs/leak.txt", "logs/not-there.txt"}, + }).CheckWorkspace(workspace) + + if err != nil { + t.Fatalf("CheckWorkspace returned error: %v", err) + } + if !reflect.DeepEqual(got.MissingRequiredFiles, []string{"docs/missing.md"}) { + t.Fatalf("missing required = %#v", got.MissingRequiredFiles) + } + if !reflect.DeepEqual(got.PresentForbiddenFiles, []string{"logs/leak.txt"}) { + t.Fatalf("present forbidden = %#v", got.PresentForbiddenFiles) + } +} + +func TestEvaluateContextChecksRejectsMissingWorkspace(t *testing.T) { + _, err := (ContextChecks{ + RequiredFiles: []string{"docs/present.md"}, + }).CheckWorkspace(filepath.Join(t.TempDir(), "missing")) + + if err == nil { + t.Fatal("expected workspace error") + } +} + +func writeContextFile(t *testing.T, workspace string, file string) { + t.Helper() + path := filepath.Join(workspace, filepath.FromSlash(file)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { + t.Fatalf("write %s: %v", file, err) + } +} diff --git a/internal/agenteval/materialize.go b/internal/agenteval/materialize.go new file mode 100644 index 000000000..0750c30c0 --- /dev/null +++ b/internal/agenteval/materialize.go @@ -0,0 +1,194 @@ +package agenteval + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" +) + +type Materializer struct{} + +type MaterializeInput struct { + WorkRoot string +} + +type Workspace struct { + Path string + TaskID string + FixturePath string +} + +func (Materializer) MaterializeTask(ctx context.Context, suitePath string, task Task, input MaterializeInput) (Workspace, error) { + if ctx == nil { + ctx = context.Background() + } + workRoot := strings.TrimSpace(input.WorkRoot) + if workRoot == "" { + return Workspace{}, errors.New("work root is required") + } + fixturePath, err := resolveFixturePath(suitePath, task.WorkspaceFixture) + if err != nil { + return Workspace{}, err + } + info, err := os.Stat(fixturePath) + if err != nil { + return Workspace{}, fmt.Errorf("workspace fixture: %w", err) + } + if !info.IsDir() { + return Workspace{}, fmt.Errorf("workspace fixture must be a directory: %s", fixturePath) + } + if err := os.MkdirAll(workRoot, 0o755); err != nil { + return Workspace{}, fmt.Errorf("create work root: %w", err) + } + workspacePath, err := os.MkdirTemp(workRoot, safeWorkspaceName(task.ID)+"-") + if err != nil { + return Workspace{}, fmt.Errorf("create workspace: %w", err) + } + if err := copyFixtureDir(fixturePath, workspacePath); err != nil { + _ = os.RemoveAll(workspacePath) + return Workspace{}, err + } + if err := initGitBaseline(ctx, workspacePath); err != nil { + _ = os.RemoveAll(workspacePath) + return Workspace{}, err + } + return Workspace{Path: workspacePath, TaskID: task.ID, FixturePath: fixturePath}, nil +} + +func resolveFixturePath(suitePath string, fixture string) (string, error) { + fixture = strings.TrimSpace(fixture) + if fixture == "" { + return "", errors.New("workspace fixture is required") + } + if filepath.IsAbs(fixture) { + return "", errors.New("workspace fixture must be relative to suite path") + } + base, err := filepath.Abs(filepath.Dir(suitePath)) + if err != nil { + return "", err + } + resolved, err := filepath.Abs(filepath.Join(base, filepath.FromSlash(fixture))) + if err != nil { + return "", err + } + rel, err := filepath.Rel(base, resolved) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { + return "", errors.New("workspace fixture must stay within suite directory") + } + return resolved, nil +} + +func copyFixtureDir(src string, dst string) error { + return filepath.WalkDir(src, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() && entry.Name() == ".git" { + return filepath.SkipDir + } + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + if rel == "." { + return nil + } + target := filepath.Join(dst, rel) + if entry.IsDir() { + return os.MkdirAll(target, 0o755) + } + info, err := entry.Info() + if err != nil { + return err + } + if !info.Mode().IsRegular() { + // Reject symlinks, devices, sockets, and other non-regular entries + // rather than silently dropping them, which would materialize an + // incomplete workspace and skew scoring. + return fmt.Errorf("unsupported fixture entry %q: only regular files and directories are supported", rel) + } + return copyFixtureFile(path, target, info.Mode().Perm()) + }) +} + +func copyFixtureFile(src string, dst string, mode os.FileMode) error { + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return err + } + source, err := os.Open(src) + if err != nil { + return err + } + defer func() { _ = source.Close() }() + target, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) + if err != nil { + return err + } + _, copyErr := io.Copy(target, source) + // Close may flush buffered data and surface write errors (e.g. disk full), + // so its error must not be discarded. + closeErr := target.Close() + if copyErr != nil { + return copyErr + } + return closeErr +} + +func initGitBaseline(ctx context.Context, workspace string) error { + commands := [][]string{ + {"init"}, + {"config", "user.name", "Zero Eval"}, + {"config", "user.email", "zero-eval@example.invalid"}, + {"add", "."}, + {"commit", "-m", "baseline"}, + } + for _, args := range commands { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = workspace + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=Zero Eval", + "GIT_AUTHOR_EMAIL=zero-eval@example.invalid", + "GIT_COMMITTER_NAME=Zero Eval", + "GIT_COMMITTER_EMAIL=zero-eval@example.invalid", + ) + var output bytes.Buffer + cmd.Stdout = &output + cmd.Stderr = &output + if err := cmd.Run(); err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + return fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(output.String())) + } + } + return nil +} + +func safeWorkspaceName(value string) string { + value = strings.TrimSpace(value) + if value == "" { + return "task" + } + var builder strings.Builder + for _, r := range value { + if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '-' || r == '_' { + builder.WriteRune(r) + } else { + builder.WriteByte('-') + } + } + name := strings.Trim(builder.String(), "-") + if name == "" { + return "task" + } + return name +} diff --git a/internal/agenteval/materialize_test.go b/internal/agenteval/materialize_test.go new file mode 100644 index 000000000..b9797d455 --- /dev/null +++ b/internal/agenteval/materialize_test.go @@ -0,0 +1,220 @@ +package agenteval + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +func TestMaterializeTaskCopiesFixtureAndInitializesGit(t *testing.T) { + suitePath := filepath.Join("testdata", "sample_suite.json") + suite, err := LoadSuite(suitePath) + if err != nil { + t.Fatal(err) + } + task := suite.Tasks[0] + workRoot := t.TempDir() + + workspace, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: workRoot}) + if err != nil { + t.Fatalf("MaterializeTask: %v", err) + } + + if workspace.TaskID != task.ID { + t.Fatalf("TaskID = %q, want %q", workspace.TaskID, task.ID) + } + if workspace.Path == "" || !strings.HasPrefix(filepath.Base(workspace.Path), task.ID) { + t.Fatalf("workspace path %q does not use task id prefix %q", workspace.Path, task.ID) + } + if workspace.FixturePath == "" || !filepath.IsAbs(workspace.FixturePath) { + t.Fatalf("FixturePath = %q, want absolute path", workspace.FixturePath) + } + if _, err := os.Stat(filepath.Join(workspace.Path, "go.mod")); err != nil { + t.Fatalf("fixture was not copied: %v", err) + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if output, err := exec.CommandContext(ctx, "git", "-C", workspace.Path, "status", "--porcelain").CombinedOutput(); err != nil || strings.TrimSpace(string(output)) != "" { + t.Fatalf("workspace baseline is dirty: err=%v output=%s", err, output) + } +} + +func TestMaterializeTaskRejectsSymlinkFixtureEntryAndCleansUp(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires privileges on windows") + } + root := t.TempDir() + suitePath := filepath.Join(root, "suite.json") + fixturePath := filepath.Join(root, "fixtures", "source") + if err := os.MkdirAll(fixturePath, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(fixturePath, "real.txt"), []byte("real"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink("real.txt", filepath.Join(fixturePath, "link.txt")); err != nil { + t.Skipf("symlink unsupported on this platform: %v", err) + } + task := Task{ID: "symlink-task", WorkspaceFixture: "fixtures/source"} + workRoot := t.TempDir() + + _, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: workRoot}) + if err == nil { + t.Fatal("MaterializeTask returned nil error for a symlink fixture entry") + } + if !strings.Contains(err.Error(), "unsupported fixture entry") { + t.Fatalf("error %q does not mention unsupported fixture entry", err) + } + entries, readErr := os.ReadDir(workRoot) + if readErr != nil { + t.Fatal(readErr) + } + if len(entries) != 0 { + t.Fatalf("work root was not cleaned up after error: %v", entries) + } +} + +func TestMaterializeTaskPreservesExecutableBit(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix file mode bits are not preserved on windows") + } + root := t.TempDir() + suitePath := filepath.Join(root, "suite.json") + fixturePath := filepath.Join(root, "fixtures", "source") + if err := os.MkdirAll(fixturePath, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(fixturePath, "run.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + task := Task{ID: "mode-task", WorkspaceFixture: "fixtures/source"} + + workspace, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: t.TempDir()}) + if err != nil { + t.Fatalf("MaterializeTask: %v", err) + } + info, err := os.Stat(filepath.Join(workspace.Path, "run.sh")) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Fatalf("executable bit not preserved: mode=%v", info.Mode()) + } +} + +func TestMaterializeTaskSkipsGitDirectory(t *testing.T) { + suitePath, task := writeMaterializerFixture(t, "fixtures/source", map[string]string{ + "keep.txt": "keep", + ".git/config": "do not copy", + ".git/objects/blob": "do not copy", + }) + + workspace, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: t.TempDir()}) + if err != nil { + t.Fatalf("MaterializeTask: %v", err) + } + + if _, err := os.Stat(filepath.Join(workspace.Path, "keep.txt")); err != nil { + t.Fatalf("fixture file was not copied: %v", err) + } + if _, err := os.Stat(filepath.Join(workspace.Path, ".git", "objects", "blob")); !os.IsNotExist(err) { + t.Fatalf("source .git directory was copied, stat err=%v", err) + } +} + +func TestMaterializerRejectsInvalidInputs(t *testing.T) { + suitePath, task := writeMaterializerFixture(t, "fixtures/source", map[string]string{ + "keep.txt": "keep", + }) + workRoot := t.TempDir() + + tests := []struct { + name string + task Task + input MaterializeInput + wantErr string + }{ + { + name: "empty work root", + task: task, + input: MaterializeInput{}, + wantErr: "work root", + }, + { + name: "missing fixture", + task: Task{ID: "missing-fixture", WorkspaceFixture: "fixtures/missing"}, + input: MaterializeInput{WorkRoot: workRoot}, + wantErr: "fixture", + }, + { + name: "absolute fixture", + task: Task{ID: "absolute-fixture", WorkspaceFixture: absoluteFixturePath()}, + input: MaterializeInput{WorkRoot: workRoot}, + wantErr: "relative", + }, + { + name: "escaping fixture", + task: Task{ID: "escaping-fixture", WorkspaceFixture: "../outside"}, + input: MaterializeInput{WorkRoot: workRoot}, + wantErr: "within", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := Materializer{}.MaterializeTask(context.Background(), suitePath, tt.task, tt.input) + if err == nil { + t.Fatal("MaterializeTask returned nil error") + } + if !strings.Contains(strings.ToLower(err.Error()), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err, tt.wantErr) + } + }) + } +} + +func TestMaterializerSanitizesWorkspacePrefix(t *testing.T) { + suitePath, task := writeMaterializerFixture(t, "fixtures/source", map[string]string{ + "keep.txt": "keep", + }) + task.ID = "Bad Task:../ID" + + workspace, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: t.TempDir()}) + if err != nil { + t.Fatalf("MaterializeTask: %v", err) + } + + base := filepath.Base(workspace.Path) + if strings.ContainsAny(base, ` :\/`) || !strings.HasPrefix(base, "Bad-Task") { + t.Fatalf("workspace base = %q, want sanitized prefix", base) + } +} + +func writeMaterializerFixture(t *testing.T, fixture string, files map[string]string) (string, Task) { + t.Helper() + root := t.TempDir() + suitePath := filepath.Join(root, "suite.json") + fixturePath := filepath.Join(root, filepath.FromSlash(fixture)) + for name, content := range files { + path := filepath.Join(fixturePath, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + return suitePath, Task{ID: "copy-fixture", WorkspaceFixture: fixture} +} + +func absoluteFixturePath() string { + if runtime.GOOS == "windows" { + return `C:\absolute\fixture` + } + return "/absolute/fixture" +} diff --git a/internal/agenteval/run.go b/internal/agenteval/run.go index 713e0773f..e6f13a500 100644 --- a/internal/agenteval/run.go +++ b/internal/agenteval/run.go @@ -20,6 +20,7 @@ const defaultCommandTimeout = 10 * time.Minute type RunInput struct { TaskID string WorkspacePath string + TraceStdout string // CommandTimeout bounds each verification command. Non-positive applies // defaultCommandTimeout. CommandTimeout time.Duration @@ -51,6 +52,16 @@ func (runner Runner) Run(ctx context.Context, suite Suite, input RunInput) Repor if err := ctx.Err(); err != nil { return runner.blocked(suite, task.ID, err.Error(), nil) } + var contextResult *ContextCheckResult + var contextError string + if len(task.ContextChecks.RequiredFiles) > 0 || len(task.ContextChecks.ForbiddenFiles) > 0 { + result, err := task.ContextChecks.CheckWorkspace(workspace) + if err != nil { + contextError = err.Error() + } else { + contextResult = &result + } + } timeout := input.CommandTimeout if timeout <= 0 { @@ -75,9 +86,12 @@ func (runner Runner) Run(ctx context.Context, suite Suite, input RunInput) Repor } return Score(suite, ScoreInput{ - TaskID: task.ID, - CommandResults: results, - ChangedFiles: files, + TaskID: task.ID, + CommandResults: results, + ChangedFiles: files, + ContextCheckResult: contextResult, + ContextCheckError: contextError, + TraceStdout: input.TraceStdout, }) } diff --git a/internal/agenteval/score.go b/internal/agenteval/score.go index 08e7acf67..73002902a 100644 --- a/internal/agenteval/score.go +++ b/internal/agenteval/score.go @@ -21,14 +21,19 @@ type ResultKind string const ( ResultCommand ResultKind = "command" ResultChangedFiles ResultKind = "changed_files" + ResultContext ResultKind = "context" + ResultTrace ResultKind = "trace" ) type ScoreInput struct { - TaskID string - CommandResults []CommandResult - ChangedFiles []string - Blocked bool - BlockReason string + TaskID string + CommandResults []CommandResult + ChangedFiles []string + ContextCheckResult *ContextCheckResult + ContextCheckError string + TraceStdout string + Blocked bool + BlockReason string } type CommandResult struct { @@ -73,6 +78,9 @@ type Result struct { ActualFiles []string `json:"actualFiles,omitempty"` MissingFiles []string `json:"missingFiles,omitempty"` UnexpectedFiles []string `json:"unexpectedFiles,omitempty"` + ExpectedEvents []string `json:"expectedEvents,omitempty"` + ActualEvents []string `json:"actualEvents,omitempty"` + MissingEvents []string `json:"missingEvents,omitempty"` } func Score(suite Suite, input ScoreInput) Report { @@ -108,6 +116,15 @@ func Score(suite Suite, input ScoreInput) Report { report.Results = append(report.Results, scoreCommand(command, result, found, input)) } report.Results = append(report.Results, scoreChangedFiles(task.ExpectedChangedFiles, report.ChangedFiles, input)) + if len(task.ForbiddenChangedFiles) > 0 { + report.Results = append(report.Results, scoreForbiddenChangedFiles(task.ForbiddenChangedFiles, report.ChangedFiles, input)) + } + if len(task.ContextChecks.RequiredFiles) > 0 || len(task.ContextChecks.ForbiddenFiles) > 0 { + report.Results = append(report.Results, scoreContextChecks(task.ContextChecks, input)) + } + if len(task.RequiredTraceEvents) > 0 { + report.Results = append(report.Results, scoreTraceEvents(task.RequiredTraceEvents, input)) + } for _, result := range unknownCommandResults(input.CommandResults, seenCommands, input) { report.Results = append(report.Results, result) } @@ -172,6 +189,83 @@ func scoreChangedFiles(expected []string, actual []string, input ScoreInput) Res return result } +func scoreForbiddenChangedFiles(forbidden []string, actual []string, input ScoreInput) Result { + result := Result{ + ID: "forbidden_changed_files", + Name: "Forbidden changed files", + Kind: ResultChangedFiles, + ExpectedFiles: append([]string{}, forbidden...), + ActualFiles: append([]string{}, actual...), + } + if input.Blocked { + result.Status = StatusBlocked + result.Message = blockMessage(input.BlockReason) + return result + } + result.UnexpectedFiles = intersectFiles(actual, forbidden) + if len(result.UnexpectedFiles) > 0 { + result.Status = StatusFail + return result + } + result.Status = StatusPass + return result +} + +func scoreContextChecks(checks ContextChecks, input ScoreInput) Result { + result := Result{ + ID: "context_checks", + Name: "Context quality checks", + Kind: ResultContext, + ExpectedFiles: append([]string{}, checks.RequiredFiles...), + } + if input.Blocked { + result.Status = StatusBlocked + result.Message = blockMessage(input.BlockReason) + return result + } + if input.ContextCheckError != "" { + result.Status = StatusError + result.Message = input.ContextCheckError + return result + } + if input.ContextCheckResult == nil { + result.Status = StatusError + result.Message = "missing context check result" + return result + } + result.MissingFiles = append([]string{}, input.ContextCheckResult.MissingRequiredFiles...) + result.UnexpectedFiles = append([]string{}, input.ContextCheckResult.PresentForbiddenFiles...) + if len(result.MissingFiles) > 0 || len(result.UnexpectedFiles) > 0 { + result.Status = StatusFail + return result + } + result.Status = StatusPass + return result +} + +func scoreTraceEvents(required []string, input ScoreInput) Result { + actual := ParseTraceEventKeys(input.TraceStdout) + result := Result{ + ID: "trace_events", + Name: "Required agent trace events", + Kind: ResultTrace, + ExpectedEvents: append([]string{}, required...), + ActualEvents: actual, + } + if input.Blocked { + result.Status = StatusBlocked + result.Message = blockMessage(input.BlockReason) + return result + } + result.MissingEvents = diffFiles(required, actual) + if len(result.MissingEvents) > 0 { + result.Status = StatusFail + return result + } + result.Status = StatusPass + return result +} + func unknownCommandResults(results []CommandResult, seen map[string]bool, input ScoreInput) []Result { unknownByID := map[string]CommandResult{} for _, result := range results { @@ -265,6 +359,10 @@ func selectTask(suite Suite, taskID string) (Task, error) { func normalizeTask(task Task) Task { task.ExpectedChangedFiles = normalizeFiles(task.ExpectedChangedFiles) + task.ForbiddenChangedFiles = normalizeFiles(task.ForbiddenChangedFiles) + task.RequiredTraceEvents = normalizeStrings(task.RequiredTraceEvents) + task.ContextChecks.RequiredFiles = normalizeFiles(task.ContextChecks.RequiredFiles) + task.ContextChecks.ForbiddenFiles = normalizeFiles(task.ContextChecks.ForbiddenFiles) return task } @@ -283,6 +381,23 @@ func diffFiles(left []string, right []string) []string { return diff } +func intersectFiles(left []string, right []string) []string { + rightSet := map[string]bool{} + for _, file := range right { + rightSet[file] = true + } + intersection := []string{} + seen := map[string]bool{} + for _, file := range left { + if rightSet[file] && !seen[file] { + seen[file] = true + intersection = append(intersection, file) + } + } + sort.Strings(intersection) + return intersection +} + func blockMessage(reason string) string { if reason == "" { return "run blocked" diff --git a/internal/agenteval/suite.go b/internal/agenteval/suite.go index e46296dc6..03d7eafbc 100644 --- a/internal/agenteval/suite.go +++ b/internal/agenteval/suite.go @@ -19,13 +19,18 @@ type Suite struct { } type Task struct { - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description,omitempty"` - Prompt string `json:"prompt"` - WorkspaceFixture string `json:"workspaceFixture"` - VerificationCommands []Command `json:"verificationCommands"` - ExpectedChangedFiles []string `json:"expectedChangedFiles"` + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description,omitempty"` + Tags []string `json:"tags,omitempty"` + Difficulty string `json:"difficulty,omitempty"` + Prompt string `json:"prompt"` + WorkspaceFixture string `json:"workspaceFixture"` + VerificationCommands []Command `json:"verificationCommands"` + ExpectedChangedFiles []string `json:"expectedChangedFiles"` + ForbiddenChangedFiles []string `json:"forbiddenChangedFiles,omitempty"` + RequiredTraceEvents []string `json:"requiredTraceEvents,omitempty"` + ContextChecks ContextChecks `json:"contextChecks,omitempty"` } type Command struct { @@ -102,7 +107,11 @@ func (suite Suite) Validate() error { if len(task.VerificationCommands) == 0 { problems = append(problems, taskPath+" verificationCommands must not be empty") } - problems = append(problems, validateExpectedChangedFiles(taskPath, task.ExpectedChangedFiles)...) + problems = append(problems, validateFileList(taskPath, "expectedChangedFiles", task.ExpectedChangedFiles, true)...) + problems = append(problems, validateFileList(taskPath, "forbiddenChangedFiles", task.ForbiddenChangedFiles, false)...) + problems = append(problems, validateFileList(taskPath, "contextChecks.requiredFiles", task.ContextChecks.RequiredFiles, false)...) + problems = append(problems, validateFileList(taskPath, "contextChecks.forbiddenFiles", task.ContextChecks.ForbiddenFiles, false)...) + problems = append(problems, validateStringList(taskPath, "requiredTraceEvents", task.RequiredTraceEvents)...) commandIndexes := map[string]int{} for commandIndex, command := range task.VerificationCommands { commandPath := fmt.Sprintf("%s verificationCommands[%d]", taskPath, commandIndex) @@ -130,6 +139,10 @@ func (suite Suite) Validate() error { func (suite *Suite) normalize() { for taskIndex := range suite.Tasks { suite.Tasks[taskIndex].ExpectedChangedFiles = normalizeFiles(suite.Tasks[taskIndex].ExpectedChangedFiles) + suite.Tasks[taskIndex].ForbiddenChangedFiles = normalizeFiles(suite.Tasks[taskIndex].ForbiddenChangedFiles) + suite.Tasks[taskIndex].RequiredTraceEvents = normalizeStrings(suite.Tasks[taskIndex].RequiredTraceEvents) + suite.Tasks[taskIndex].ContextChecks.RequiredFiles = normalizeFiles(suite.Tasks[taskIndex].ContextChecks.RequiredFiles) + suite.Tasks[taskIndex].ContextChecks.ForbiddenFiles = normalizeFiles(suite.Tasks[taskIndex].ContextChecks.ForbiddenFiles) } } @@ -148,24 +161,24 @@ func normalizeFiles(files []string) []string { return normalized } -func validateExpectedChangedFiles(taskPath string, files []string) []string { - if len(files) == 0 { - return []string{taskPath + " expectedChangedFiles must not be empty"} +func validateFileList(taskPath string, field string, files []string, required bool) []string { + if required && len(files) == 0 { + return []string{taskPath + " " + field + " must not be empty"} } problems := []string{} seen := map[string]int{} for index, file := range files { normalized, ok := normalizeEvalPath(file) if !ok { - problems = append(problems, fmt.Sprintf("%s expectedChangedFiles[%d] must be a relative workspace path", taskPath, index)) + problems = append(problems, fmt.Sprintf("%s %s[%d] must be a relative workspace path", taskPath, field, index)) continue } if normalized == "" { - problems = append(problems, fmt.Sprintf("%s expectedChangedFiles[%d] must not be empty", taskPath, index)) + problems = append(problems, fmt.Sprintf("%s %s[%d] must not be empty", taskPath, field, index)) continue } if previous, ok := seen[normalized]; ok { - problems = append(problems, fmt.Sprintf("%s expectedChangedFiles[%d] duplicates expectedChangedFiles[%d]", taskPath, index, previous)) + problems = append(problems, fmt.Sprintf("%s %s[%d] duplicates %s[%d]", taskPath, field, index, field, previous)) continue } seen[normalized] = index @@ -173,6 +186,28 @@ func validateExpectedChangedFiles(taskPath string, files []string) []string { return problems } +func validateExpectedChangedFiles(taskPath string, files []string) []string { + return validateFileList(taskPath, "expectedChangedFiles", files, true) +} + +func validateStringList(taskPath string, field string, values []string) []string { + problems := []string{} + seen := map[string]int{} + for index, value := range values { + value = strings.TrimSpace(value) + if value == "" { + problems = append(problems, fmt.Sprintf("%s %s[%d] must not be empty", taskPath, field, index)) + continue + } + if previous, ok := seen[value]; ok { + problems = append(problems, fmt.Sprintf("%s %s[%d] duplicates %s[%d]", taskPath, field, index, field, previous)) + continue + } + seen[value] = index + } + return problems +} + func normalizeEvalPath(file string) (string, bool) { file = strings.TrimSpace(strings.ReplaceAll(file, "\\", "/")) if file == "" { @@ -192,6 +227,21 @@ func blank(value string) bool { return strings.TrimSpace(value) == "" } +func normalizeStrings(values []string) []string { + seen := map[string]bool{} + normalized := make([]string, 0, len(values)) + for _, value := range values { + value = strings.TrimSpace(value) + if value == "" || seen[value] { + continue + } + seen[value] = true + normalized = append(normalized, value) + } + sort.Strings(normalized) + return normalized +} + func emptyCommand(command []string) bool { if len(command) == 0 { return true diff --git a/internal/agenteval/testdata/sample_suite.json b/internal/agenteval/testdata/sample_suite.json index e0fcc5d53..67314bc3c 100644 --- a/internal/agenteval/testdata/sample_suite.json +++ b/internal/agenteval/testdata/sample_suite.json @@ -1,17 +1,37 @@ { "id": "zero-local-agent-quality", "name": "zero-local-agent-quality", - "description": "Clean-room local fixture suite for exercising Zero coding-agent task success without live provider calls.", + "description": "Clean-room local fixture suite for exercising Zero coding-agent task success without provider or network calls.", "tasks": [ { "id": "document-stream-json-verify-events", "name": "Document stream JSON verify events", - "description": "Docs-only task covering the verify and self-verify event contract.", - "prompt": "Update the stream-json protocol docs so maintainers can see how `zero verify --json` and `zero selfverify --json` report start, check, summary, and error events. Keep the wording accurate, local-first, and do not change runtime code.", + "description": "Docs-only, low-risk task covering the verify and self-verify JSON event contract.", + "tags": [ + "docs", + "jsonl", + "verify" + ], + "difficulty": "easy", + "prompt": "Update the stream-json protocol docs so maintainers can see how `zero verify --json` and `zero selfverify --json` report start, check, summary, and error events. Keep the wording accurate, local-first, deterministic, and do not change runtime code.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "docs/STREAM_JSON_PROTOCOL.md" ], + "forbiddenChangedFiles": [ + "go.mod", + "package.json" + ], + "contextChecks": { + "requiredFiles": [ + "docs/STREAM_JSON_PROTOCOL.md", + "internal/verify/verify.go", + "internal/selfverify/selfverify.go" + ], + "forbiddenFiles": [ + "node_modules/cache.txt" + ] + }, "verificationCommands": [ { "id": "verify-contract-tests", @@ -28,12 +48,29 @@ { "id": "separate-go-release-from-node-wrapper-smoke", "name": "Separate Go release checks from Node wrapper checks", - "description": "Docs task that keeps the npm wrapper guidance honest and dependency-free.", - "prompt": "Improve the npm wrapper smoke checklist so a maintainer can distinguish Go release checks from direct `bin/zero.js` wrapper checks. Keep Node usage limited to the wrapper itself and do not add Bun, TypeScript, or package build dependencies.", + "description": "Docs-only task that keeps npm wrapper guidance honest and dependency-free.", + "tags": [ + "docs", + "release", + "npm-wrapper" + ], + "difficulty": "easy", + "prompt": "Improve the npm wrapper smoke checklist so a maintainer can distinguish Go release checks from direct `bin/zero.js` wrapper checks. Keep Node usage limited to the wrapper itself, avoid provider or network calls, and do not add Bun, TypeScript, or package build dependencies.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "docs/NPM_WRAPPER_SMOKE.md" ], + "forbiddenChangedFiles": [ + "package.json", + "go.mod" + ], + "contextChecks": { + "requiredFiles": [ + "bin/zero.js", + "docs/NPM_WRAPPER_SMOKE.md", + "internal/npmwrapper/wrapper.go" + ] + }, "verificationCommands": [ { "id": "npm-wrapper-release-tests", @@ -50,12 +87,31 @@ { "id": "add-provider-catalog-stability-coverage", "name": "Add provider catalog stability coverage", - "description": "Focused test task for model registry aliases and provider names.", - "prompt": "Add focused tests for provider catalog defaults so common model aliases and provider display names remain stable. Keep the change inside the model registry package.", + "description": "Test-only task for model registry aliases and provider names.", + "tags": [ + "providers", + "models", + "tests" + ], + "difficulty": "medium", + "prompt": "Add focused tests for provider catalog defaults so common model aliases and provider display names remain stable. Keep the change inside the model registry package, use only the local fixture code, and do not call provider APIs.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "internal/modelregistry/catalog_test.go" ], + "forbiddenChangedFiles": [ + "internal/modelregistry/catalog.go", + "go.mod" + ], + "requiredTraceEvents": [ + "tool:read_file", + "tool:apply_patch" + ], + "contextChecks": { + "requiredFiles": [ + "internal/modelregistry/catalog.go" + ] + }, "verificationCommands": [ { "id": "modelregistry-tests", @@ -71,13 +127,34 @@ { "id": "tighten-config-validation-errors", "name": "Tighten config validation errors", - "description": "Small Go task for deterministic config validation behavior.", - "prompt": "Make config validation return actionable errors when the default provider is missing from the provider map, and cover the behavior with a table test. Do not change config file locations or environment variable names.", + "description": "Small Go behavior task for deterministic config validation errors.", + "tags": [ + "config", + "errors", + "tests" + ], + "difficulty": "medium", + "prompt": "Make config validation return actionable errors when the default provider is missing from the provider map, and cover the behavior with a table test. Do not change config file locations, environment variable names, or provider lookup semantics.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "internal/config/validate.go", "internal/config/validate_test.go" ], + "forbiddenChangedFiles": [ + "internal/config/config.go", + "go.mod" + ], + "requiredTraceEvents": [ + "tool:read_file", + "tool:grep", + "tool:apply_patch" + ], + "contextChecks": { + "requiredFiles": [ + "internal/config/validate.go", + "internal/config/validate_test.go" + ] + }, "verificationCommands": [ { "id": "config-tests", @@ -94,12 +171,32 @@ "id": "preserve-mcp-server-order", "name": "Preserve MCP server order", "description": "Regression task for deterministic MCP configuration output.", - "prompt": "Add coverage showing MCP server names are emitted in deterministic sorted order when rendering config summaries. Keep the production change, if any, inside the MCP config package.", + "tags": [ + "mcp", + "determinism", + "tests" + ], + "difficulty": "medium", + "prompt": "Add coverage showing MCP server names are emitted in deterministic sorted order when rendering config summaries. Keep the production change, if any, inside the MCP config package and do not introduce external dependencies.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "internal/mcp/config.go", "internal/mcp/config_test.go" ], + "forbiddenChangedFiles": [ + "go.mod", + "package.json" + ], + "requiredTraceEvents": [ + "tool:read_file", + "tool:apply_patch" + ], + "contextChecks": { + "requiredFiles": [ + "internal/mcp/config.go", + "internal/mcp/config_test.go" + ] + }, "verificationCommands": [ { "id": "mcp-tests", @@ -116,12 +213,32 @@ "id": "clarify-agent-permission-denial-copy", "name": "Clarify agent permission denial copy", "description": "Prompt and test task for local permission-denial messaging.", - "prompt": "Clarify the local agent system prompt so denied file operations explain the blocked path and the next safe action. Add a focused test that checks the denial guidance without depending on a live model.", + "tags": [ + "agent", + "permissions", + "prompt" + ], + "difficulty": "medium", + "prompt": "Clarify the local agent system prompt so denied file operations explain the blocked path and the next safe action. Add a focused test that checks the denial guidance without depending on a live model or provider response.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "internal/agent/system_prompt.md", "internal/agent/denial_test.go" ], + "forbiddenChangedFiles": [ + "internal/tools/bash.go", + "internal/sandbox/policy.go" + ], + "requiredTraceEvents": [ + "tool:read_file", + "tool:apply_patch" + ], + "contextChecks": { + "requiredFiles": [ + "internal/agent/system_prompt.md", + "internal/agent/denial_test.go" + ] + }, "verificationCommands": [ { "id": "agent-prompt-tests", @@ -138,12 +255,33 @@ "id": "add-release-smoke-dry-run-note", "name": "Add release smoke dry-run note", "description": "Release-facing task that ties docs to local smoke behavior.", - "prompt": "Update the release smoke guidance and tests so maintainers can tell when a smoke run is using an already-built local binary. Keep the checks local and deterministic.", + "tags": [ + "release", + "smoke", + "docs" + ], + "difficulty": "medium", + "prompt": "Update the release smoke guidance and tests so maintainers can tell when a smoke run is using an already-built local binary. Keep the checks local, deterministic, and limited to the release command fixture.", "workspaceFixture": "fixtures/zero-mini", "expectedChangedFiles": [ "cmd/zero-release/main_test.go", "docs/RELEASE_SMOKE.md" ], + "forbiddenChangedFiles": [ + "go.mod", + "bin/zero.js" + ], + "requiredTraceEvents": [ + "tool:read_file", + "tool:apply_patch" + ], + "contextChecks": { + "requiredFiles": [ + "cmd/zero-release/main.go", + "cmd/zero-release/main_test.go", + "docs/RELEASE_SMOKE.md" + ] + }, "verificationCommands": [ { "id": "release-command-tests", @@ -156,6 +294,49 @@ ] } ] + }, + { + "id": "add-npm-wrapper-argv-helper", + "name": "Add npm wrapper argv helper", + "description": "Small Go task that improves wrapper command construction without adding JavaScript tooling.", + "tags": [ + "npm-wrapper", + "go", + "tests" + ], + "difficulty": "medium", + "prompt": "Refactor the npm wrapper helper to expose a reusable `WrapperCommand(args ...string) []string` that always prefixes `node bin/zero.js`, then update `DirectCommand` to use it for `--version`. Add focused tests for argument preservation. Keep Node usage limited to invoking `bin/zero.js` and do not add Bun, TypeScript, or package build dependencies.", + "workspaceFixture": "fixtures/zero-mini", + "expectedChangedFiles": [ + "internal/npmwrapper/wrapper.go", + "internal/npmwrapper/wrapper_test.go" + ], + "forbiddenChangedFiles": [ + "package.json", + "go.mod" + ], + "requiredTraceEvents": [ + "tool:read_file", + "tool:apply_patch" + ], + "contextChecks": { + "requiredFiles": [ + "bin/zero.js", + "internal/npmwrapper/wrapper.go", + "internal/npmwrapper/wrapper_test.go" + ] + }, + "verificationCommands": [ + { + "id": "npm-wrapper-tests", + "name": "NPM wrapper tests", + "command": [ + "go", + "test", + "./internal/npmwrapper" + ] + } + ] } ] } diff --git a/internal/agenteval/trace.go b/internal/agenteval/trace.go new file mode 100644 index 000000000..1ac36fc78 --- /dev/null +++ b/internal/agenteval/trace.go @@ -0,0 +1,85 @@ +package agenteval + +import ( + "encoding/json" + "sort" + "strings" +) + +func ParseTraceEventKeys(stdout string) []string { + seen := map[string]bool{} + for _, line := range strings.Split(stdout, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + var event map[string]any + if err := json.Unmarshal([]byte(line), &event); err != nil { + continue + } + if key, ok := traceEventKey(event); ok { + seen[key] = true + } + } + keys := make([]string, 0, len(seen)) + for key := range seen { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func TraceEventKeys(stdout string) []string { + return ParseTraceEventKeys(stdout) +} + +func MissingTraceEvents(required []string, stdout string) []string { + seen := map[string]bool{} + for _, key := range ParseTraceEventKeys(stdout) { + seen[key] = true + } + missingSet := map[string]bool{} + for _, key := range required { + key = strings.TrimSpace(key) + if key == "" || seen[key] { + continue + } + missingSet[key] = true + } + missing := make([]string, 0, len(missingSet)) + for key := range missingSet { + missing = append(missing, key) + } + sort.Strings(missing) + return missing +} + +func traceEventKey(event map[string]any) (string, bool) { + if eventType, ok := traceString(event, "type"); ok { + return traceKey(eventType, event), true + } + if eventType, ok := traceString(event, "event"); ok { + return traceKey(eventType, event), true + } + return "", false +} + +func traceKey(eventType string, event map[string]any) string { + name, ok := traceString(event, "name") + if !ok { + return eventType + } + return eventType + ":" + name +} + +func traceString(event map[string]any, field string) (string, bool) { + value, ok := event[field].(string) + if !ok { + return "", false + } + value = strings.TrimSpace(value) + if value == "" { + return "", false + } + return value, true +} diff --git a/internal/agenteval/trace_test.go b/internal/agenteval/trace_test.go new file mode 100644 index 000000000..1e9d50970 --- /dev/null +++ b/internal/agenteval/trace_test.go @@ -0,0 +1,31 @@ +package agenteval + +import ( + "reflect" + "testing" +) + +func TestParseTraceEventKeysIgnoresNoiseAndNormalizesJSONLines(t *testing.T) { + stdout := "starting agent\n" + + "{\"type\":\"tool\",\"name\":\"read_file\"}\n" + + "{\"event\":\"verify\",\"name\":\"go-test\"}\n" + + "{\"type\":\"finish\"}\n" + + "{\"event\":\"verify\"}\n" + + "not-json\n" + + got := ParseTraceEventKeys(stdout) + want := []string{"finish", "tool:read_file", "verify", "verify:go-test"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("event keys = %#v, want %#v", got, want) + } +} + +func TestMissingTraceEventsReturnsSortedMissingRequiredEvents(t *testing.T) { + stdout := "{\"type\":\"tool\",\"name\":\"read_file\"}\n" + + got := MissingTraceEvents([]string{"tool:apply_patch", "tool:read_file", "verify:go-test"}, stdout) + want := []string{"tool:apply_patch", "verify:go-test"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("missing events = %#v, want %#v", got, want) + } +} diff --git a/internal/cli/agent_eval.go b/internal/cli/agent_eval.go index fdb6e02d1..f6aabb8a1 100644 --- a/internal/cli/agent_eval.go +++ b/internal/cli/agent_eval.go @@ -3,39 +3,58 @@ package cli import ( "context" "encoding/json" + "errors" "fmt" "io" "os" "path/filepath" "strings" + "time" "github.com/Gitlawb/zero/internal/agenteval" ) type agentEvalOptions struct { - Mode string `json:"mode"` - SuitePath string `json:"suite_path"` - TaskID string `json:"task_id,omitempty"` - WorkspacePath string `json:"workspace_path,omitempty"` - ReportDir string `json:"report_dir,omitempty"` - JSON bool `json:"json"` + Mode string `json:"mode"` + SuitePath string `json:"suite_path"` + TaskID string `json:"task_id,omitempty"` + WorkspacePath string `json:"workspace_path,omitempty"` + WorkRoot string `json:"work_root,omitempty"` + AgentCommand []string `json:"agent_command,omitempty"` + Models []string `json:"models,omitempty"` + KeepWorkspaces bool `json:"keep_workspaces,omitempty"` + Timeout time.Duration `json:"timeout,omitempty"` + ReportDir string `json:"report_dir,omitempty"` + JSON bool `json:"json"` } +// agentEvalRuntimeError marks an eval failure that is a runtime/environment +// problem (e.g. failing to create a work root) rather than a usage error, so +// the command can exit with a crash code instead of the usage code. +type agentEvalRuntimeError struct{ err error } + +func (e agentEvalRuntimeError) Error() string { return e.err.Error() } + +func (e agentEvalRuntimeError) Unwrap() error { return e.err } + type agentEvalReport struct { - Suite string `json:"suite"` - Name string `json:"name,omitempty"` - TaskID string `json:"task_id,omitempty"` - Status string `json:"status,omitempty"` - OK bool `json:"ok"` - Tasks int `json:"tasks"` - Checks int `json:"checks"` - Total int `json:"total"` - Passed int `json:"passed"` - Failed int `json:"failed"` - Blocked int `json:"blocked"` - Errors int `json:"errors"` - ReportPath string `json:"report_path,omitempty"` - Failures []agentEvalFailure `json:"failures,omitempty"` + Suite string `json:"suite"` + Name string `json:"name,omitempty"` + TaskID string `json:"task_id,omitempty"` + Status string `json:"status,omitempty"` + OK bool `json:"ok"` + Tasks int `json:"tasks"` + Checks int `json:"checks"` + Total int `json:"total"` + Passed int `json:"passed"` + Failed int `json:"failed"` + Blocked int `json:"blocked"` + Errors int `json:"errors"` + Truncated bool `json:"truncated,omitempty"` + WorkRoot string `json:"work_root,omitempty"` + ReportPath string `json:"report_path,omitempty"` + Failures []agentEvalFailure `json:"failures,omitempty"` + Benchmark *agenteval.BenchmarkReport `json:"benchmark,omitempty"` } type agentEvalFailure struct { @@ -60,6 +79,10 @@ func runAgentEvalCommand(args []string, stdout io.Writer, stderr io.Writer, deps defer stop() report, err := deps.runAgentEval(ctx, options) if err != nil { + var runtimeErr agentEvalRuntimeError + if errors.As(err, &runtimeErr) { + return writeAppError(stderr, runtimeErr.Error(), exitCrash) + } return writeExecUsageError(stderr, err.Error()) } if options.ReportDir != "" { @@ -85,7 +108,7 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { options := agentEvalOptions{Mode: "validate"} if len(args) > 0 { switch args[0] { - case "run", "validate": + case "bench", "run", "validate": options.Mode = args[0] args = args[1:] } @@ -107,8 +130,8 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { case strings.HasPrefix(arg, "--suite="): options.SuitePath = strings.TrimSpace(strings.TrimPrefix(arg, "--suite=")) case arg == "--task": - if options.Mode != "run" { - return options, false, execUsageError{"--task is only valid for eval run"} + if options.Mode != "run" && options.Mode != "bench" { + return options, false, execUsageError{"--task is only valid for eval run or eval bench"} } value, next, err := nextFlagValue(args, index, arg) if err != nil { @@ -117,8 +140,8 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { options.TaskID = strings.TrimSpace(value) index = next case strings.HasPrefix(arg, "--task="): - if options.Mode != "run" { - return options, false, execUsageError{"--task is only valid for eval run"} + if options.Mode != "run" && options.Mode != "bench" { + return options, false, execUsageError{"--task is only valid for eval run or eval bench"} } value, err := requiredInlineFlagValue(arg, "--task") if err != nil { @@ -144,9 +167,116 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { return options, false, err } options.WorkspacePath = strings.TrimSpace(value) + case arg == "--work-root": + if options.Mode != "bench" { + return options, false, execUsageError{"--work-root is only valid for eval bench"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.WorkRoot = strings.TrimSpace(value) + index = next + case strings.HasPrefix(arg, "--work-root="): + if options.Mode != "bench" { + return options, false, execUsageError{"--work-root is only valid for eval bench"} + } + value, err := requiredInlineFlagValue(arg, "--work-root") + if err != nil { + return options, false, err + } + options.WorkRoot = strings.TrimSpace(value) + case arg == "--model": + if options.Mode != "bench" { + return options, false, execUsageError{"--model is only valid for eval bench"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.Models = appendAgentEvalModel(options.Models, value) + index = next + case strings.HasPrefix(arg, "--model="): + if options.Mode != "bench" { + return options, false, execUsageError{"--model is only valid for eval bench"} + } + value, err := requiredInlineFlagValue(arg, "--model") + if err != nil { + return options, false, err + } + options.Models = appendAgentEvalModel(options.Models, value) + case arg == "--models": + if options.Mode != "bench" { + return options, false, execUsageError{"--models is only valid for eval bench"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.Models = appendAgentEvalModels(options.Models, value) + index = next + case strings.HasPrefix(arg, "--models="): + if options.Mode != "bench" { + return options, false, execUsageError{"--models is only valid for eval bench"} + } + value, err := requiredInlineFlagValue(arg, "--models") + if err != nil { + return options, false, err + } + options.Models = appendAgentEvalModels(options.Models, value) + case arg == "--timeout": + if options.Mode != "bench" { + return options, false, execUsageError{"--timeout is only valid for eval bench"} + } + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + duration, err := parseEvalTimeout(value) + if err != nil { + return options, false, err + } + options.Timeout = duration + index = next + case strings.HasPrefix(arg, "--timeout="): + if options.Mode != "bench" { + return options, false, execUsageError{"--timeout is only valid for eval bench"} + } + value, err := requiredInlineFlagValue(arg, "--timeout") + if err != nil { + return options, false, err + } + duration, err := parseEvalTimeout(value) + if err != nil { + return options, false, err + } + options.Timeout = duration + case arg == "--agent-command": + if options.Mode != "bench" { + return options, false, execUsageError{"--agent-command is only valid for eval bench"} + } + if index+1 >= len(args) { + return options, false, execUsageError{"--agent-command requires at least one argument"} + } + options.AgentCommand = append([]string{}, args[index+1:]...) + index = len(args) + case strings.HasPrefix(arg, "--agent-command="): + if options.Mode != "bench" { + return options, false, execUsageError{"--agent-command is only valid for eval bench"} + } + value, err := requiredInlineFlagValue(arg, "--agent-command") + if err != nil { + return options, false, err + } + options.AgentCommand = []string{value} + case arg == "--keep-workspaces": + if options.Mode != "bench" { + return options, false, execUsageError{"--keep-workspaces is only valid for eval bench"} + } + options.KeepWorkspaces = true case arg == "--report-dir": - if options.Mode != "run" { - return options, false, execUsageError{"--report-dir is only valid for eval run"} + if options.Mode != "run" && options.Mode != "bench" { + return options, false, execUsageError{"--report-dir is only valid for eval run or eval bench"} } value, next, err := nextFlagValue(args, index, arg) if err != nil { @@ -155,8 +285,8 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { options.ReportDir = strings.TrimSpace(value) index = next case strings.HasPrefix(arg, "--report-dir="): - if options.Mode != "run" { - return options, false, execUsageError{"--report-dir is only valid for eval run"} + if options.Mode != "run" && options.Mode != "bench" { + return options, false, execUsageError{"--report-dir is only valid for eval run or eval bench"} } value, err := requiredInlineFlagValue(arg, "--report-dir") if err != nil { @@ -181,6 +311,38 @@ func parseAgentEvalArgs(args []string) (agentEvalOptions, bool, error) { return options, false, nil } +func appendAgentEvalModel(models []string, value string) []string { + value = strings.TrimSpace(value) + if value == "" { + return models + } + for _, model := range models { + if model == value { + return models + } + } + return append(models, value) +} + +func appendAgentEvalModels(models []string, value string) []string { + for _, model := range strings.Split(value, ",") { + models = appendAgentEvalModel(models, model) + } + return models +} + +func parseEvalTimeout(value string) (time.Duration, error) { + value = strings.TrimSpace(value) + duration, err := time.ParseDuration(value) + if err != nil { + return 0, execUsageError{fmt.Sprintf("--timeout must be a Go duration such as 90s or 5m: %v", err)} + } + if duration < 0 { + return 0, execUsageError{"--timeout must not be negative"} + } + return duration, nil +} + func formatAgentEvalReport(report agentEvalReport) string { lines := []string{ "Zero agent eval", @@ -192,7 +354,9 @@ func formatAgentEvalReport(report agentEvalReport) string { if report.TaskID != "" { lines = append(lines, "task: "+report.TaskID) } - if report.Tasks > 0 || report.Checks > 0 { + // Only validate mode reports a static check count; run and bench modes carry + // scored pass/fail/blocked/error tallies that must be surfaced instead. + if report.Checks > 0 { lines = append(lines, fmt.Sprintf("summary: %d tasks, %d checks", report.Tasks, report.Checks)) } else { lines = append(lines, fmt.Sprintf("summary: %d total, %d passed, %d failed, %d blocked, %d errors", report.Total, report.Passed, report.Failed, report.Blocked, report.Errors)) @@ -206,9 +370,15 @@ func formatAgentEvalReport(report agentEvalReport) string { } } lines = append(lines, "status: "+status) + if report.WorkRoot != "" { + lines = append(lines, "work-root: "+report.WorkRoot) + } if report.ReportPath != "" { lines = append(lines, "report: "+report.ReportPath) } + if report.Truncated { + lines = append(lines, "note: agent output was truncated") + } if len(report.Failures) > 0 { lines = append(lines, "failures:") } @@ -230,16 +400,25 @@ func writeAgentEvalHelp(w io.Writer) error { _, err := fmt.Fprint(w, `Usage: zero eval --suite [flags] zero eval run --suite [flags] + zero eval bench --suite [flags] [--model ] [--agent-command ] -Validates offline agent eval suites and runs local scoring when a runner is wired in. +Validates offline agent eval suites, scores an existing workspace, or benchmarks an agent command against fixture workspaces. Flags: - --suite Eval suite JSON file - --task Run one task (eval run only) - --workspace Workspace path for local scoring (eval run only, required) - --report-dir Write agent-eval-report.json (eval run only) - --json Print JSON output - -h, --help Show this help + --suite Eval suite JSON file + --task Run one task (eval run or eval bench) + --workspace Workspace path for local scoring (eval run only, required) + --work-root Work root for materialized benchmark workspaces (eval bench only) + --model Model id for eval bench model-matrix runs; repeatable + --models Comma-separated model ids for eval bench model-matrix runs + --timeout Per-task timeout for eval bench (Go duration, e.g. 90s, 5m) + --keep-workspaces Keep materialized benchmark workspaces (eval bench only) + --report-dir Write agent-eval-report.json (eval run or eval bench) + --agent-command Agent command argv for eval bench; may include {prompt}, + {workspace}, {task_id}, and {model}; must be last as it + consumes all remaining arguments + --json Print JSON output + -h, --help Show this help `) return err } @@ -256,11 +435,36 @@ func defaultRunAgentEval(ctx context.Context, options agentEvalOptions) (agentEv }) return agentEvalReportFromRunner(suite, report), nil } + if options.Mode == "bench" { + workRoot, cleanup, err := agentEvalBenchWorkRoot(options) + if err != nil { + return agentEvalReport{}, err + } + if cleanup != "" { + defer func() { _ = os.RemoveAll(cleanup) }() + } + harness := agenteval.Harness{} + if len(options.AgentCommand) > 0 { + harness.Agent = agenteval.CommandAgentRunner{Command: options.AgentCommand} + } + report := harness.Run(ctx, options.SuitePath, suite, agenteval.BenchmarkInput{ + TaskID: options.TaskID, + WorkRoot: workRoot, + Models: options.Models, + KeepWorkspaces: options.KeepWorkspaces, + Timeout: options.Timeout, + }) + converted := agentEvalReportFromBenchmark(suite, report) + if options.KeepWorkspaces { + // Surface where the kept workspaces live, especially when the work + // root was an unnamed temp dir the caller never specified. + converted.WorkRoot = workRoot + } + return converted, nil + } checks := 0 for _, task := range suite.Tasks { - // Every task has N verification commands plus one changed-file expectation - // check in the scoring contract. - checks += len(task.VerificationCommands) + 1 + checks += agentEvalTaskCheckCount(task) } return agentEvalReport{ Suite: suite.ID, @@ -272,6 +476,103 @@ func defaultRunAgentEval(ctx context.Context, options agentEvalOptions) (agentEv }, nil } +func agentEvalBenchWorkRoot(options agentEvalOptions) (string, string, error) { + workRoot := strings.TrimSpace(options.WorkRoot) + if workRoot != "" { + return workRoot, "", nil + } + created, err := os.MkdirTemp("", "zero-eval-") + if err != nil { + return "", "", agentEvalRuntimeError{fmt.Errorf("create benchmark work root: %w", err)} + } + if options.KeepWorkspaces { + return created, "", nil + } + return created, created, nil +} + +func agentEvalReportFromBenchmark(suite agenteval.Suite, report agenteval.BenchmarkReport) agentEvalReport { + converted := agentEvalReport{ + Suite: report.SuiteID, + Name: suite.Name, + Status: benchmarkStatus(report), + OK: report.OK, + Tasks: report.Summary.TotalTasks, + Total: report.Summary.TotalTasks, + Passed: report.Summary.PassedTasks, + Failed: report.Summary.FailedTasks, + Blocked: report.Summary.BlockedTasks, + Errors: report.Summary.ErrorTasks, + Benchmark: &report, + } + if converted.Suite == "" { + converted.Suite = suite.ID + } + if len(report.Tasks) == 1 { + converted.TaskID = report.Tasks[0].TaskID + } + for _, task := range report.Tasks { + if task.Agent.Truncated { + converted.Truncated = true + } + for _, failure := range agentEvalFailuresFromTaskReport(task) { + converted.Failures = append(converted.Failures, failure) + } + } + return converted +} + +func benchmarkStatus(report agenteval.BenchmarkReport) string { + switch { + case report.Summary.BlockedTasks > 0: + return string(agenteval.StatusBlocked) + case report.Summary.ErrorTasks > 0: + return string(agenteval.StatusError) + case report.Summary.FailedTasks > 0: + return string(agenteval.StatusFail) + default: + return string(agenteval.StatusPass) + } +} + +func agentEvalFailuresFromTaskReport(task agenteval.BenchmarkTaskReport) []agentEvalFailure { + failures := []agentEvalFailure{} + taskID := benchmarkFailureTaskID(task) + if task.Agent.Error != "" { + failures = append(failures, agentEvalFailure{ + ID: taskID, + Message: task.Agent.Error, + }) + } + for _, result := range task.Report.Results { + if result.Status == agenteval.StatusPass { + continue + } + id := taskID + if result.ID != "" { + id += "." + result.ID + } + failures = append(failures, agentEvalFailure{ + ID: id, + Message: agentEvalResultMessage(result), + }) + } + if task.Report.Error != "" { + failures = append(failures, agentEvalFailure{ + ID: taskID, + Message: task.Report.Error, + }) + } + return failures +} + +func benchmarkFailureTaskID(task agenteval.BenchmarkTaskReport) string { + if task.Model == "" { + return task.TaskID + } + return task.Model + "." + task.TaskID +} + func agentEvalReportFromRunner(suite agenteval.Suite, report agenteval.Report) agentEvalReport { converted := agentEvalReport{ Suite: report.SuiteID, @@ -326,3 +627,17 @@ func writeAgentEvalReportFile(reportDir string, report agentEvalReport) error { } return os.WriteFile(report.ReportPath, append(data, '\n'), 0o644) } + +func agentEvalTaskCheckCount(task agenteval.Task) int { + checks := len(task.VerificationCommands) + 1 + if len(task.ForbiddenChangedFiles) > 0 { + checks++ + } + if len(task.ContextChecks.RequiredFiles) > 0 || len(task.ContextChecks.ForbiddenFiles) > 0 { + checks++ + } + if len(task.RequiredTraceEvents) > 0 { + checks++ + } + return checks +} diff --git a/internal/cli/agent_eval_test.go b/internal/cli/agent_eval_test.go index 82e9c45aa..49ccf9eba 100644 --- a/internal/cli/agent_eval_test.go +++ b/internal/cli/agent_eval_test.go @@ -10,6 +10,9 @@ import ( "path/filepath" "strings" "testing" + "time" + + "github.com/Gitlawb/zero/internal/agenteval" ) func TestRunEvalHelpIsListed(t *testing.T) { @@ -140,6 +143,491 @@ func TestRunEvalRunJSONModePassesRunnerOptions(t *testing.T) { } } +func TestRunEvalBenchJSONModePassesHarnessOptions(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", "evals/context.json", + "--task", "edit-reader", + "--work-root", "D:\\tmp\\zero-evals", + "--json", + "--agent-command", "zero", "exec", "{prompt}", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Mode != "bench" || options.SuitePath != "evals/context.json" || options.TaskID != "edit-reader" || options.WorkRoot != "D:\\tmp\\zero-evals" || !options.JSON { + t.Fatalf("unexpected eval bench options: %#v", options) + } + if got, want := strings.Join(options.AgentCommand, "\x00"), strings.Join([]string{"zero", "exec", "{prompt}"}, "\x00"); got != want { + t.Fatalf("agent command = %#v, want zero exec {prompt}", options.AgentCommand) + } + return agentEvalReport{ + Suite: "quality-context", + TaskID: "edit-reader", + Status: "pass", + OK: true, + Total: 1, + Passed: 1, + }, nil + }, + }) + + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } + var decoded agentEvalReport + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("decode eval bench JSON: %v\n%s", err, stdout.String()) + } + if decoded.TaskID != "edit-reader" || decoded.Status != "pass" || decoded.Passed != 1 { + t.Fatalf("unexpected eval bench JSON: %#v", decoded) + } +} + +func TestRunEvalBenchModelFlagsDeduplicateAndPreserveOrder(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + wantModels := []string{"gpt-5", "o4-mini", "claude-sonnet-4.5", "o3"} + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", "evals/context.json", + "--model", " gpt-5 ", + "--models", "o4-mini,,gpt-5, claude-sonnet-4.5 ", + "--model=o3", + "--models=o4-mini", + "--json", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Mode != "bench" || options.SuitePath != "evals/context.json" || !options.JSON { + t.Fatalf("unexpected eval bench options: %#v", options) + } + if got, want := strings.Join(options.Models, "\x00"), strings.Join(wantModels, "\x00"); got != want { + t.Fatalf("models = %#v, want %#v", options.Models, wantModels) + } + return agentEvalReport{ + Suite: "quality-context", + Status: "pass", + OK: true, + Total: 1, + Passed: 1, + }, nil + }, + }) + + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } +} + +func TestRunEvalBenchReportDirAndKeepWorkspacesPassHarnessOptions(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + reportDir := t.TempDir() + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite=evals/context.json", + "--task=edit-reader", + "--keep-workspaces", + "--report-dir", reportDir, + }, &stdout, &stderr, appDeps{ + runAgentEval: func(ctx context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Mode != "bench" || !options.KeepWorkspaces || options.ReportDir != reportDir { + t.Fatalf("unexpected eval bench options: %#v", options) + } + if options.WorkRoot != "" { + t.Fatalf("default work root should remain empty at parse layer: %#v", options) + } + return agentEvalReport{ + Suite: "quality-context", + TaskID: "edit-reader", + Status: "blocked", + OK: false, + Total: 1, + Blocked: 1, + }, nil + }, + }) + + if exitCode != exitProvider { + t.Fatalf("expected provider-style failure exit %d, got %d", exitProvider, exitCode) + } + reportPath := filepath.Join(reportDir, "agent-eval-report.json") + if _, err := os.Stat(reportPath); err != nil { + t.Fatalf("expected report file: %v", err) + } + if !strings.Contains(stdout.String(), "report: "+reportPath) { + t.Fatalf("expected text output to mention report path, got:\n%s", stdout.String()) + } +} + +func TestRunEvalBenchDefaultHarnessBlocksWithoutAgentCommand(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + suitePath := filepath.Join("..", "agenteval", "testdata", "sample_suite.json") + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", suitePath, + "--task", "document-stream-json-verify-events", + "--json", + }, &stdout, &stderr, appDeps{}) + + if exitCode != exitProvider { + t.Fatalf("expected provider-style failure exit %d, got %d: %s", exitProvider, exitCode, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } + var decoded agentEvalReport + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("decode eval bench JSON: %v\n%s", err, stdout.String()) + } + if decoded.Status != "blocked" || decoded.Blocked != 1 { + t.Fatalf("expected blocked benchmark report, got %#v", decoded) + } + if len(decoded.Failures) == 0 || !strings.Contains(decoded.Failures[0].Message, "agent command is required") { + t.Fatalf("expected agent command failure, got %#v", decoded.Failures) + } +} + +func TestRunEvalBenchDefaultHarnessRunsModelMatrix(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + suitePath := filepath.Join("..", "agenteval", "testdata", "sample_suite.json") + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", suitePath, + "--task", "document-stream-json-verify-events", + "--models", "model-a,model-b", + "--json", + }, &stdout, &stderr, appDeps{}) + + if exitCode != exitProvider { + t.Fatalf("expected provider-style failure exit %d, got %d: %s", exitProvider, exitCode, stderr.String()) + } + var decoded agentEvalReport + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("decode eval bench JSON: %v\n%s", err, stdout.String()) + } + if decoded.Total != 2 || decoded.Blocked != 2 { + t.Fatalf("expected two blocked model runs, got %#v", decoded) + } + if decoded.Benchmark == nil || len(decoded.Benchmark.Tasks) != 2 { + t.Fatalf("expected nested benchmark detail, got %#v", decoded.Benchmark) + } + if decoded.Benchmark.Tasks[0].Model != "model-a" || decoded.Benchmark.Tasks[1].Model != "model-b" { + t.Fatalf("benchmark model order = %#v", decoded.Benchmark.Tasks) + } + if len(decoded.Failures) < 2 || + !strings.HasPrefix(decoded.Failures[0].ID, "model-a.document-stream-json-verify-events") || + !strings.HasPrefix(decoded.Failures[len(decoded.Failures)-1].ID, "model-b.document-stream-json-verify-events") { + t.Fatalf("model-prefixed failures = %#v", decoded.Failures) + } +} + +func TestRunEvalBenchTextOutputShowsScores(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", "evals/context.json", + "--agent-command", "zero", "exec", "{prompt}", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(_ context.Context, _ agentEvalOptions) (agentEvalReport, error) { + // agentEvalReportFromBenchmark populates both Tasks and Total; the + // text formatter must still surface the scored tallies. + return agentEvalReport{ + Suite: "quality-context", + Status: "fail", + OK: false, + Tasks: 2, + Total: 2, + Passed: 1, + Failed: 1, + }, nil + }, + }) + + if exitCode != exitProvider { + t.Fatalf("expected provider-style failure exit %d, got %d: %s", exitProvider, exitCode, stderr.String()) + } + out := stdout.String() + if !strings.Contains(out, "summary: 2 total, 1 passed, 1 failed, 0 blocked, 0 errors") { + t.Fatalf("expected scored summary in bench text output, got:\n%s", out) + } + if strings.Contains(out, "tasks, 0 checks") { + t.Fatalf("bench text output must not hide scores behind the validate-style summary:\n%s", out) + } +} + +func TestRunEvalBenchTextOutputSurfacesWorkRoot(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", "evals/context.json", + "--keep-workspaces", + "--agent-command", "zero", "exec", "{prompt}", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(_ context.Context, options agentEvalOptions) (agentEvalReport, error) { + if !options.KeepWorkspaces { + t.Fatalf("expected keep-workspaces option: %#v", options) + } + return agentEvalReport{ + Suite: "quality-context", + Status: "pass", + OK: true, + Total: 1, + Passed: 1, + WorkRoot: "/tmp/zero-eval-abc", + }, nil + }, + }) + + if exitCode != exitSuccess { + t.Fatalf("expected success exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "work-root: /tmp/zero-eval-abc") { + t.Fatalf("expected kept work root in text output, got:\n%s", stdout.String()) + } +} + +func TestRunEvalBenchKeepWorkspacesSurfacesRealWorkRoot(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + suitePath := filepath.Join("..", "agenteval", "testdata", "sample_suite.json") + workRoot := t.TempDir() + + // Drive the real defaultRunAgentEval (appDeps{}) so the keep-workspaces + // WorkRoot wiring is exercised, not just the formatter. + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", suitePath, + "--task", "document-stream-json-verify-events", + "--work-root", workRoot, + "--keep-workspaces", + }, &stdout, &stderr, appDeps{}) + + if exitCode != exitProvider { + t.Fatalf("expected provider-style exit %d, got %d: %s", exitProvider, exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "work-root: "+workRoot) { + t.Fatalf("expected real work root surfaced in text output, got:\n%s", stdout.String()) + } +} + +func TestAgentEvalReportFromBenchmarkSurfacesTruncation(t *testing.T) { + report := agenteval.BenchmarkReport{ + SuiteID: "s", + OK: true, + Summary: agenteval.BenchmarkSummary{TotalTasks: 1, PassedTasks: 1}, + Tasks: []agenteval.BenchmarkTaskReport{{ + TaskID: "t1", + Agent: agenteval.AgentRunResult{ExitCode: 0, Truncated: true}, + Report: agenteval.Report{Status: agenteval.StatusPass, OK: true}, + }}, + } + + converted := agentEvalReportFromBenchmark(agenteval.Suite{ID: "s", Name: "S"}, report) + + if !converted.Truncated { + t.Fatal("expected Truncated to be surfaced from the task agent result") + } + if text := formatAgentEvalReport(converted); !strings.Contains(text, "note: agent output was truncated") { + t.Fatalf("expected truncation note in text output:\n%s", text) + } +} + +func TestRunEvalBenchParsesTimeout(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{ + "eval", "bench", + "--suite", "evals/context.json", + "--timeout", "90s", + "--json", + "--agent-command", "zero", "exec", "{prompt}", + }, &stdout, &stderr, appDeps{ + runAgentEval: func(_ context.Context, options agentEvalOptions) (agentEvalReport, error) { + if options.Timeout != 90*time.Second { + t.Fatalf("Timeout = %s, want 90s", options.Timeout) + } + return agentEvalReport{Suite: "quality-context", OK: true, Total: 1, Passed: 1}, nil + }, + }) + + if exitCode != exitSuccess { + t.Fatalf("expected success exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } +} + +func TestRunEvalTimeoutValidation(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + { + name: "run mode rejected", + args: []string{"eval", "run", "--suite", "evals/context.json", "--timeout", "5s"}, + want: "--timeout is only valid for eval bench", + }, + { + name: "invalid duration", + args: []string{"eval", "bench", "--suite", "evals/context.json", "--timeout=soon"}, + want: "--timeout must be a Go duration", + }, + { + name: "negative duration", + args: []string{"eval", "bench", "--suite", "evals/context.json", "--timeout=-5s"}, + want: "--timeout must not be negative", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps(tt.args, &stdout, &stderr, appDeps{ + runAgentEval: func(context.Context, agentEvalOptions) (agentEvalReport, error) { + t.Fatal("runAgentEval should not be called for invalid --timeout") + return agentEvalReport{}, nil + }, + }) + + if exitCode != exitUsage { + t.Fatalf("expected usage exit %d, got %d", exitUsage, exitCode) + } + if !strings.Contains(stderr.String(), tt.want) { + t.Fatalf("expected %q, got %q", tt.want, stderr.String()) + } + }) + } +} + +func TestRunEvalRuntimeErrorReturnsCrashExit(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{"eval", "bench", "--suite", "evals/context.json"}, &stdout, &stderr, appDeps{ + runAgentEval: func(context.Context, agentEvalOptions) (agentEvalReport, error) { + return agentEvalReport{}, agentEvalRuntimeError{errors.New("create benchmark work root: disk full")} + }, + }) + + if exitCode != exitCrash { + t.Fatalf("expected crash exit %d, got %d", exitCrash, exitCode) + } + if !strings.Contains(stderr.String(), "disk full") { + t.Fatalf("expected runtime error message in stderr, got %q", stderr.String()) + } +} + +func TestRunEvalBenchRejectsRunOnlyWorkspaceFlag(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{"eval", "bench", "--suite", "evals/context.json", "--workspace", "."}, &stdout, &stderr, appDeps{ + runAgentEval: func(context.Context, agentEvalOptions) (agentEvalReport, error) { + t.Fatal("runAgentEval should not be called for invalid bench flags") + return agentEvalReport{}, nil + }, + }) + + if exitCode != exitUsage { + t.Fatalf("expected usage exit %d, got %d", exitUsage, exitCode) + } + if !strings.Contains(stderr.String(), "--workspace is only valid for eval run") { + t.Fatalf("expected workspace mode error, got %q", stderr.String()) + } +} + +func TestRunEvalRunRejectsBenchOnlyFlags(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + { + name: "work root", + args: []string{"eval", "run", "--suite", "evals/context.json", "--work-root", "D:\\tmp\\zero-evals"}, + want: "--work-root is only valid for eval bench", + }, + { + name: "keep workspaces", + args: []string{"eval", "run", "--suite", "evals/context.json", "--keep-workspaces"}, + want: "--keep-workspaces is only valid for eval bench", + }, + { + name: "agent command", + args: []string{"eval", "run", "--suite", "evals/context.json", "--agent-command", "zero", "exec", "{prompt}"}, + want: "--agent-command is only valid for eval bench", + }, + { + name: "model", + args: []string{"eval", "run", "--suite", "evals/context.json", "--model", "gpt-5"}, + want: "--model is only valid for eval bench", + }, + { + name: "models", + args: []string{"eval", "run", "--suite", "evals/context.json", "--models=gpt-5,o4-mini"}, + want: "--models is only valid for eval bench", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps(tt.args, &stdout, &stderr, appDeps{ + runAgentEval: func(context.Context, agentEvalOptions) (agentEvalReport, error) { + t.Fatal("runAgentEval should not be called for invalid run flags") + return agentEvalReport{}, nil + }, + }) + + if exitCode != exitUsage { + t.Fatalf("expected usage exit %d, got %d", exitUsage, exitCode) + } + if !strings.Contains(stderr.String(), tt.want) { + t.Fatalf("expected %q, got %q", tt.want, stderr.String()) + } + }) + } +} + +func TestRunEvalHelpMentionsBenchModelFlags(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + exitCode := runWithDeps([]string{"eval", "bench", "--help"}, &stdout, &stderr, appDeps{}) + + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } + for _, want := range []string{"--model ", "--models ", "{model}"} { + if !strings.Contains(stdout.String(), want) { + t.Fatalf("expected help to contain %q, got:\n%s", want, stdout.String()) + } + } +} + func TestRunEvalRunRequiresExplicitWorkspace(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer From d2334a2057f65df9a990499d6d5dce0425e8d054 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sat, 13 Jun 2026 11:23:23 +0530 Subject: [PATCH 5/5] Fix agent eval review findings --- internal/agenteval/agent_command.go | 4 +++ internal/agenteval/agent_command_test.go | 24 +++++++++++++-- internal/agenteval/benchmark.go | 8 ++++- internal/agenteval/benchmark_test.go | 9 ++++++ internal/agenteval/materialize.go | 39 ++++++++++++++++++++++-- internal/agenteval/materialize_test.go | 28 +++++++++++++++++ internal/cli/agent_eval.go | 24 +++++++-------- internal/cli/agent_eval_test.go | 25 +++++++++++++++ 8 files changed, 143 insertions(+), 18 deletions(-) diff --git a/internal/agenteval/agent_command.go b/internal/agenteval/agent_command.go index 7eae6a490..88f3f8757 100644 --- a/internal/agenteval/agent_command.go +++ b/internal/agenteval/agent_command.go @@ -52,6 +52,10 @@ func (runner CommandAgentRunner) Run(ctx context.Context, input AgentRunInput) A result.Error = "agent command is required" return result } + if strings.TrimSpace(input.WorkspacePath) == "" { + result.Error = "workspace path is required" + return result + } if ctx == nil { ctx = context.Background() } diff --git a/internal/agenteval/agent_command_test.go b/internal/agenteval/agent_command_test.go index 714ff1736..313c30670 100644 --- a/internal/agenteval/agent_command_test.go +++ b/internal/agenteval/agent_command_test.go @@ -137,6 +137,18 @@ func TestCommandAgentRunnerEmptyCommandReturnsError(t *testing.T) { } } +func TestCommandAgentRunnerRequiresWorkspacePath(t *testing.T) { + result := (CommandAgentRunner{Command: helperCommand("record", filepath.Join(t.TempDir(), "out.txt"))}). + Run(context.Background(), AgentRunInput{}) + + if result.ExitCode != -1 { + t.Fatalf("ExitCode = %d, want -1", result.ExitCode) + } + if result.Error != "workspace path is required" { + t.Fatalf("Error = %q", result.Error) + } +} + func TestCommandAgentRunnerReportsContextErrors(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -209,8 +221,12 @@ func TestCommandAgentRunnerHelperProcess(t *testing.T) { os.Exit(2) } case "exit": - os.Stdout.WriteString("agent stdout\n") - os.Stderr.WriteString("agent stderr\n") + if _, err := os.Stdout.WriteString("agent stdout\n"); err != nil { + os.Exit(2) + } + if _, err := os.Stderr.WriteString("agent stderr\n"); err != nil { + os.Exit(2) + } os.Exit(7) case "spew": if len(args) < 3 { @@ -220,7 +236,9 @@ func TestCommandAgentRunnerHelperProcess(t *testing.T) { if err != nil { os.Exit(2) } - os.Stdout.Write([]byte(strings.Repeat("a", count))) + if _, err := os.Stdout.Write([]byte(strings.Repeat("a", count))); err != nil { + os.Exit(2) + } case "sleep": time.Sleep(30 * time.Second) default: diff --git a/internal/agenteval/benchmark.go b/internal/agenteval/benchmark.go index 0bf3e0a7b..f2d6aff18 100644 --- a/internal/agenteval/benchmark.go +++ b/internal/agenteval/benchmark.go @@ -62,6 +62,7 @@ func (harness Harness) Run(ctx context.Context, suitePath string, suite Suite, i taskID := input.TaskID report.Tasks = append(report.Tasks, BenchmarkTaskReport{ TaskID: taskID, + Agent: AgentRunResult{ExitCode: -1, Error: err.Error()}, Report: Report{ Contract: ReportContractVersion, SuiteID: suite.ID, @@ -98,7 +99,11 @@ func (harness Harness) runTask(ctx context.Context, suitePath string, suite Suit ctx, cancel = context.WithTimeout(ctx, input.Timeout) defer cancel() } - taskReport := BenchmarkTaskReport{TaskID: task.ID, Model: model} + taskReport := BenchmarkTaskReport{ + TaskID: task.ID, + Model: model, + Agent: AgentRunResult{ExitCode: -1}, + } if harness.Agent == nil { taskReport.Agent = AgentRunResult{ExitCode: -1, Error: "agent command is required"} taskReport.Report = Score(suite, ScoreInput{ @@ -113,6 +118,7 @@ func (harness Harness) runTask(ctx context.Context, suitePath string, suite Suit WorkRoot: input.WorkRoot, }) if err != nil { + taskReport.Agent.Error = err.Error() taskReport.Report = errorReport(suite.ID, task.ID, fmt.Sprintf("workspace materialization failed: %v", err)) return taskReport } diff --git a/internal/agenteval/benchmark_test.go b/internal/agenteval/benchmark_test.go index 39032a412..ea110fc47 100644 --- a/internal/agenteval/benchmark_test.go +++ b/internal/agenteval/benchmark_test.go @@ -263,6 +263,9 @@ func TestHarnessBlocksSelectedTasksWhenAgentIsNil(t *testing.T) { if report.Tasks[0].Agent.Error != "agent command is required" { t.Fatalf("Agent.Error = %q", report.Tasks[0].Agent.Error) } + if report.Tasks[0].Agent.ExitCode != -1 { + t.Fatalf("Agent.ExitCode = %d, want -1", report.Tasks[0].Agent.ExitCode) + } if report.Tasks[0].Report.Status != StatusBlocked { t.Fatalf("Report.Status = %q", report.Tasks[0].Report.Status) } @@ -335,6 +338,9 @@ func TestHarnessReportsErrorForUnknownTaskID(t *testing.T) { if report.Tasks[0].Report.Status != StatusError { t.Fatalf("Report.Status = %q", report.Tasks[0].Report.Status) } + if report.Tasks[0].Agent.ExitCode != -1 || !strings.Contains(report.Tasks[0].Agent.Error, "no-such-task") { + t.Fatalf("Agent should record non-run selection error, got %#v", report.Tasks[0].Agent) + } } func TestHarnessReportsErrorWhenMaterializationFails(t *testing.T) { @@ -379,6 +385,9 @@ func TestHarnessReportsErrorWhenMaterializationFails(t *testing.T) { if !strings.Contains(report.Tasks[0].Report.Error, "materialization failed") { t.Fatalf("Report.Error = %q", report.Tasks[0].Report.Error) } + if report.Tasks[0].Agent.ExitCode != -1 || report.Tasks[0].Agent.Error == "" { + t.Fatalf("Agent should record non-run materialization error, got %#v", report.Tasks[0].Agent) + } if agentCalled { t.Fatal("agent should not run when materialization fails") } diff --git a/internal/agenteval/materialize.go b/internal/agenteval/materialize.go index 0750c30c0..71e683dfc 100644 --- a/internal/agenteval/materialize.go +++ b/internal/agenteval/materialize.go @@ -73,18 +73,53 @@ func resolveFixturePath(suitePath string, fixture string) (string, error) { if err != nil { return "", err } + canonicalBase, err := filepath.EvalSymlinks(base) + if err != nil { + return "", fmt.Errorf("resolve suite directory: %w", err) + } resolved, err := filepath.Abs(filepath.Join(base, filepath.FromSlash(fixture))) if err != nil { return "", err } - rel, err := filepath.Rel(base, resolved) + canonicalResolved, err := evalSymlinkPath(resolved) + if err != nil { + return "", err + } + rel, err := filepath.Rel(canonicalBase, canonicalResolved) if err != nil { return "", err } if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { return "", errors.New("workspace fixture must stay within suite directory") } - return resolved, nil + return canonicalResolved, nil +} + +func evalSymlinkPath(path string) (string, error) { + path, err := filepath.Abs(path) + if err != nil { + return "", err + } + current := filepath.Clean(path) + missing := []string{} + for { + resolved, err := filepath.EvalSymlinks(current) + if err == nil { + for i := len(missing) - 1; i >= 0; i-- { + resolved = filepath.Join(resolved, missing[i]) + } + return filepath.Clean(resolved), nil + } + if !os.IsNotExist(err) { + return "", err + } + parent := filepath.Dir(current) + if parent == current { + return "", err + } + missing = append(missing, filepath.Base(current)) + current = parent + } } func copyFixtureDir(src string, dst string) error { diff --git a/internal/agenteval/materialize_test.go b/internal/agenteval/materialize_test.go index b9797d455..80076d1c8 100644 --- a/internal/agenteval/materialize_test.go +++ b/internal/agenteval/materialize_test.go @@ -79,6 +79,34 @@ func TestMaterializeTaskRejectsSymlinkFixtureEntryAndCleansUp(t *testing.T) { } } +func TestMaterializeTaskRejectsSymlinkFixtureRootEscape(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires privileges on windows") + } + root := t.TempDir() + suitePath := filepath.Join(root, "suite.json") + fixturesDir := filepath.Join(root, "fixtures") + if err := os.MkdirAll(fixturesDir, 0o755); err != nil { + t.Fatal(err) + } + outside := t.TempDir() + if err := os.WriteFile(filepath.Join(outside, "go.mod"), []byte("module outside\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(fixturesDir, "escape")); err != nil { + t.Skipf("symlink unsupported on this platform: %v", err) + } + task := Task{ID: "escape-task", WorkspaceFixture: "fixtures/escape"} + + _, err := Materializer{}.MaterializeTask(context.Background(), suitePath, task, MaterializeInput{WorkRoot: t.TempDir()}) + if err == nil { + t.Fatal("MaterializeTask returned nil error for a symlink-escaped fixture root") + } + if !strings.Contains(strings.ToLower(err.Error()), "within") { + t.Fatalf("error %q does not mention containment", err) + } +} + func TestMaterializeTaskPreservesExecutableBit(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("unix file mode bits are not preserved on windows") diff --git a/internal/cli/agent_eval.go b/internal/cli/agent_eval.go index f6aabb8a1..e8cb80fef 100644 --- a/internal/cli/agent_eval.go +++ b/internal/cli/agent_eval.go @@ -537,12 +537,18 @@ func benchmarkStatus(report agenteval.BenchmarkReport) string { func agentEvalFailuresFromTaskReport(task agenteval.BenchmarkTaskReport) []agentEvalFailure { failures := []agentEvalFailure{} + seen := map[string]struct{}{} + appendUnique := func(id string, message string) { + key := strings.TrimSpace(id) + "\x00" + strings.TrimSpace(message) + if _, ok := seen[key]; ok { + return + } + seen[key] = struct{}{} + failures = append(failures, agentEvalFailure{ID: id, Message: message}) + } taskID := benchmarkFailureTaskID(task) if task.Agent.Error != "" { - failures = append(failures, agentEvalFailure{ - ID: taskID, - Message: task.Agent.Error, - }) + appendUnique(taskID, task.Agent.Error) } for _, result := range task.Report.Results { if result.Status == agenteval.StatusPass { @@ -552,16 +558,10 @@ func agentEvalFailuresFromTaskReport(task agenteval.BenchmarkTaskReport) []agent if result.ID != "" { id += "." + result.ID } - failures = append(failures, agentEvalFailure{ - ID: id, - Message: agentEvalResultMessage(result), - }) + appendUnique(id, agentEvalResultMessage(result)) } if task.Report.Error != "" { - failures = append(failures, agentEvalFailure{ - ID: taskID, - Message: task.Report.Error, - }) + appendUnique(taskID, task.Report.Error) } return failures } diff --git a/internal/cli/agent_eval_test.go b/internal/cli/agent_eval_test.go index 49ccf9eba..0a6baef42 100644 --- a/internal/cli/agent_eval_test.go +++ b/internal/cli/agent_eval_test.go @@ -335,6 +335,31 @@ func TestRunEvalBenchDefaultHarnessRunsModelMatrix(t *testing.T) { } } +func TestAgentEvalFailuresFromTaskReportDeduplicatesTaskLevelFailures(t *testing.T) { + failures := agentEvalFailuresFromTaskReport(agenteval.BenchmarkTaskReport{ + TaskID: "task-a", + Model: "model-a", + Agent: agenteval.AgentRunResult{Error: "boom"}, + Report: agenteval.Report{ + Error: "boom", + Results: []agenteval.Result{ + {Status: agenteval.StatusError, Message: "boom"}, + {ID: "verify", Status: agenteval.StatusFail, Message: "boom"}, + }, + }, + }) + + if len(failures) != 2 { + t.Fatalf("failures = %#v, want two unique entries", failures) + } + if failures[0] != (agentEvalFailure{ID: "model-a.task-a", Message: "boom"}) { + t.Fatalf("first failure = %#v", failures[0]) + } + if failures[1] != (agentEvalFailure{ID: "model-a.task-a.verify", Message: "boom"}) { + t.Fatalf("second failure = %#v", failures[1]) + } +} + func TestRunEvalBenchTextOutputShowsScores(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer