Fix benchmark trace publication

[rgarcia]

summary

• publish useful Braintrust traces with inputs, structured tool turns, phase timelines, browser lifetime, and correct cached-token accounting
• redact typed values, credentials, private-info output, and browser identifiers, then validate the complete payload before publication
• fail before Harbor starts when PurelyMail cannot create a disposable account, and retry isolated setup failures
• classify ungraded step failures as infrastructure instead of task outcomes

validation

• bun test (281 tests)
• bunx tsc --noEmit
• bunx prettier --check ...
• bash -n benchmarks/harbor/clawbench/run.sh
• go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 .github/workflows/benchmark-clawbench.yml
• live Braintrust single-trial publication: verified phase spans, non-empty LLM inputs, positive turn durations, exact cached-token cost, redaction, and session-match diagnostics; deleted the temporary experiment afterward
• local PurelyMail preflight correctly rejects the currently invalid token before creating a Harbor dataset

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
mcp	Ready	Preview	Sep 2, 2026 1:59am UTC

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 003080a. Configure here.}

[vercel]

The TYPED_CALL regex captures .fill()/.type() arguments with [^)]*, which truncates at the first ), so form values containing ) (e.g. strong passwords) or nested-call selectors are neither redacted nor caught by the fail-closed assertSafeToPublish guard, leaking secrets to Braintrust.

[bmsaadat]

Review

Verdict: Request changes (two fixes; direction is right)

The trace shape is a real improvement: inputs on llm spans, phase timeline, browser lifetime, cached-token metrics. The PurelyMail preflight is the right fix for the setup-failure class, and the step-level exception classification matches how harbor 0.21.0 records step failures (multi_step.py stores them on the step and the trial completes normally). Verified locally at 2586a70: 281 tests, tsc --noEmit, prettier, and bash -n run.sh all pass. I also checked the new fields and retry names against the pins (harbor 0.21.0, harbor-hypeman 0.1.2, kernel/ClawBench c7feaa2) rather than the PR text.

Requested changes

1. A ) inside a typed value defeats both the redactor and the safety assert. TYPED_CALL captures with [^)]*, so the argument list is cut at the first ), the selector gets redacted, and the value is published. assertSafeToPublish re-runs the same truncated scan, so it passes. Reproduced at head:

   page.fill('#password', 'Str0ng)Pass!')  -> page.fill('[REDACTED]', 'Str0ng)Pass!')   assert passes
   page.locator('#pw').fill('abc)def')     -> unchanged                                 assert passes
   

   Multi-line template literals, pressSequentially, and keyboard.insertText are also uncovered. The benchmark's own password is token_urlsafe so it cannot trigger this; it needs an agent-invented value, which is what the 38 sign-up tasks and 2 password-manager tasks produce. Suggest tokenizing the argument list to find the matching close paren, adding pressSequentially|insertText, letting the literal regex span newlines, and giving the assert a check that does not share typedCallValues, so fail-closed stays independent of the redactor.

2. Row-level cost is gone, and the per-span replacement is uneven across agents. metricRecord dropped cost_usd and the token counts from the root row (main published harbor's agent_result.cost_usd, which both converters set). The replacement reads step.metrics.cost_usd on llm spans: harbor's Codex converter sets it per API call (litellm estimate), but the Claude Code converter never sets it per step (_build_metrics writes cost_usd=None; only final_metrics.total_cost_usd and agent_result.cost_usd are set), so a Claude Code run publishes no cost anywhere. estimated_cost is Braintrust's documented cost field. Suggest keeping row-level metrics under canonical names (prompt_tokens = n_input_tokens, which harbor documents as cache-inclusive; prompt_cached_tokens = n_cache_tokens; completion_tokens; tokens; estimated_cost = cost_usd) and emitting estimated_cost on llm spans only when ATIF carries it. The synthetic fixture puts cost_usd on the step, which is what hid the Claude Code gap.

Non-blocking

• Private-info harvesting scrubs ordinary words out of the whole trace. privateInfoValues collects every JSON string value of 4+ chars from a my-info read (172 unique values from the pinned alex_green_personal_info.json) and substring-replaces them across every string in the trial. With the real values, {"name":"Email"} <button>Close Window</button> "Single sign-on" {"width":1208} becomes {"name":"[REDACTED]"} <button>Close [REDACTED]</button> "[REDACTED] sign-on" {"width":[REDACTED]}. The fake IDs and account numbers are worth scrubbing; the names and common words are not. Harvesting only from sensitive-shaped keys would keep the protection and the readability.
• --retry-include RuntimeError does not retry step setup failures. harbor's retry loop returns as soon as the trial-level exception_info is None, and a failed setup.sh is stored on the step (multi_step.py:_run_step_setup) while the trial completes normally, which is exactly the shape the new fixture models. AgentSetupTimeoutError is raised at trial level and does retry. The README line should say agent setup only; the preflight is the real fix for the setup.sh class. Matching is by exact class name, so what RuntimeError now catches is harbor-hypeman's bare raises, transient and deterministic alike; a named exception for the transient paths in harbor-hypeman would let us drop the bare include.
• LLM-typed spans carry tool time. ATIF has one timestamp per step, so the turn interval includes the neighbouring tool execution, which dominates browser tasks, and Braintrust charts type: "llm" as model latency. The metadata label is honest; naming the span turn would make it visible in the UI.
• llmInput publishes only the previous agent step. prior.slice(previousAgent, previousAgent + 1) is a one-element slice, so user/system steps between two agent turns are dropped despite the README's "preceding context". prior.slice(previousAgent) gives the claimed behavior; the fixture has one agent step so the branch is untested.
• Fail-closed publish with no locator. assertSafeToPublish runs after all arms are built and throws a generic message with no trial, span, or field, so with the CI gate a red publish is a red multi-hour run debuggable only locally. Keep fail-closed, add the event id and key path to the error.
• Set-Cookie object keys are no longer redacted. SENSITIVE_FIELDS holds "set-cookie" but normalizedField rewrites - to _ first, so it never matches (main caught ^SET-COOKIE$). Small exposure since object keys only come from tool arguments and the string-level cookie rule still covers header text, but it shows the key vocabulary is now hand-copied into five lists that already disagree (the assert omits token, the collector knows five names, the URL regex lacks four). One exported list with a pattern builder would prevent the next drift.
• Smaller: insert batches are count-only (100 events) while llm inputs now carry the previous step's observations, so a byte bound would be cheap insurance; llm/tool span IDs now key on trajectoryIndex, so re-publishing an already-published job dir leaves the old spans orphaned (rows are still replaced); harvested secrets apply to ATIF spans only, not the root row's instruction/error; the preflight runs once per arm and a failed deleteUser leaves a cbpreflight… account behind; sources[arm].stats.nErroredTrials still comes from harbor's trial-level count, so it reads 0 for an arm that benchByArm.infraErrors says had step failures.

Question

• The README previously kept task instructions off experiment rows; this publishes the redacted last user message as input.instruction. ClawBench's instruction is the public prompt plus browser rules, the my-info file list, and extra_info names and descriptions, so it looks fine. Just confirming the policy change is deliberate.