From 3805fc3e95f5307082185454cdbe99b98e758fcc Mon Sep 17 00:00:00 2001 From: melmennaoui Date: Tue, 1 Sep 2026 17:44:23 +0200 Subject: [PATCH] chore: remove verify script dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the verify field from EvalCriteria and all supporting code. The verify concept (running a shell script via docker exec on the eval container after the agent completes) cannot work without container lifecycle changes: the container uses --rm and is deleted before verify could run. The assertion system already covers verifiable cases — response text, tool calls, cost — via the captured event stream. Rather than ship dead code that creates user confusion (the schema accepts it, nothing runs it), remove it entirely. The schema's DisallowUnknownFields rejection ensures a future verify field can be added cleanly when a real need is proven. Removed: - EvalCriteria.Verify field - VerifyCheck type and EvalResultChecks.Verify field - cloneEvalResultChecks verify branch - verify.go and verify_test.go - verify test case in session_test.go --- pkg/evaluation/verify.go | 79 ----------------------------------- pkg/evaluation/verify_test.go | 78 ---------------------------------- pkg/session/branch.go | 4 -- pkg/session/session.go | 9 ---- pkg/session/session_test.go | 8 ---- 5 files changed, 178 deletions(-) delete mode 100644 pkg/evaluation/verify.go delete mode 100644 pkg/evaluation/verify_test.go diff --git a/pkg/evaluation/verify.go b/pkg/evaluation/verify.go deleted file mode 100644 index 3bab27be7..000000000 --- a/pkg/evaluation/verify.go +++ /dev/null @@ -1,79 +0,0 @@ -package evaluation - -import ( - "bytes" - "context" - "errors" - "fmt" - "log/slog" - "os/exec" - "strings" - "time" -) - -// verifyResult holds the outcome of running a verify script inside the -// evaluation container. -type verifyResult struct { - Passed bool - ExitCode int - Output string -} - -// runVerifyScript executes a shell verify script and returns its outcome. -// The script is run with sh -c; a zero exit code means pass. Output is -// capped at maxVerifyOutputBytes to avoid unbounded memory from chatty -// scripts. -func runVerifyScript(ctx context.Context, script, containerRuntime, containerName string) (verifyResult, error) { - if script == "" { - return verifyResult{Passed: true}, nil - } - - ctx, cancel := context.WithTimeout(ctx, 60*time.Second) - defer cancel() - - args := []string{"exec", containerName, "sh", "-c", script} - cmd := exec.CommandContext(ctx, containerRuntime, args...) - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - err := cmd.Run() - - output := strings.TrimSpace(stdout.String()) - if errOutput := strings.TrimSpace(stderr.String()); errOutput != "" { - if output != "" { - output += "\n" - } - output += errOutput - } - - // Cap output to avoid unbounded memory. - if len(output) > maxVerifyOutputBytes { - output = output[:maxVerifyOutputBytes] + "...(truncated)" - } - - exitCode := 0 - if err != nil { - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - exitCode = exitErr.ExitCode() - } else { - return verifyResult{ExitCode: -1, Output: output}, fmt.Errorf("running verify script: %w", err) - } - } - - slog.DebugContext(ctx, "Verify script completed", - "exit_code", exitCode, - "output_length", len(output), - ) - - return verifyResult{ - Passed: exitCode == 0, - ExitCode: exitCode, - Output: output, - }, nil -} - -// maxVerifyOutputBytes caps the output captured from a verify script. -const maxVerifyOutputBytes = 8192 diff --git a/pkg/evaluation/verify_test.go b/pkg/evaluation/verify_test.go deleted file mode 100644 index 0c5b6c355..000000000 --- a/pkg/evaluation/verify_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package evaluation - -import ( - "os" - "path/filepath" - "runtime" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestRunVerifyScript_EmptyScriptPasses(t *testing.T) { - t.Parallel() - result, err := runVerifyScript(t.Context(), "", "docker", "c") - require.NoError(t, err) - assert.True(t, result.Passed) -} - -func TestRunVerifyScript_ZeroExitCodePasses(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("POSIX shell required") - } - t.Parallel() - - // Use a fake "container runtime" script that execs the verify command - // directly (no real container needed). - fake := writeFakeExecRuntime(t, "echo 'all good'; exit 0") - - result, err := runVerifyScript(t.Context(), "verify", fake, "container-1") - require.NoError(t, err) - assert.True(t, result.Passed) - assert.Equal(t, 0, result.ExitCode) - assert.Contains(t, result.Output, "all good") -} - -func TestRunVerifyScript_NonZeroExitCodeFails(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("POSIX shell required") - } - t.Parallel() - - fake := writeFakeExecRuntime(t, "echo 'file missing'; exit 1") - - result, err := runVerifyScript(t.Context(), "verify", fake, "container-1") - require.NoError(t, err) - assert.False(t, result.Passed) - assert.Equal(t, 1, result.ExitCode) - assert.Contains(t, result.Output, "file missing") -} - -func TestRunVerifyScript_OutputCapped(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("POSIX shell required") - } - t.Parallel() - - // Generate output larger than the cap. - fake := writeFakeExecRuntime(t, "dd if=/dev/zero bs=1 count=20000 2>/dev/null | tr '\\0' 'x'; exit 0") - - result, err := runVerifyScript(t.Context(), "verify", fake, "container-1") - require.NoError(t, err) - assert.True(t, result.Passed) - assert.LessOrEqual(t, len(result.Output), maxVerifyOutputBytes+20) // +20 for truncation marker -} - -// writeFakeExecRuntime creates a shell script that ignores docker exec args -// and runs the given shell command instead, returning the script path. -func writeFakeExecRuntime(t *testing.T, shCmd string) string { - t.Helper() - dir := t.TempDir() - path := filepath.Join(dir, "fake-runtime") - // The script ignores its arguments (exec container sh -c ...) and runs - // shCmd directly, simulating what the container would do. - script := "#!/bin/sh\n" + shCmd + "\n" - require.NoError(t, os.WriteFile(path, []byte(script), 0o755)) - return path -} diff --git a/pkg/session/branch.go b/pkg/session/branch.go index 3037e6d62..16faaade2 100644 --- a/pkg/session/branch.go +++ b/pkg/session/branch.go @@ -367,10 +367,6 @@ func cloneEvalResultChecks(src EvalResultChecks) EvalResultChecks { assertions.Results = slices.Clone(src.Assertions.Results) cp.Assertions = &assertions } - if src.Verify != nil { - verify := *src.Verify - cp.Verify = &verify - } return cp } diff --git a/pkg/session/session.go b/pkg/session/session.go index 953b836d2..6ca408973 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -642,7 +642,6 @@ type EvalResultChecks struct { ToolCalls *ToolCallsCheck `json:"tool_calls,omitempty"` Relevance *RelevanceCheck `json:"relevance,omitempty"` Assertions *AssertionsCheck `json:"assertions,omitempty"` - Verify *VerifyCheck `json:"verify,omitempty"` } // SizeCheck contains the result of the response size check. @@ -689,18 +688,10 @@ type AssertionResult struct { Reason string `json:"reason,omitempty"` } -// VerifyCheck contains the result of the post-agent verify script. -type VerifyCheck struct { - Passed bool `json:"passed"` - ExitCode int `json:"exit_code"` - Output string `json:"output,omitempty"` -} - // EvalCriteria contains the evaluation criteria for a session. type EvalCriteria struct { Relevance []string `json:"relevance"` // Statements that should be true about the response Assertions []Assertion `json:"assertions,omitempty"` // Code-based assertions evaluated against the agent output - Verify string `json:"verify,omitempty"` // Shell script for post-agent outcome verification WorkingDir string `json:"working_dir,omitempty"` // Subdirectory under evals/working_dirs/ Size string `json:"size,omitempty"` // Expected response size: S, M, L, XL Setup string `json:"setup,omitempty"` // Optional sh script to run in the container before docker agent run --exec diff --git a/pkg/session/session_test.go b/pkg/session/session_test.go index e45045ef6..a1a8b5514 100644 --- a/pkg/session/session_test.go +++ b/pkg/session/session_test.go @@ -520,14 +520,6 @@ func TestEvalCriteriaUnmarshalJSON(t *testing.T) { Assertions: []Assertion{{Name: "has greeting", Type: "contains", Value: "hello"}}, }, }, - { - name: "valid with verify", - input: `{"relevance":[],"verify":"test -f output.txt"}`, - want: EvalCriteria{ - Relevance: []string{}, - Verify: "test -f output.txt", - }, - }, { name: "empty object", input: `{}`,