fix grader sandbox clone fallback and parser edge cases

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
pelikhan · Aug 22, 2026 · Aug 22, 2026 · Aug 22, 2026 · Aug 22, 2026 · Aug 22, 2026
commit 455639fcfac213cfa0965a2ede15fad7edcc8a29
diff --git a/actions/setup/js/trace_graders.cjs b/actions/setup/js/trace_graders.cjs
@@ -320,19 +320,27 @@ const BUILTIN_META = {
   "artifact-production": { unit: "count", direction: "higher_is_better" },
 };
 
+/**
+ * @param {any} toolCall
+ * @returns {boolean}
+ */
+function isToolFailure(toolCall) {
+  return toolCall.success === false || toolCall.status === "error" || toolCall.status === "failure" || toolCall.error !== undefined;
+}
+
 /**
  * @param {PreprocessedTrace} trace
  * @returns {number} Success rate of tool calls (0-1), or 1 if no tool calls
  */
 function gradeToolSuccessRate(trace) {
   if (trace.toolCalls.length === 0) return 1;
-  const successes = trace.toolCalls.filter(t => t.success === true || (t.success !== false && t.status !== "error" && t.status !== "failure" && t.error === undefined)).length;
+  const successes = trace.toolCalls.length - trace.toolCalls.filter(isToolFailure).length;
   return successes / trace.toolCalls.length;
 }
 
 /** @param {PreprocessedTrace} trace @returns {number} */
 function gradeToolFailureCount(trace) {
-  return trace.toolCalls.filter(t => t.success === false || t.status === "error" || t.status === "failure" || t.error !== undefined).length;
+  return trace.toolCalls.filter(isToolFailure).length;
 }
 
 /** @param {PreprocessedTrace} trace @returns {number} */

diff --git a/actions/setup/js/trace_graders.test.cjs b/actions/setup/js/trace_graders.test.cjs
@@ -198,6 +198,15 @@ describe("trace_graders", () => {
       });
       expect(gradeToolFailureCount(trace)).toBe(2);
     });
+
+    it("matches success-rate complement for ambiguous calls", () => {
+      const trace = makeTrace({
+        toolCalls: [{ name: "a", success: true }, { name: "b", status: "error" }, { name: "c" }],
+      });
+      const failures = gradeToolFailureCount(trace);
+      const successRate = gradeToolSuccessRate(trace);
+      expect(successRate).toBeCloseTo((trace.toolCalls.length - failures) / trace.toolCalls.length);
+    });
   });
 
   describe("gradeRetries", () => {
@@ -491,7 +500,7 @@ describe("trace_graders", () => {
     });
 
     it("handles nested malicious JSON in JSONL", () => {
-      const hostile = '{"a": 1, "__proto__": {"polluted": true}}';
+      const hostile = '{"a":1,"constructor":{"prototype":{"polluted":true}}}';
       const result = safeParseJsonl(hostile);
       expect(result.length).toBe(1);
       expect(result[0].a).toBe(1);

diff --git a/actions/setup/js/trace_graders_worker.cjs b/actions/setup/js/trace_graders_worker.cjs
@@ -3,25 +3,17 @@
 const vm = require("vm");
 
 /**
- * @param {any} obj
+ * @param {any} value
+ * @param {string} label
  * @returns {any}
  */
-function deepClone(obj) {
-  if (obj === null || obj === undefined) return obj;
+function tryStructuredClone(value, label) {
+  if (value === null || value === undefined) return value;
   try {
-    return structuredClone(obj);
+    return structuredClone(value);
   } catch {
-    if (Array.isArray(obj)) {
-      return obj.map(item => deepClone(item));
-    }
-    if (obj !== null && typeof obj === "object") {
-      const out = {};
-      for (const [k, v] of Object.entries(obj)) {
-        out[k] = deepClone(v);
-      }
-      return out;
-    }
-    return obj;
+    process.stderr.write(`grader worker: failed to structuredClone ${label}; value will default to {}\n`);
+    return undefined;
   }
 }
 
@@ -64,8 +56,8 @@ async function main() {
   }
 
   try {
-    const trace = deepFreeze(deepClone(payload.trace || {}));
-    const config = deepFreeze(deepClone(payload.config || {}));
+    const trace = deepFreeze(tryStructuredClone(payload.trace, "trace") ?? {});
+    const config = deepFreeze(tryStructuredClone(payload.config, "config") ?? {});
     const run = deepFreeze({ graderCount: Number(payload.graderCount) || 0 });
     const workflow = deepFreeze({});
     const script = String(payload.script || "");

diff --git a/pkg/workflow/graders_config.go b/pkg/workflow/graders_config.go
@@ -206,8 +206,14 @@ func (c *Compiler) parseGradersFromFrontmatter(frontmatter map[string]any) (*Gra
 	}
 
 	// Parse explicit entries
+	seenNormalizedIDs := make(map[string]string, len(m))
 	for id, entryRaw := range m {
+		rawID := id
 		id = strings.TrimSpace(id)
+		if existingRawID, exists := seenNormalizedIDs[id]; exists {
+			return nil, fmt.Errorf("graders has duplicate id %q after normalization. Remove whitespace variants (for example %q and %q)", id, existingRawID, rawID)
+		}
+		seenNormalizedIDs[id] = rawID
 		if !graderIDPattern.MatchString(id) {
 			return nil, fmt.Errorf("graders has invalid id %q: must match %s. Example:\ngraders:\n  my-metric:\n    script: \"return { value: trace.toolCalls.length }\"", id, graderIDPattern.String())
 		}
@@ -219,7 +225,11 @@ func (c *Compiler) parseGradersFromFrontmatter(frontmatter map[string]any) (*Gra
 			def = builtinDefFromMeta(meta)
 		}
 
+		_, isBuiltin := builtinSet[id]
 		if entryRaw == nil {
+			if !isBuiltin {
+				return nil, fmt.Errorf("graders.%s is not a built-in grader and requires a 'script' field. Built-in graders: %s", id, strings.Join(BuiltinGraderIDs, ", "))
+			}
 			cfg.Graders[id] = def
 			continue
 		}
@@ -229,7 +239,6 @@ func (c *Compiler) parseGradersFromFrontmatter(frontmatter map[string]any) (*Gra
 			return nil, fmt.Errorf("graders.%s must be a map or null, got %T. Example:\ngraders:\n  %s:\n    enabled: true", id, entryRaw, id)
 		}
 
-		_, isBuiltin := builtinSet[id]
 		if err := parseGraderEntryFields(def, entry, id, isBuiltin); err != nil {
 			return nil, err
 		}

diff --git a/pkg/workflow/graders_config_test.go b/pkg/workflow/graders_config_test.go
@@ -160,6 +160,22 @@ func TestParseGradersFromFrontmatter_CustomWithoutScript(t *testing.T) {
 	}
 }
 
+// TestParseGradersFromFrontmatter_CustomNullRejected verifies null custom graders are rejected at parse time.
+func TestParseGradersFromFrontmatter_CustomNullRejected(t *testing.T) {
+	var c Compiler
+	_, err := c.parseGradersFromFrontmatter(map[string]any{
+		"graders": map[string]any{
+			"my-custom": nil,
+		},
+	})
+	if err == nil {
+		t.Fatal("expected error for null custom grader")
+	}
+	if !strings.Contains(err.Error(), "requires a 'script' field") {
+		t.Fatalf("expected missing-script error, got: %v", err)
+	}
+}
+
 // TestParseGradersFromFrontmatter_ScriptLengthUsesCharacters verifies limits align to character count.
 func TestParseGradersFromFrontmatter_ScriptLengthUsesCharacters(t *testing.T) {
 	var c Compiler
@@ -199,6 +215,23 @@ func TestParseGradersFromFrontmatter_InvalidID(t *testing.T) {
 	}
 }
 
+// TestParseGradersFromFrontmatter_DuplicateNormalizedID verifies whitespace variants are rejected.
+func TestParseGradersFromFrontmatter_DuplicateNormalizedID(t *testing.T) {
+	var c Compiler
+	_, err := c.parseGradersFromFrontmatter(map[string]any{
+		"graders": map[string]any{
+			"retry-test":  map[string]any{"script": "return 1"},
+			" retry-test": map[string]any{"script": "return 2"},
+		},
+	})
+	if err == nil {
+		t.Fatal("expected duplicate normalized id error")
+	}
+	if !strings.Contains(err.Error(), "duplicate id") {
+		t.Fatalf("expected duplicate id error, got: %v", err)
+	}
+}
+
 // TestParseGradersFromFrontmatter_AllDisabledError verifies error when all disabled.
 func TestParseGradersFromFrontmatter_AllDisabledError(t *testing.T) {
 	var c Compiler