feat: Playwright-batch facade surface, HardBench audit, and harness fixes from the smoke gates - #2867
feat: Playwright-batch facade surface, HardBench audit, and harness fixes from the smoke gates#2867miguelg719 wants to merge 49 commits into
Conversation
Sync facade/runtime.ts with the evals playwrightCompatRuntime experiment:
accessibility-tree fallback for getByRole misses, shadow-root XPath
resolution for snapshot-derived paths, 10 s default locator timeout.
Keep the facade's browser.close() -> closeRequested semantics.
run now returns a full batch envelope (telemetry, screenshot artifacts,
closeRequested, batch runtime), writes page.screenshot({ path }) files,
retries without a page target when the batch page vanished, and reports
telemetry to stderr as stagehand_playwright_compat lines.
Tool description and FACADE_AGENT_INSTRUCTIONS move to the Playwright
idiom (page/context/browser in scope, no AI methods). The previous text
is preserved verbatim as LEGACY_* and selectable with --surface=legacy.
…agehand_facade_legacy stagehand_facade_legacy starts the same facade server with --surface=legacy and mounts LEGACY_FACADE_AGENT_INSTRUCTIONS so runs on the earlier prompt are never mixed with the Playwright-idiom surface. Registered on every harness that mounts the facade (claude_code, codex, mastra, pi, eve, deepagents, fx, cursor).
V3Evaluator's built-in default (google/gemini-2.5-flash) was retired on 2026-07-09; without EVAL_VERIFIER_MODEL every rubric criterion failed with "Fused judgment call failed" and whole runs were silently unscored. The verifier now defaults to google/gemini-3.5-flash; an explicit override still fails loudly when its provider key is missing.
…flag 20 for verdict review Add scripts/audit-hardbenchmark.ts, a reusable four-check audit for the suite: live reachability in a Browserbase verified-mode session (start URL plus the deep pages each task needs, proxy retry on captcha/WAF; only a block on both attempts counts), rubric shape, achievability against past trajectory runs (model dirs aliased), and a stop-before-purchase heuristic. `--apply` writes `valid: false` + `invalid_reason` / `verdict_review` into the jsonl without deleting rows and can clear its own earlier quarantines. Suite builder now skips rows with valid === false and logs how many, and carries verdict_review into testcase metadata. Audit 2026-08-30: 45/46 valid. - quarantined: heb_comparison_shopping_1 (PerimeterX wall on both attempts) - 20 tasks flagged stop-before-purchase (see AUDIT-REVIEW.md) - all rubrics well-formed; no task showed missing content
The fx settings.json carried a "*": "deny" catch-all. In fx 0.0.3 a deny rule hides the tool from the model, and "*" also hid fx's own mcp_search_tools / mcp_select_tool, so no dynamic MCP tool could ever be selected: the trace showed tool_schema_count=1 and every HardBench step was web_fetch (which is not permission-gated in 0.0.3, so its deny rule is inert). Drop the catch-all; explicit per-tool denies still hide the built-ins. The per-run workspace also sat beside the throwaway $HOME instead of below it, so fx skipped AGENTS.md (project_rules_omitted: workspace is not below home) and the exact-tool-name guidance never reached the model. Nest the workspace under home via resolveFxRuntimePaths and spell out in AGENTS.md that web_fetch is not a browser. Verified: 1-task HardBench probe now runs mcp_select_tool -> 9x mcp_stagehand_run (Playwright idiom), zero web_fetch, verifier pass.
…nd duplicate screenshots Both in-process harnesses exhausted Node's default heap at concurrency 5 on HardBenchmark. Mastra's step-finish chunks carry the full request body under payload.metadata, which summarizeMastraEvent stringified into an unclipped log detail every step while events[] kept every chunk; pi kept each screenshot as base64 in tool_execution_end, again in the toolResult message_end, and again in both log details. Mastra now retains only the fields the trajectory adapter and usage accounting read (compactMastraEvent) and clips fallback details. pi decodes screenshots to one Buffer on the tool event, drops image payloads from non-assistant messages, and redacts image data from log details; the pi trajectory adapter accepts the Buffer form.
With langchain_openai on the Responses API, AIMessage.content carries function_call (and reasoning) blocks beside the text blocks. flatten_text json-dumped anything that was not text or an image, so every step's reasoning and evidence text became the raw function_call JSON. Skip tool-call block types (they are already reported via message.tool_calls) and reduce reasoning blocks to their summary text.
…utput Braintrust leaves result.output undefined when the task function threw or its span failed after the task returned, and the end-of-run mapping dereferenced output._success, so one such row crashed the CLI before the summary, per-model table, and experiment.json link were written for the whole run. Such rows now count as failures carrying the Braintrust error message.
…ade tool calls Codex 0.147 treats MCP tools without a readOnlyHint as needing approval under the read-only sandbox. Headless runs have no reviewer, so every stagehand facade call failed as "user cancelled MCP tool call" and gpt-5.4-mini fell back to whatever the operator's ~/.codex/config.toml exposed (node_repl, bundled plugins) — three HardBench tasks "passed" without touching the browser. - Pre-approve tools on every runner-mounted MCP server (default_tools_approval_mode = "approve"); the sandbox stays read-only. - Run each Codex session with CODEX_HOME pointed at a per-run directory so the global config can never leak extra servers or plugins into a cell. - Record facade_tool_calls / facade_tool_call_failures on external-harness results so a pass with zero browser use is visible in Braintrust.
… layout errors; lenient snapshot ids
page.frameLocator(selector) (plus locator.frameLocator/contentFrame) returns
a locator factory scoped to that iframe. css/xpath/text= tails compile to
Stagehand hop selectors ("iframe >> tail"), which the extension's deepLocator
resolves across cross-origin frames; getByRole/getByLabel/exact getByText
resolve through the includeIframes accessibility snapshot, scoped to nodes
under the first hop's iframe XPath, and act via deep XPaths. Supports nested
frameLocator chaining, first/nth/last, click/fill/type/press/hover/
selectOption/check/setInputFiles, count/isVisible/waitFor/text getters.
Unsupported operations fail with a targeted message.
Strict-mode violations now list up to five candidates (tag, text, visibility)
plus how to disambiguate; check()/uncheck() report their own method name.
A matched element with no layout box is reported as not rendered instead of
the bare -32000 CDP error after the retry window.
Snapshot actions accept a backend id without its frame-ordinal prefix when
it is unambiguous (models frequently pass "7812" for "0-7812"), and retry
once after 250 ms on layout-object errors.
Per-arm verifiability now counts successful runs whose facade_tool_calls metric is 0 and prints them next to the unverifiable-criteria summary. When EVAL_MAX_UNVERIFIABLE_CRITERIA gates the batch, such passes fail it too: the rubric verifier cannot tell an answer fetched with curl from one found in the browser.
…provider, dataset Bench experiments previously exposed only environment/harness/tier (and model only when --model was passed), so cells could not be grouped by surface or model in Braintrust/LangSmith. Derive them from the planned rows.
|
There was a problem hiding this comment.
24 issues found across 53 files
Confidence score: 2/5
packages/integrations/core/src/facade/runtime.tsforwards arbitrary browser/runtime text throughdescribeActionFailure, which can expose unsanitized external content in errors—sanitize failure messages before wrapping them inErrorinstances.packages/evals/scripts/audit-hardbenchmark.tscan mark skipped or failed reachability checks as valid and clear existing quarantines, whileAUDIT-REVIEW.mdassumes proxies the runner does not enable; gate unquarantining on completed audits and align campaign proxy configuration or task documentation.packages/integrations/core/src/facade/tools.tsmay leave the browser/session alive afterbrowser.close()and replay earlier side-effecting actions after a later snapshot failure, risking leaked resources and duplicate clicks or input—route close requests to the host and retry only unexecuted work.packages/integrations/core/src/facade/runtime.tscan resolve nested frames incorrectly and ignore role-state or visibility options, whilepackages/evals/framework/codexToolAdapter.tscan hide existing Codex login credentials in its isolated home; resolve every frame hop, preserve query options, and copy login credentials into the run environment.
Not reviewed (too large): packages/evals/datasets/hardbenchmark/audit-2026-08-30.json (~2,831 lines), packages/evals/datasets/hardbenchmark/HardBenchmark_data.jsonl (~42 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/core/src/facade/runtime.ts">
<violation number="1" location="packages/integrations/core/src/facade/runtime.ts:248">
P2: When the accessibility fallback sees a hidden role node, `getByRole` can return it even without `includeHidden: true` because this conversion discards the visibility option. Preserve `includeHidden` in the fallback step and reject non-visible candidates unless it is enabled.</violation>
<violation number="2" location="packages/integrations/core/src/facade/runtime.ts:949">
P1: Custom agent: **Exception and error message sanitization**
When a facade action fails, `describeActionFailure` forwards arbitrary browser/runtime text and wraps it in generic `new Error()` instances. Sanitize external details and raise a dedicated typed facade error instead; apply the same treatment to the new strict-mode and frame-locator errors.</violation>
<violation number="3" location="packages/integrations/core/src/facade/runtime.ts:1640">
P2: When code uses state options with `frame.getByRole`, the facade silently ignores them and may act on the wrong control. Carry the role state options through `FrameQuery` and enforce them during accessibility-tree matching.</violation>
<violation number="4" location="packages/integrations/core/src/facade/runtime.ts:1787">
P1: Nested `frameLocator()` calls are not scoped to their nested iframe: the accessibility route filters only by the first hop. Resolve and apply every hop before accepting a candidate, so `frameLocator("#outer").frameLocator("#inner")` cannot select an element elsewhere under `#outer`.</violation>
</file>
<file name="packages/integrations/core/src/facade/contract.ts">
<violation number="1" location="packages/integrations/core/src/facade/contract.ts:157">
P2: Updating the canonical prompt here leaves CrewAI's copied `FACADE_AGENT_INSTRUCTIONS` stale. Its contract test compares the copy with this template, so the integration test fails; update the CrewAI copy alongside this change or generate it from the canonical constant.</violation>
<violation number="2" location="packages/integrations/core/src/facade/contract.ts:159">
P2: When an agent follows this prompt and calls `page.waitForURL`, the facade rejects the call because the runtime does not implement that method. Remove this example or implement `waitForURL` before advertising it.</violation>
</file>
<file name="packages/evals/framework/runner.ts">
<violation number="1" location="packages/evals/framework/runner.ts:385">
P2: Default `stagehand` benchmark runs omit `tool_surface` from experiment metadata, so those experiments cannot be grouped by the promised surface metadata. Propagate a stable default surface for these runs, or explicitly revise the metadata contract to allow this field to be absent.</violation>
</file>
<file name="packages/evals/tui/commands/verify.ts">
<violation number="1" location="packages/evals/tui/commands/verify.ts:51">
P2: When `evals verify` runs without `--model`, `handleVerify` still uses V3Evaluator's built-in default, so this help advertises a model the command does not select. Pass `DEFAULT_VERIFIER_MODEL` here or reuse `createVerifierEvaluator` before documenting 3.5.</violation>
</file>
<file name="packages/evals/framework/harnesses/piAdapter.ts">
<violation number="1" location="packages/evals/framework/harnesses/piAdapter.ts:139">
P2: The new retained-`Buffer` path is not covered by the adapter tests, so a regression could silently drop screenshots from trajectories while the existing test still passes through the base64 fallback. Add a focused adapter test with `{ bytes: image, mimeType: "image/png" }` and assert the resulting image modality preserves `image`.</violation>
</file>
<file name="packages/evals/evals.config.json">
<violation number="1" location="packages/evals/evals.config.json:27">
P2: Because this marker is committed, every fresh checkout skips the first-run welcome. Remove the runtime-generated `_meta` block from the baseline config and let `markFirstRunComplete` persist it locally.</violation>
</file>
<file name="packages/evals/framework/codexToolAdapter.ts">
<violation number="1" location="packages/evals/framework/codexToolAdapter.ts:114">
P1: When an eval uses an existing `codex login` instead of `OPENAI_API_KEY`, this empty per-run home removes the login credentials and Codex fails before the task starts. Copy the login credentials into the isolated home, or isolate configuration while preserving the authenticated home.</violation>
</file>
<file name="packages/evals/framework/verifierAdapter.ts">
<violation number="1" location="packages/evals/framework/verifierAdapter.ts:35">
P2: The new default selection and credential policy have no focused tests, so a future model or key-resolution change can silently ungrade verifier runs. Add tests for the default model, explicit missing-key failure, and keyless providers.
(Based on your team's feedback about unit coverage for changed logic.)</violation>
</file>
<file name="packages/integrations/core/src/facade/tools.ts">
<violation number="1" location="packages/integrations/core/src/facade/tools.ts:200">
P1: When a later snapshot action fails with a layout error, this retry replays every earlier action in the batch, causing duplicate clicks, typing, or other side effects. Retry only the failed or unexecuted action, or remove the whole-batch retry.</violation>
<violation number="2" location="packages/integrations/core/src/facade/tools.ts:219">
P1: When facade code calls `browser.close()`, `runNow` only reports `closeRequested`; the stdio host logs that report and never closes Stagehand or the browser, leaving the persistent browser/session alive. Route this flag to the host cleanup lifecycle after the batch.</violation>
</file>
<file name="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md">
<violation number="1" location="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md:1">
P3: The report says 2026-08-30, but the linked audit JSON records 2026-08-31. Align the report and artifact dates so campaign provenance is unambiguous.</violation>
<violation number="2" location="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md:50">
P1: These tasks are not guaranteed reachable in campaigns: the runner does not enable Browserbase proxies, despite this note claiming it always does. Enable proxies for campaign sessions or document these tasks as proxy-dependent.</violation>
</file>
<file name="packages/integrations/pi-sdk/src/session.ts">
<violation number="1" location="packages/integrations/pi-sdk/src/session.ts:324">
P2: When a detail contains a secret near the 20,000-character boundary, `clip` runs before `sanitizeErrorMessage` and can leave an unmatched secret fragment in logs and transcripts. Sanitize the complete detail first, then apply the length cap at all three detail sites.</violation>
</file>
<file name="packages/evals/scripts/audit-hardbenchmark.ts">
<violation number="1" location="packages/evals/scripts/audit-hardbenchmark.ts:294">
P2: If Stagehand initialization or page discovery fails after `browserbase.launch`, this helper leaks the live Browserbase session because cleanup only exists on the returned `Session`. Wrap attachment and page acquisition in `try/catch`, close Stagehand and the browser, then rethrow.</violation>
<violation number="2" location="packages/evals/scripts/audit-hardbenchmark.ts:349">
P2: Raw Browserbase/Stagehand errors are persisted in `audit-<date>.json`, where they can expose credential-bearing URLs or session details. Sanitize these messages or record a generic probe error before serialization.
(Based on your team's feedback about sanitized Browserbase session errors.)</violation>
<violation number="3" location="packages/evals/scripts/audit-hardbenchmark.ts:635">
P2: When `--only` reruns a task without `--trajectories`, this replacement discards the prior achievability record from the existing report. Preserve the previous task’s achievability whenever the optional trajectory check was not requested.</violation>
<violation number="4" location="packages/evals/scripts/audit-hardbenchmark.ts:663">
P1: When reachability is skipped or the Browserbase session fails, `a.valid` remains true and this branch clears existing bot-wall/dead quarantines. Guard the cleanup so only a completed reachability audit can unquarantine a row.</violation>
</file>
<file name="packages/evals/suites/hardbenchmark.ts">
<violation number="1" location="packages/evals/suites/hardbenchmark.ts:166">
P3: The new manual-review marker is not part of the `Testcase.metadata` type. Add `verdict_review?: string` to that metadata contract so downstream reporting and review tooling can consume it without casts.</violation>
</file>
<file name="packages/integrations/deepagents/runner/run_eval.py">
<violation number="1" location="packages/integrations/deepagents/runner/run_eval.py:146">
P2: If a provider emits a content block with a non-hashable `type`, this membership test raises `TypeError` before fallback serialization and fails the run; validate the type before set membership.</violation>
<violation number="2" location="packages/integrations/deepagents/runner/run_eval.py:148">
P2: When an Anthropic response contains a native `thinking` block, this branch is skipped and the entire block is serialized into `assistant.text` and `final.text; handle `thinking` blocks here.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Suite as Suite Builder
participant Audit as Audit Script
participant Runner as External Runner
participant Adapter as Tool Adapter
participant Server as Facade MCP Server
participant Runtime as Playwright Runtime
participant Browser as Browser Session
participant Verifier as Rubric Verifier
participant Metrics as Metrics/Verifiability
Note over Suite,Audit: Dataset Validity Pipeline
Suite->>Audit: Read HardBenchmark rows
Audit->>Audit: Reachability probe (verified mode)
Audit->>Audit: Rubric schema validation
Audit->>Audit: Store audit results
Audit-->>Suite: Quarantine invalid rows (valid:false)
Suite->>Runner: Build testcase list (skip quarantined)
Note over Runner,Adapter: Harness Startup & Facade Mount
Runner->>Adapter: Prepare harness (eve/deepagents/codex/fx)
Adapter->>Server: Spawn facade stdio server
Adapter->>Adapter: Select surface (playwright/legacy)
Adapter->>Server: Mount MCP server
Server->>Runtime: Initialize Stagehand/Playwright
Note over Adapter,Runtime: Browser Session & Facade Calls
Adapter->>Server: Set up MCP config
Server->>Runtime: Execute agent code (playwright fallback)
Runtime->>Browser: Launch browser (Browserbase/LOCAL)
Browser-->>Runtime: Active page/context
alt Playwright-surface run
Adapter->>Server: run() with Playwright API
else Legacy-surface run
Adapter->>Server: run() with legacy prompt contract
end
Runtime->>Runtime: getByRole a11y-tree fallback
Runtime->>Runtime: frameLocator / contentFrame
Runtime->>Browser: Navigate, locate, interact
Runtime-->>Adapter: Return results + telemetry
Note over Runner,Verifier: Verdict & Metrics
Adapter->>Runner: Trajectory + tool call data
Runner->>Verifier: Grade trajectory
Verifier->>Runner: Return graded result
Runner->>Metrics: Compute facade_tool_calls/failures
Metrics->>Runner: Check passesWithoutBrowserUse
Note over Adapter,Verifier: Harness-specific Fixes
opt codex harness
Adapter->>Adapter: Set default_tools_approval_mode=approve
Adapter->>Adapter: Isolate CODEX_HOME per-run
end
opt fx harness
Adapter->>Adapter: Nested workspace under home
Adapter->>Adapter: Remove "*" deny rule
end
opt mastra/pi harness
Adapter->>Adapter: Compact events (drop request bodies)
Adapter->>Adapter: Decode screenshots once
end
Runner->>Runner: Build summary (survive missing Braintrust output)
Runner-->>Suite: Task result with metrics/gates
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * "-32000 Node does not have a layout object". After the retry window that | ||
| * is what the agent saw; say what it means instead. | ||
| */ | ||
| const describeActionFailure = (method: string, error: unknown, timeout: number): Error => { |
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
When a facade action fails, describeActionFailure forwards arbitrary browser/runtime text and wraps it in generic new Error() instances. Sanitize external details and raise a dedicated typed facade error instead; apply the same treatment to the new strict-mode and frame-locator errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/runtime.ts, line 949:
<comment>When a facade action fails, `describeActionFailure` forwards arbitrary browser/runtime text and wraps it in generic `new Error()` instances. Sanitize external details and raise a dedicated typed facade error instead; apply the same treatment to the new strict-mode and frame-locator errors.</comment>
<file context>
@@ -650,6 +932,30 @@ export async function createPlaywrightCompatRuntime(
+ * "-32000 Node does not have a layout object". After the retry window that
+ * is what the agent saw; say what it means instead.
+ */
+ const describeActionFailure = (method: string, error: unknown, timeout: number): Error => {
+ const message = error instanceof Error ? error.message : String(error);
+ if (!LAYOUT_ERROR_RE.test(message)) return error instanceof Error ? error : new Error(message);
</file context>
| xpaths: string[]; | ||
| names: string[]; | ||
| }> { | ||
| const prefix = await iframeHostXPath(this.state, this.hops[0]!); |
There was a problem hiding this comment.
P1: Nested frameLocator() calls are not scoped to their nested iframe: the accessibility route filters only by the first hop. Resolve and apply every hop before accepting a candidate, so frameLocator("#outer").frameLocator("#inner") cannot select an element elsewhere under #outer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/runtime.ts, line 1787:
<comment>Nested `frameLocator()` calls are not scoped to their nested iframe: the accessibility route filters only by the first hop. Resolve and apply every hop before accepting a candidate, so `frameLocator("#outer").frameLocator("#inner")` cannot select an element elsewhere under `#outer`.</comment>
<file context>
@@ -1145,6 +1489,638 @@ export async function createPlaywrightCompatRuntime(
+ xpaths: string[];
+ names: string[];
+ }> {
+ const prefix = await iframeHostXPath(this.state, this.hops[0]!);
+ if (!prefix) return { xpaths: [], names: [] };
+ const snapshot = (await this.state.rawPage.snapshot({ includeIframes: true })) as {
</file context>
| for (const [key, value] of Object.entries(baseEnv)) { | ||
| if (value !== undefined) env[key] = value; | ||
| } | ||
| env.CODEX_HOME = codexHome; |
There was a problem hiding this comment.
P1: When an eval uses an existing codex login instead of OPENAI_API_KEY, this empty per-run home removes the login credentials and Codex fails before the task starts. Copy the login credentials into the isolated home, or isolate configuration while preserving the authenticated home.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/codexToolAdapter.ts, line 114:
<comment>When an eval uses an existing `codex login` instead of `OPENAI_API_KEY`, this empty per-run home removes the login credentials and Codex fails before the task starts. Copy the login credentials into the isolated home, or isolate configuration while preserving the authenticated home.</comment>
<file context>
@@ -60,30 +60,73 @@ export const CODEX_TOOL_SURFACES: ToolSurface[] = [
+ for (const [key, value] of Object.entries(baseEnv)) {
+ if (value !== undefined) env[key] = value;
+ }
+ env.CODEX_HOME = codexHome;
+ return env;
+}
</file context>
| telemetry: envelope.telemetry, | ||
| batchRoundTripMs: performance.now() - startedAt, | ||
| batchRuntimeMs: envelope.batchRuntimeMs, | ||
| closeRequested: envelope.closeRequested, |
There was a problem hiding this comment.
P1: When facade code calls browser.close(), runNow only reports closeRequested; the stdio host logs that report and never closes Stagehand or the browser, leaving the persistent browser/session alive. Route this flag to the host cleanup lifecycle after the batch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/tools.ts, line 219:
<comment>When facade code calls `browser.close()`, `runNow` only reports `closeRequested`; the stdio host logs that report and never closes Stagehand or the browser, leaving the persistent browser/session alive. Route this flag to the host cleanup lifecycle after the batch.</comment>
<file context>
@@ -166,11 +210,15 @@ export class StagehandFacadeTools {
+ telemetry: envelope.telemetry,
+ batchRoundTripMs: performance.now() - startedAt,
+ batchRuntimeMs: envelope.batchRuntimeMs,
+ closeRequested: envelope.closeRequested,
+ });
+ await this.writeScreenshotArtifacts(envelope.artifacts);
</file context>
| // A freshly hydrated element with no layout box is usually mid-render | ||
| // (menus, lazy lists); give it one beat before reporting. | ||
| if (!isLayoutError(error)) throw error; | ||
| await page.waitForTimeout(250); |
There was a problem hiding this comment.
P1: When a later snapshot action fails with a layout error, this retry replays every earlier action in the batch, causing duplicate clicks, typing, or other side effects. Retry only the failed or unexecuted action, or remove the whole-batch retry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/tools.ts, line 200:
<comment>When a later snapshot action fails with a layout error, this retry replays every earlier action in the batch, causing duplicate clicks, typing, or other side effects. Retry only the failed or unexecuted action, or remove the whole-batch retry.</comment>
<file context>
@@ -147,15 +180,26 @@ export class StagehandFacadeTools {
+ // A freshly hydrated element with no layout box is usually mid-render
+ // (menus, lazy lists); give it one beat before reporting.
+ if (!isLayoutError(error)) throw error;
+ await page.waitForTimeout(250);
+ result = await runBatch();
+ }
</file context>
| const stagehand = await Stagehand.create({ browser }); | ||
| const pages = await browser.context.pages(); | ||
| const page = pages[0] ?? (await browser.context.newPage()); | ||
| return { | ||
| stagehand, | ||
| page, | ||
| sessionId: browser.sessionId ?? "", | ||
| close: async () => { | ||
| await stagehand.close().catch(() => {}); | ||
| await browser.close().catch(() => {}); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
P2: If Stagehand initialization or page discovery fails after browserbase.launch, this helper leaks the live Browserbase session because cleanup only exists on the returned Session. Wrap attachment and page acquisition in try/catch, close Stagehand and the browser, then rethrow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/scripts/audit-hardbenchmark.ts, line 294:
<comment>If Stagehand initialization or page discovery fails after `browserbase.launch`, this helper leaks the live Browserbase session because cleanup only exists on the returned `Session`. Wrap attachment and page acquisition in `try/catch`, close Stagehand and the browser, then rethrow.</comment>
<file context>
@@ -0,0 +1,680 @@
+ },
+ userMetadata: { stagehand: "true", evals: "true", audit: "hardbenchmark" },
+ } as Parameters<typeof browserbase.launch>[0]);
+ const stagehand = await Stagehand.create({ browser });
+ const pages = await browser.context.pages();
+ const page = pages[0] ?? (await browser.context.newPage());
</file context>
| const stagehand = await Stagehand.create({ browser }); | |
| const pages = await browser.context.pages(); | |
| const page = pages[0] ?? (await browser.context.newPage()); | |
| return { | |
| stagehand, | |
| page, | |
| sessionId: browser.sessionId ?? "", | |
| close: async () => { | |
| await stagehand.close().catch(() => {}); | |
| await browser.close().catch(() => {}); | |
| }, | |
| }; | |
| let stagehand: Stagehand | undefined; | |
| try { | |
| const createdStagehand = await Stagehand.create({ browser }); | |
| stagehand = createdStagehand; | |
| const pages = await browser.context.pages(); | |
| const page = pages[0] ?? (await browser.context.newPage()); | |
| return { | |
| stagehand: createdStagehand, | |
| page, | |
| sessionId: browser.sessionId ?? "", | |
| close: async () => { | |
| await createdStagehand.close().catch(() => {}); | |
| await browser.close().catch(() => {}); | |
| }, | |
| }; | |
| } catch (error) { | |
| if (stagehand) await stagehand.close().catch(() => {}); | |
| await browser.close().catch(() => {}); | |
| throw error; | |
| } |
| and isinstance(block.get("text"), str) | ||
| ): | ||
| parts.append(block["text"]) | ||
| elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES: |
There was a problem hiding this comment.
P2: If a provider emits a content block with a non-hashable type, this membership test raises TypeError before fallback serialization and fails the run; validate the type before set membership.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/deepagents/runner/run_eval.py, line 146:
<comment>If a provider emits a content block with a non-hashable `type`, this membership test raises `TypeError` before fallback serialization and fails the run; validate the type before set membership.</comment>
<file context>
@@ -122,6 +143,11 @@ def flatten_text(content: object) -> str:
and isinstance(block.get("text"), str)
):
parts.append(block["text"])
+ elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES:
+ continue
+ elif isinstance(block, dict) and block.get("type") == "reasoning":
</file context>
| elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES: | |
| elif isinstance(block, dict) and isinstance(block.get("type"), str) and block.get("type") in _TOOL_CALL_BLOCK_TYPES: |
| parts.append(block["text"]) | ||
| elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES: | ||
| continue | ||
| elif isinstance(block, dict) and block.get("type") == "reasoning": |
There was a problem hiding this comment.
P2: When an Anthropic response contains a native thinking block, this branch is skipped and the entire block is serialized into assistant.text and final.text; handle thinking` blocks here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/deepagents/runner/run_eval.py, line 148:
<comment>When an Anthropic response contains a native `thinking` block, this branch is skipped and the entire block is serialized into `assistant.text` and `final.text; handle `thinking` blocks here.</comment>
<file context>
@@ -122,6 +143,11 @@ def flatten_text(content: object) -> str:
parts.append(block["text"])
+ elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES:
+ continue
+ elif isinstance(block, dict) and block.get("type") == "reasoning":
+ if text := _reasoning_text(block):
+ parts.append(text)
</file context>
| elif isinstance(block, dict) and block.get("type") == "reasoning": | |
| elif isinstance(block, dict) and block.get("type") in ("reasoning", "thinking"): |
| @@ -0,0 +1,58 @@ | |||
| # HardBenchmark validity audit — 2026-08-30 | |||
There was a problem hiding this comment.
P3: The report says 2026-08-30, but the linked audit JSON records 2026-08-31. Align the report and artifact dates so campaign provenance is unambiguous.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md, line 1:
<comment>The report says 2026-08-30, but the linked audit JSON records 2026-08-31. Align the report and artifact dates so campaign provenance is unambiguous.</comment>
<file context>
@@ -0,0 +1,58 @@
+# HardBenchmark validity audit — 2026-08-30
+
+Source: `scripts/audit-hardbenchmark.ts`, full record in `audit-2026-08-30.json`. Live probes ran in Browserbase `verified` mode with a proxy retry on captcha/WAF; achievability cross-referenced two frontier-model trajectory sets from 2026-08-24 (aliased `model-1`/`model-2`).
</file context>
| source_suite: row.source_suite, | ||
| failure_mode: row.failure_mode, | ||
| capability_axis: row.capability_axis, | ||
| ...(row.verdict_review ? { verdict_review: row.verdict_review } : {}), |
There was a problem hiding this comment.
P3: The new manual-review marker is not part of the Testcase.metadata type. Add verdict_review?: string to that metadata contract so downstream reporting and review tooling can consume it without casts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/suites/hardbenchmark.ts, line 166:
<comment>The new manual-review marker is not part of the `Testcase.metadata` type. Add `verdict_review?: string` to that metadata contract so downstream reporting and review tooling can consume it without casts.</comment>
<file context>
@@ -144,6 +163,7 @@ export const buildHardBenchmarkTestcases = (models: string[] | AgentModelEntry[]
source_suite: row.source_suite,
failure_mode: row.failure_mode,
capability_axis: row.capability_axis,
+ ...(row.verdict_review ? { verdict_review: row.verdict_review } : {}),
},
expected: true,
</file context>
…eltas Every session layer (claude-agent-sdk, codex-sdk, mastra-sdk, pi-sdk, eve-sdk, deepagents-sdk, fx-sdk, cursor-sdk) now classifies its raw events through a shared harnessEventLogLevel helper: stream fragments (*-delta, *_update, item.updated, stream_event) and bare lifecycle markers (*-start/*-end without content) are not logged at all, failures stay at level 1, everything else drops to level 2. The readable per-step trace is emitted from the normalized trajectory instead.
…starts
The facade stdio server gains a runner-only session_info tool (absent
from tools/list) that launches the browser if needed and reports
{ provider, sessionId }. The bridge exposes it as sessionInfo() with a
launch-sized timeout, and StagehandFacadeTool calls it for BROWSERBASE
runs so browserbaseSessionId/Url land on the start metadata; LOCAL runs
keep the lazy launch. stagehand_code publishes its session/debug URLs on
metadata the same way.
runExternalHarnessTask emits one 'trace' log line per normalized trajectory step (think / tool · ok|ERR · code → result) plus a final 'result · status · steps · facade_calls · tokens' line, so claude_code, codex, mastra, pi, eve, deepagents, fx and cursor rows all read the same. Full code/results ride in auxiliary, capped at 16 KB. EvalLogger.getLogs now filters to level <= 1 by default so debug lines stay out of output.logs; the runner re-reads logs after grading so trace and verifier lines ship with the row.
…ry harness task startAgentToolRuntime folds runner-provided target metadata and tool-published metadata into a BrowserSessionInfo that every prepared adapter (claude_code, codex, mastra, pi, eve, deepagents, fx, cursor) carries as browserSession. defineExternalHarness logs 'Browserbase session: <url>' (level 0, category session) right after adapter preparation and stamps sessionUrl / browserbaseSessionId / browserProvider on the TaskResult; the OTEL task span carries them too. Adapter setup chatter moves to level 2 so the session line heads output.logs. browse_cli logs the bare provider because the daemon does not report its session id.
There was a problem hiding this comment.
11 issues found across 51 files (changes from recent commits).
Confidence score: 2/5
packages/evals/framework/harnesses/externalRunner.tsandpackages/evals/framework/harnesses/traceLog.tscan persist credential-bearingstopReasonvalues in trace logs andresult.logs, creating a concrete sensitive-data exposure risk — pass the sanitized stop reason through every logging path.packages/integrations/core/src/facade/stdio-server.tsstill acceptssession_inforequests despite omitting the method fromtools/list, so an agent can obtain the Browserbase session ID — move the authorization or dispatch guard to the server-side handler.- Failure classification is incomplete across
packages/integrations/eve-sdk/src/session.ts,fx-sdk/src/session.ts,pi-sdk/src/session.ts,claude-agent-sdk/src/session.ts, andcursor-sdk/src/session.ts, which can hide real SDK/tool errors at debug level — classify nested and explicit error fields as failures and add coverage for each path. packages/evals/framework/browserSession.tscan construct the wrong URL when a session ID contains reserved characters, whilepackages/evals/framework/harnesses/traceLog.tscan emit inconsistent shared tool names and formats — encode IDs as one path segment and align trace output with the advertised surface.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/framework/browserSession.ts">
<violation number="1" location="packages/evals/framework/browserSession.ts:15">
P2: When a session ID contains a reserved path character, this helper generates a URL for a different path or query. Encode the ID as one URL path segment before interpolation.
(Based on your team's feedback about encoding session IDs in URL path segments.)</violation>
</file>
<file name="packages/evals/framework/harnesses/traceLog.ts">
<violation number="1" location="packages/evals/framework/harnesses/traceLog.ts:106">
P2: When an SDK stop reason contains credentials or other sensitive error text, this line writes the unsanitized value into persisted trace logs. Use the sanitized stop reason when constructing the trace input, or sanitize `outcome.stopReason` before adding it to the message.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging and returning them.)</violation>
<violation number="2" location="packages/evals/framework/harnesses/traceLog.ts:134">
P2: When the Fx or Eve harness emits its native tool name, the trace keeps the harness prefix instead of producing the advertised shared surface name. This also prints run code as JSON and skips snapshot/screenshot formatting; normalize `mcp_stagehand_*` and `stagehand__*` before the existing stagehand regex.</violation>
</file>
<file name="packages/integrations/eve-sdk/src/session.ts">
<violation number="1" location="packages/integrations/eve-sdk/src/session.ts:374">
P2: When an Eve tool returns `action.result` with a non-`completed` status or `result.isError === true`, this predicate leaves it at debug level 2, hiding the failure from normal harness logs. Classify failed `action.result` payloads as errors as well.</violation>
</file>
<file name="packages/integrations/cursor-sdk/src/session.ts">
<violation number="1" location="packages/integrations/cursor-sdk/src/session.ts:402">
P2: When a Cursor `result` carries `is_error: true` with no non-success subtype, this classifier logs it at debug level even though `resolveCursorStatus` treats it as an SDK failure. Include `event.is_error === true` in the result error predicate.</violation>
</file>
<file name="packages/integrations/fx-sdk/src/session.ts">
<violation number="1" location="packages/integrations/fx-sdk/src/session.ts:663">
P2: When fx ends a committed turn as cancelled/interrupted or returns an ask error, `logFxEvent` records the failure at level 2. Classify the turn and ask failure fields as `isError` so SDK failures remain visible at level 1.</violation>
</file>
<file name="packages/integrations/pi-sdk/src/session.ts">
<violation number="1" location="packages/integrations/pi-sdk/src/session.ts:308">
P2: When Pi reports a provider failure through `message_end.message.stopReason === "error"`, this predicate classifies it as level 2. `runPiSession` records the stop reason without logging a warning on that path, so the default persisted logs hide the failure; classify that assistant message as an error too.</violation>
</file>
<file name="packages/integrations/claude-agent-sdk/src/session.ts">
<violation number="1" location="packages/integrations/claude-agent-sdk/src/session.ts:289">
P2: When a Claude tool returns an error, the SDK places `is_error` on the nested `user.message.content` tool-result block, not necessarily on the outer message. Inspect those blocks before classifying the event, otherwise the failure is logged at level 2 and omitted from default persisted logs.</violation>
</file>
<file name="packages/evals/tests/core/stagehand-facade.test.ts">
<violation number="1" location="packages/evals/tests/core/stagehand-facade.test.ts:238">
P2: The stderr-based assertions can race: start() resolves as soon as the session_info stdout reply is processed (routeServerLine settles the call), while the "fake tools/call session_info" line is delivered by the separate child.stderr 'data' handler. Nothing waits for stderr to drain, so the assertion on logger.lines can run before the line is captured. Wait for the expected line (bounded) instead of asserting synchronously after start().</violation>
</file>
<file name="packages/evals/framework/harnesses/externalRunner.ts">
<violation number="1" location="packages/evals/framework/harnesses/externalRunner.ts:251">
P1: When a verifier runs and the SDK supplies a credential-bearing `outcome.stopReason`, this call sends the unsanitized value to the trace logger and `result.logs`. Pass `sanitizedStopReason` to the trace emitter so the trace cannot bypass the existing redaction.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging them.)</violation>
</file>
<file name="packages/integrations/core/src/facade/stdio-server.ts">
<violation number="1" location="packages/integrations/core/src/facade/stdio-server.ts:111">
P2: Because the agent relay forwards every `tools/call` to this dispatcher, omitting `session_info` from `tools/list` does not keep it runner-only; an agent can request it and receive the Browserbase session ID. Move this metadata exchange to an authenticated out-of-band runner channel, or reject this name for agent-originated calls instead of relying on discovery omission.
(Based on your team's feedback about runner-side facade session operations.)</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // every harness logs the same shape; a formatting bug must never fail | ||
| // the grade. | ||
| try { | ||
| emitTrajectoryTrace(logger, { trajectory, outcome, isFacadeTool }); |
There was a problem hiding this comment.
P1: When a verifier runs and the SDK supplies a credential-bearing outcome.stopReason, this call sends the unsanitized value to the trace logger and result.logs. Pass sanitizedStopReason to the trace emitter so the trace cannot bypass the existing redaction.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging them.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/externalRunner.ts, line 251:
<comment>When a verifier runs and the SDK supplies a credential-bearing `outcome.stopReason`, this call sends the unsanitized value to the trace logger and `result.logs`. Pass `sanitizedStopReason` to the trace emitter so the trace cannot bypass the existing redaction.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging them.) </comment>
<file context>
@@ -242,6 +244,18 @@ export async function runExternalHarnessTask<TRaw>({
+ // every harness logs the same shape; a formatting bug must never fail
+ // the grade.
+ try {
+ emitTrajectoryTrace(logger, { trajectory, outcome, isFacadeTool });
+ } catch (traceError) {
+ logger.warn({
</file context>
| } | ||
|
|
||
| export function browserbaseSessionUrl(sessionId: string): string { | ||
| return `https://www.browserbase.com/sessions/${sessionId}`; |
There was a problem hiding this comment.
P2: When a session ID contains a reserved path character, this helper generates a URL for a different path or query. Encode the ID as one URL path segment before interpolation.
(Based on your team's feedback about encoding session IDs in URL path segments.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/browserSession.ts, line 15:
<comment>When a session ID contains a reserved path character, this helper generates a URL for a different path or query. Encode the ID as one URL path segment before interpolation.
(Based on your team's feedback about encoding session IDs in URL path segments.) </comment>
<file context>
@@ -0,0 +1,89 @@
+}
+
+export function browserbaseSessionUrl(sessionId: string): string {
+ return `https://www.browserbase.com/sessions/${sessionId}`;
+}
+
</file context>
| } else if (name.includes(".")) { | ||
| name = name.slice(name.lastIndexOf(".") + 1); | ||
| } | ||
| if (/^stagehand_(?:browser_)?(run|snapshot|screenshot)$/u.test(name)) { |
There was a problem hiding this comment.
P2: When the Fx or Eve harness emits its native tool name, the trace keeps the harness prefix instead of producing the advertised shared surface name. This also prints run code as JSON and skips snapshot/screenshot formatting; normalize mcp_stagehand_* and stagehand__* before the existing stagehand regex.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/traceLog.ts, line 134:
<comment>When the Fx or Eve harness emits its native tool name, the trace keeps the harness prefix instead of producing the advertised shared surface name. This also prints run code as JSON and skips snapshot/screenshot formatting; normalize `mcp_stagehand_*` and `stagehand__*` before the existing stagehand regex.</comment>
<file context>
@@ -0,0 +1,239 @@
+ } else if (name.includes(".")) {
+ name = name.slice(name.lastIndexOf(".") + 1);
+ }
+ if (/^stagehand_(?:browser_)?(run|snapshot|screenshot)$/u.test(name)) {
+ name = name.replace(/^stagehand_(?:browser_)?/u, "");
+ }
</file context>
| const message = [ | ||
| "result", | ||
| outcome.status, | ||
| ...(outcome.stopReason ? [singleLine(outcome.stopReason)] : []), |
There was a problem hiding this comment.
P2: When an SDK stop reason contains credentials or other sensitive error text, this line writes the unsanitized value into persisted trace logs. Use the sanitized stop reason when constructing the trace input, or sanitize outcome.stopReason before adding it to the message.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging and returning them.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/traceLog.ts, line 106:
<comment>When an SDK stop reason contains credentials or other sensitive error text, this line writes the unsanitized value into persisted trace logs. Use the sanitized stop reason when constructing the trace input, or sanitize `outcome.stopReason` before adding it to the message.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging and returning them.) </comment>
<file context>
@@ -0,0 +1,239 @@
+ const message = [
+ "result",
+ outcome.status,
+ ...(outcome.stopReason ? [singleLine(outcome.stopReason)] : []),
+ `steps=${trajectory.steps.length}`,
+ ...(facadeCalls !== undefined ? [`facade_calls=${facadeCalls}`] : []),
</file context>
|
|
||
| export function logEveEvent(logger: HarnessLogger, event: EveEvent): void { | ||
| const level = harnessEventLogLevel(event.type, { | ||
| isError: event.type.endsWith(".failed"), |
There was a problem hiding this comment.
P2: When an Eve tool returns action.result with a non-completed status or result.isError === true, this predicate leaves it at debug level 2, hiding the failure from normal harness logs. Classify failed action.result payloads as errors as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/eve-sdk/src/session.ts, line 374:
<comment>When an Eve tool returns `action.result` with a non-`completed` status or `result.isError === true`, this predicate leaves it at debug level 2, hiding the failure from normal harness logs. Classify failed `action.result` payloads as errors as well.</comment>
<file context>
@@ -369,13 +370,18 @@ export function buildEveTranscript(events: EveEvent[]): string {
export function logEveEvent(logger: HarnessLogger, event: EveEvent): void {
+ const level = harnessEventLogLevel(event.type, {
+ isError: event.type.endsWith(".failed"),
+ hasContent: event.type.endsWith(".completed") || event.type === "action.result",
+ });
</file context>
| isError: event.type.endsWith(".failed"), | |
| isError: | |
| event.type.endsWith(".failed") || | |
| (event.type === "action.result" && | |
| isRecord(event.data) && | |
| isRecord(event.data.result) && | |
| event.data.result.kind === "tool-result" && | |
| (event.data.status !== "completed" || event.data.result.isError === true)), |
| isError: | ||
| (event.type === "stderr" && /\b(?:error|fatal|failed|panic)\b/iu.test(event.line)) || | ||
| (event.type === "tool_step" && | ||
| event.tool_results.some((result) => /^(?:error|failed)$/iu.test(result.status ?? ""))), |
There was a problem hiding this comment.
P2: When fx ends a committed turn as cancelled/interrupted or returns an ask error, logFxEvent records the failure at level 2. Classify the turn and ask failure fields as isError so SDK failures remain visible at level 1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/fx-sdk/src/session.ts, line 663:
<comment>When fx ends a committed turn as cancelled/interrupted or returns an ask error, `logFxEvent` records the failure at level 2. Classify the turn and ask failure fields as `isError` so SDK failures remain visible at level 1.</comment>
<file context>
@@ -658,11 +659,19 @@ export function buildFxTranscript(events: FxEvent[]): string {
export function logFxEvent(logger: HarnessLogger, event: FxEvent): void {
+ const level = harnessEventLogLevel(event.type, {
+ isError:
+ (event.type === "stderr" && /\b(?:error|fatal|failed|panic)\b/iu.test(event.line)) ||
+ (event.type === "tool_step" &&
</file context>
| isError: | |
| (event.type === "stderr" && /\b(?:error|fatal|failed|panic)\b/iu.test(event.line)) || | |
| (event.type === "tool_step" && | |
| event.tool_results.some((result) => /^(?:error|failed)$/iu.test(result.status ?? ""))), | |
| isError: | |
| (event.type === "stderr" && /\b(?:error|fatal|failed|panic)\b/iu.test(event.line)) || | |
| (event.type === "tool_step" && | |
| event.tool_results.some((result) => /^(?:error|failed)$/iu.test(result.status ?? ""))) || | |
| (event.type === "turn_committed" && | |
| /^(cancel|interrupt|error|fail|abort|timeout|deadline|terminat|unreadable)/iu.test( | |
| `${event.terminal_reason ?? ""} ${event.turn_kind ?? ""}`, | |
| )) || | |
| (event.type === "ask_result" && | |
| (typeof event.ask.error === "string" || | |
| (typeof event.ask.exit_code === "number" && event.ask.exit_code !== 0) || | |
| /^(cancel|interrupt|error|fail|abort|timeout|deadline|terminat|unreadable)/iu.test( | |
| event.ask.terminal_reason ?? "", | |
| ))), |
| export function logPiEvent(logger: HarnessLogger, event: PiEvent): void { | ||
| const type = String(event.type ?? "unknown"); | ||
| const level = harnessEventLogLevel(type, { | ||
| isError: type === "error" || (type === "tool_execution_end" && event.isError === true), |
There was a problem hiding this comment.
P2: When Pi reports a provider failure through message_end.message.stopReason === "error", this predicate classifies it as level 2. runPiSession records the stop reason without logging a warning on that path, so the default persisted logs hide the failure; classify that assistant message as an error too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/pi-sdk/src/session.ts, line 308:
<comment>When Pi reports a provider failure through `message_end.message.stopReason === "error"`, this predicate classifies it as level 2. `runPiSession` records the stop reason without logging a warning on that path, so the default persisted logs hide the failure; classify that assistant message as an error too.</comment>
<file context>
@@ -302,11 +303,17 @@ export function buildPiTranscript(events: PiEvent[]): string {
export function logPiEvent(logger: HarnessLogger, event: PiEvent): void {
+ const type = String(event.type ?? "unknown");
+ const level = harnessEventLogLevel(type, {
+ isError: type === "error" || (type === "tool_execution_end" && event.isError === true),
+ hasContent: type === "message_end" || type === "tool_execution_end",
+ });
</file context>
| isError: type === "error" || (type === "tool_execution_end" && event.isError === true), | |
| isError: type === "error" || (type === "message_end" && isRecord(event.message) && event.message.stopReason === "error") || (type === "tool_execution_end" && event.isError === true), |
| const level = harnessEventLogLevel(type, { | ||
| isError: | ||
| (type === "result" && message.subtype !== undefined && message.subtype !== "success") || | ||
| message.is_error === true, |
There was a problem hiding this comment.
P2: When a Claude tool returns an error, the SDK places is_error on the nested user.message.content tool-result block, not necessarily on the outer message. Inspect those blocks before classifying the event, otherwise the failure is logged at level 2 and omitted from default persisted logs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/claude-agent-sdk/src/session.ts, line 289:
<comment>When a Claude tool returns an error, the SDK places `is_error` on the nested `user.message.content` tool-result block, not necessarily on the outer message. Inspect those blocks before classifying the event, otherwise the failure is logged at level 2 and omitted from default persisted logs.</comment>
<file context>
@@ -277,11 +282,19 @@ export function buildClaudeCodeTranscript(messages: ClaudeSdkMessage[]): string
+ const level = harnessEventLogLevel(type, {
+ isError:
+ (type === "result" && message.subtype !== undefined && message.subtype !== "success") ||
+ message.is_error === true,
+ hasContent: type === "assistant" || type === "user" || type === "result",
+ });
</file context>
| message.is_error === true, | |
| message.is_error === true || | |
| (type === "user" && | |
| isRecord(message.message) && | |
| Array.isArray(message.message.content) && | |
| message.message.content.some( | |
| (block) => isRecord(block) && block.type === "tool_result" && block.is_error === true, | |
| )), |
| expect( | ||
| logger.lines.find((line) => line.message.startsWith("Started runner-owned"))?.level, | ||
| ).toBe(2); | ||
| expect(logger.lines.some((line) => line.message === "fake tools/call session_info")).toBe( |
There was a problem hiding this comment.
P2: The stderr-based assertions can race: start() resolves as soon as the session_info stdout reply is processed (routeServerLine settles the call), while the "fake tools/call session_info" line is delivered by the separate child.stderr 'data' handler. Nothing waits for stderr to drain, so the assertion on logger.lines can run before the line is captured. Wait for the expected line (bounded) instead of asserting synchronously after start().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/tests/core/stagehand-facade.test.ts, line 238:
<comment>The stderr-based assertions can race: start() resolves as soon as the session_info stdout reply is processed (routeServerLine settles the call), while the "fake tools/call session_info" line is delivered by the separate child.stderr 'data' handler. Nothing waits for stderr to drain, so the assertion on logger.lines can run before the line is captured. Wait for the expected line (bounded) instead of asserting synchronously after start().</comment>
<file context>
@@ -174,6 +212,58 @@ describe("stagehand facade tool surface", () => {
+ expect(
+ logger.lines.find((line) => line.message.startsWith("Started runner-owned"))?.level,
+ ).toBe(2);
+ expect(logger.lines.some((line) => line.message === "fake tools/call session_info")).toBe(
+ true,
+ );
</file context>
| // Runner-side only (absent from tools/list): launches the browser if | ||
| // needed and reports where it lives so the harness can log the | ||
| // Browserbase session URL before the agent's first call. | ||
| const browser = (await ensureResources()).browser; |
There was a problem hiding this comment.
P2: Because the agent relay forwards every tools/call to this dispatcher, omitting session_info from tools/list does not keep it runner-only; an agent can request it and receive the Browserbase session ID. Move this metadata exchange to an authenticated out-of-band runner channel, or reject this name for agent-originated calls instead of relying on discovery omission.
(Based on your team's feedback about runner-side facade session operations.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/stdio-server.ts, line 111:
<comment>Because the agent relay forwards every `tools/call` to this dispatcher, omitting `session_info` from `tools/list` does not keep it runner-only; an agent can request it and receive the Browserbase session ID. Move this metadata exchange to an authenticated out-of-band runner channel, or reject this name for agent-originated calls instead of relying on discovery omission.
(Based on your team's feedback about runner-side facade session operations.) </comment>
<file context>
@@ -103,6 +104,18 @@ server.server.setRequestHandler(CallToolRequestSchema, async (request) => {
+ // Runner-side only (absent from tools/list): launches the browser if
+ // needed and reports where it lives so the harness can log the
+ // Browserbase session URL before the agent's first call.
+ const browser = (await ensureResources()).browser;
+ return textResult(
+ JSON.stringify({
</file context>
The step trace ended at the result line, so the logs showed every action but not what the agent concluded. Emit 'summary · …' and 'answer · …' (full text in auxiliary) before the result line; a missing answer is stated explicitly with the stop status so budget/error stops that never produced a final message are visible instead of silent.
The pi harness replaced pi's entire system prompt with the two-sentence evaluation brief that claude_code only *appends* to its preset, and ran with thinkingLevel "off". Same model, same surface, same tasks: pi scored 22% vs 50-60% on the other harnesses, quitting at a median of 10 steps with partial self-reports. The brief is now appended to pi's stock prompt (getAppendSystemPrompt) and thinking defaults to "medium"; both remain overridable (systemPrompt replaces, EVAL_PI_THINKING sets the level).
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Confidence score: 3/5
- In
packages/integrations/pi-sdk/src/session.ts,runPiSessionoverwrites a resolver-provided thinking level with"medium"beforeloadPiSdkcan apply it, so configured model reasoning behavior may be ignored — forwardthinkingLevelonly when explicitly configured and let the loader retain the resolved value.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/pi-sdk/src/session.ts">
<violation number="1" location="packages/integrations/pi-sdk/src/session.ts:210">
P2: When the model resolver supplies a thinking level, `runPiSession` overwrites it with `"medium"` before `loadPiSdk` can apply it. Forward `thinkingLevel` only when explicitly configured and let the loader retain the resolved level or its medium default.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| ...(input.session.appendSystemPrompt && { | ||
| appendSystemPrompt: input.session.appendSystemPrompt, | ||
| }), | ||
| thinkingLevel: input.session.thinkingLevel ?? DEFAULT_PI_THINKING_LEVEL, |
There was a problem hiding this comment.
P2: When the model resolver supplies a thinking level, runPiSession overwrites it with "medium" before loadPiSdk can apply it. Forward thinkingLevel only when explicitly configured and let the loader retain the resolved level or its medium default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/pi-sdk/src/session.ts, line 210:
<comment>When the model resolver supplies a thinking level, `runPiSession` overwrites it with `"medium"` before `loadPiSdk` can apply it. Forward `thinkingLevel` only when explicitly configured and let the loader retain the resolved level or its medium default.</comment>
<file context>
@@ -187,7 +204,10 @@ export async function runPiSession(input: {
+ ...(input.session.appendSystemPrompt && {
+ appendSystemPrompt: input.session.appendSystemPrompt,
+ }),
+ thinkingLevel: input.session.thinkingLevel ?? DEFAULT_PI_THINKING_LEVEL,
customTools,
});
</file context>
| thinkingLevel: input.session.thinkingLevel ?? DEFAULT_PI_THINKING_LEVEL, | |
| ...(input.session.thinkingLevel && { thinkingLevel: input.session.thinkingLevel }), |
…ath), Best Buy trade-in (date rot) — n 45 → 42
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 5/5
packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.mdcontains contradictory validity and date-rot summaries, which could mislead readers about the benchmark’s current achievability; align the earlier summary and related wording with the audit findings.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md">
<violation number="1" location="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md:66">
P2: This section contradicts the earlier achievability summary, which still calls Macy’s and Best Buy valid and says no task had date rot. Update the earlier summary and related validity wording so the audit does not report conflicting quarantine state.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| |---|---|---| | ||
| | a0a18ca6a352 (Macy's add-to-cart) | bot-wall | Akamai "Access Denied" on every request across 7/7 harness runs (mastra ×2, pi, eve, deepagents, codex, fx) | | ||
| | 7e6993f2c5cd (imgur meme) | environment / facade | Browser session dies in the Picsart iframe upload flow on 3/3 harnesses this run + 4 prior runs; verifier passed one run with status=error and empty answer (FP) | | ||
| | 7e1047f4803237f319c004f7a7f6bccb (Best Buy trade-in) | task_invalid (date rot) | Two independent audits: the specified device (HP, Intel 7th Gen Core i3, 8 GB) no longer exists in Best Buy's trade-in catalog; every harness silently substitutes another device | |
There was a problem hiding this comment.
P2: This section contradicts the earlier achievability summary, which still calls Macy’s and Best Buy valid and says no task had date rot. Update the earlier summary and related validity wording so the audit does not report conflicting quarantine state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md, line 66:
<comment>This section contradicts the earlier achievability summary, which still calls Macy’s and Best Buy valid and says no task had date rot. Update the earlier summary and related validity wording so the audit does not report conflicting quarantine state.</comment>
<file context>
@@ -56,3 +56,11 @@ Never passed by either model (kept valid — content is present; failures are cr
+|---|---|---|
+| a0a18ca6a352 (Macy's add-to-cart) | bot-wall | Akamai "Access Denied" on every request across 7/7 harness runs (mastra ×2, pi, eve, deepagents, codex, fx) |
+| 7e6993f2c5cd (imgur meme) | environment / facade | Browser session dies in the Picsart iframe upload flow on 3/3 harnesses this run + 4 prior runs; verifier passed one run with status=error and empty answer (FP) |
+| 7e1047f4803237f319c004f7a7f6bccb (Best Buy trade-in) | task_invalid (date rot) | Two independent audits: the specified device (HP, Intel 7th Gen Core i3, 8 GB) no longer exists in Best Buy's trade-in catalog; every harness silently substitutes another device |
</file context>
Browserbase's project default session timeout is 15 min, shorter than many agent tasks (codex 7e6993f2 hit TIMED_OUT at 901 s). The facade now creates sessions with timeout = STAGEHAND_BROWSERBASE_SESSION_TIMEOUT_SECONDS (positive int <= 21600, default 3600) and keepAlive: false. Evals sets it from EVAL_BROWSERBASE_SESSION_TIMEOUT_SECONDS (default 3600) in buildStagehandFacadeEnv so every harness inherits it. The stagehand_facade_session_lost telemetry line now carries provider, sessionId, sessionAgeMs and sessionTimeoutMs so a timeout is distinguishable from a remote close.
There was a problem hiding this comment.
3 existing issues remain and 12 new issues found across 57 files (changes from recent commits).
Confidence score: 2/5
- Sensitive data can escape through
packages/integrations/deepagents-sdk/src/session.tsreasoning summaries and raw errors rethrown bypackages/integrations/core/src/facade/tools.ts, exposing credentials, page content, or connection details in trajectories, traces, or callers — classify reasoning as sensitive and sanitize error fields before propagation. packages/evals/framework/verifierGates.tscan treat pre-tool reasoning as browser evidence, allowing prior-knowledge answers to pass grounding checks — restrict evidence to tool output modalities.- Browser session-loss signals can be dropped across
packages/evals/core/contracts/tool.tsandpackages/evals/core/tools/stagehandFacadeBridge.ts, whilepackages/integrations/core/src/facade/tools.tsmisses timeout-based loss; dead sessions may retry or fail to become terminal — forward and detect all terminal loss paths before response handling. - Eval result parsing in
packages/evals/framework/harnesses/externalRunner.tsandpackages/integrations/deepagents-sdk/src/session.tscan accept report-shaped JSON from arbitrary final text or reasoning, creating false passes or incomplete-run results — require explicit report markers and required fields before accepting a result.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/framework/codexRunner.ts">
<violation number="1" location="packages/evals/framework/codexRunner.ts:190">
P2: When a Codex run supplies a custom or isolated `env`, this call still reads `EVAL_REASONING_SUMMARY` from global `process.env`, so that run's reasoning setting can be ignored or replaced by a stale global value. Pass `env` into `buildEvalCodexConfig` so the SDK and its config use the same environment.</violation>
</file>
<file name="packages/evals/framework/verifierAdapter.ts">
<violation number="1" location="packages/evals/framework/verifierAdapter.ts:354">
P2: When the final answer contains only advisory datums such as entities or short integers, this line still emits `answer_grounded=1`, contaminating the metric with rows that had no numeric grounding check. Emit the metric only when `groundedNumeric + ungroundedNumeric > 0`.</violation>
</file>
<file name="packages/evals/framework/harnesses/externalRunner.ts">
<violation number="1" location="packages/evals/framework/harnesses/externalRunner.ts:86">
P2: When a fallback final message contains a legitimate JSON answer or data, `stripEmbeddedJsonObjects` removes it because it accepts every JSON record, leaving the verifier a truncated answer. Strip only report-shaped objects, not arbitrary records.</violation>
<violation number="2" location="packages/evals/framework/harnesses/externalRunner.ts:483">
P2: When an unmarked final message ends with arbitrary JSON containing a boolean `success`, `trailingEvalResultJson` records it as the eval report and can create a false pass. Require at least the report fields (`summary` or `finalAnswer`) before accepting the trailing object.</violation>
</file>
<file name="packages/evals/core/tools/stagehandFacadeBridge.ts">
<violation number="1" location="packages/evals/core/tools/stagehandFacadeBridge.ts:257">
P2: When a legacy facade reports the terminal session-loss tool error only to a runner-issued request, this check never runs because the `evals-facade-*` response path returns first. Parse terminal tool errors before that early return, or the runner can grade a browser-dead run as a normal agent outcome.</violation>
</file>
<file name="packages/evals/framework/mastraRunner.ts">
<violation number="1" location="packages/evals/framework/mastraRunner.ts:100">
P2: When Mastra runs with `model = "mastra/default"`, this helper returns `undefined` even though Mastra normalizes the alias to OpenAI, so the run omits the default reasoning summary. Pass the normalized model to `openAiReasoningProviderOptions` and cover this alias.</violation>
</file>
<file name="packages/sdk-ts/src/stagehand.ts">
<violation number="1" location="packages/sdk-ts/src/stagehand.ts:165">
P2: When callers set `clientTimeoutMs` above `MAX_TIMER_DELAY_MS`, this validation accepts it but `RPCClient` schedules a 1 ms timer. Reject explicit values above `MAX_TIMER_DELAY_MS` so oversized deadlines do not fail immediately.</violation>
</file>
<file name="packages/evals/framework/deepagentsRunner.ts">
<violation number="1" location="packages/evals/framework/deepagentsRunner.ts:114">
P2: When `model` is a bare OpenAI ID such as `gpt-5.4-mini`, this branch sends `reasoningSummary`, but the deepagents runner leaves the ID bare and ignores it. Canonicalize the model consistently before this check and the runner gate, then add regression coverage for bare and prefixed IDs.</violation>
</file>
<file name="packages/integrations/deepagents-sdk/src/session.ts">
<violation number="1" location="packages/integrations/deepagents-sdk/src/session.ts:448">
P1: Reasoning summaries can quote credentials or sensitive page content, but this new `reasoning` field bypasses event redaction and reaches persisted trajectory and trace logs. Classify reasoning as sensitive and sanitize it before retaining or logging the event.</violation>
</file>
<file name="packages/evals/framework/verifierGates.ts">
<violation number="1" location="packages/evals/framework/verifierGates.ts:425">
P1: When an agent states a number in pre-tool reasoning, `toolOutputText` treats that reasoning as browser evidence. Exclude `step.reasoning` modalities or restrict grounding to tool output, otherwise prior-knowledge answers can bypass `ungrounded_answer`.</violation>
</file>
<file name="packages/integrations/core/src/facade/tools.ts">
<violation number="1" location="packages/integrations/core/src/facade/tools.ts:324">
P2: When `onSessionLost` throws, `enqueue` propagates the hook error instead of `StagehandFacadeSessionLostError`, so the first lost-session call is not terminal. Invoke the hook defensively and always rethrow the session-loss error.</violation>
<violation number="2" location="packages/integrations/core/src/facade/tools.ts:371">
P2: When an RPC stops responding before `snapshot` or `screenshot` starts, `sessionLossCause` returns `undefined` for `RPCResponseTimeoutError`, so dead-session calls keep retrying instead of becoming terminal. Handle this timeout as session loss and preserve its timeout duration in the cause.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } { | ||
| const type = String(event.type ?? "unknown"); | ||
| if (type === "assistant" && typeof event.text === "string") { | ||
| const reasoning = typeof event.reasoning === "string" ? event.reasoning : ""; |
There was a problem hiding this comment.
P1: Reasoning summaries can quote credentials or sensitive page content, but this new reasoning field bypasses event redaction and reaches persisted trajectory and trace logs. Classify reasoning as sensitive and sanitize it before retaining or logging the event.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/deepagents-sdk/src/session.ts, line 448:
<comment>Reasoning summaries can quote credentials or sensitive page content, but this new `reasoning` field bypasses event redaction and reaches persisted trajectory and trace logs. Classify reasoning as sensitive and sanitize it before retaining or logging the event.</comment>
<file context>
@@ -442,9 +445,11 @@ export function summarizeDeepagentsEvent(event: DeepagentsEvent): {
} {
const type = String(event.type ?? "unknown");
if (type === "assistant" && typeof event.text === "string") {
+ const reasoning = typeof event.reasoning === "string" ? event.reasoning : "";
+ const text = reasoning ? `${reasoning}\n${event.text}`.trim() : event.text;
return {
</file context>
| function toolOutputText(step: TrajectoryStep): string { | ||
| const parts = [safeStringify(step.toolOutput?.result), step.toolOutput?.error ?? ""]; | ||
| for (const modality of step.agentEvidence?.modalities ?? []) { | ||
| if (modality.type === "text") parts.push(modality.content); |
There was a problem hiding this comment.
P1: When an agent states a number in pre-tool reasoning, toolOutputText treats that reasoning as browser evidence. Exclude step.reasoning modalities or restrict grounding to tool output, otherwise prior-knowledge answers can bypass ungrounded_answer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/verifierGates.ts, line 425:
<comment>When an agent states a number in pre-tool reasoning, `toolOutputText` treats that reasoning as browser evidence. Exclude `step.reasoning` modalities or restrict grounding to tool output, otherwise prior-knowledge answers can bypass `ungrounded_answer`.</comment>
<file context>
@@ -0,0 +1,497 @@
+function toolOutputText(step: TrajectoryStep): string {
+ const parts = [safeStringify(step.toolOutput?.result), step.toolOutput?.error ?? ""];
+ for (const modality of step.agentEvidence?.modalities ?? []) {
+ if (modality.type === "text") parts.push(modality.content);
+ else if (modality.type === "json") parts.push(safeStringify(modality.content));
+ }
</file context>
| apiKey: process.env.OPENAI_API_KEY, | ||
| rawReasoning: process.env.EVAL_CODEX_RAW_REASONING === "true", | ||
| extraConfig, | ||
| extraConfig: buildEvalCodexConfig(extraConfig), |
There was a problem hiding this comment.
P2: When a Codex run supplies a custom or isolated env, this call still reads EVAL_REASONING_SUMMARY from global process.env, so that run's reasoning setting can be ignored or replaced by a stale global value. Pass env into buildEvalCodexConfig so the SDK and its config use the same environment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/codexRunner.ts, line 190:
<comment>When a Codex run supplies a custom or isolated `env`, this call still reads `EVAL_REASONING_SUMMARY` from global `process.env`, so that run's reasoning setting can be ignored or replaced by a stale global value. Pass `env` into `buildEvalCodexConfig` so the SDK and its config use the same environment.</comment>
<file context>
@@ -170,7 +187,7 @@ async function loadEvalCodexSdk(
apiKey: process.env.OPENAI_API_KEY,
rawReasoning: process.env.EVAL_CODEX_RAW_REASONING === "true",
- extraConfig,
+ extraConfig: buildEvalCodexConfig(extraConfig),
});
}
</file context>
| extraConfig: buildEvalCodexConfig(extraConfig), | |
| extraConfig: buildEvalCodexConfig(extraConfig, env), |
| answer_grounded: flag(!gates.grounding.gatesOutcome), | ||
| }), | ||
| }; |
There was a problem hiding this comment.
P2: When the final answer contains only advisory datums such as entities or short integers, this line still emits answer_grounded=1, contaminating the metric with rows that had no numeric grounding check. Emit the metric only when groundedNumeric + ungroundedNumeric > 0.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/verifierAdapter.ts, line 354:
<comment>When the final answer contains only advisory datums such as entities or short integers, this line still emits `answer_grounded=1`, contaminating the metric with rows that had no numeric grounding check. Emit the metric only when `groundedNumeric + ungroundedNumeric > 0`.</comment>
<file context>
@@ -291,6 +336,38 @@ function formatProcessScore(score: number | undefined): string {
+ process_score_lenient: { count: 1, value: gates.processScoreLenient },
+ }),
+ ...(gates.grounding && {
+ answer_grounded: flag(!gates.grounding.gatesOutcome),
+ }),
+ };
</file context>
| answer_grounded: flag(!gates.grounding.gatesOutcome), | |
| }), | |
| }; | |
| ...(gates.grounding && | |
| gates.grounding.groundedNumeric + gates.grounding.ungroundedNumeric > 0 && { | |
| answer_grounded: flag(!gates.grounding.gatesOutcome), | |
| }), |
| export function stripEmbeddedJsonObjects(text: string): string { | ||
| let output = text; | ||
| for (const span of extractJsonObjects(text)) { | ||
| if (isRecordJson(span)) output = output.replace(span, ""); |
There was a problem hiding this comment.
P2: When a fallback final message contains a legitimate JSON answer or data, stripEmbeddedJsonObjects removes it because it accepts every JSON record, leaving the verifier a truncated answer. Strip only report-shaped objects, not arbitrary records.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/externalRunner.ts, line 86:
<comment>When a fallback final message contains a legitimate JSON answer or data, `stripEmbeddedJsonObjects` removes it because it accepts every JSON record, leaving the verifier a truncated answer. Strip only report-shaped objects, not arbitrary records.</comment>
<file context>
@@ -59,6 +64,30 @@ export function parseEvalResult(raw: string): ParsedEvalResult {
+export function stripEmbeddedJsonObjects(text: string): string {
+ let output = text;
+ for (const span of extractJsonObjects(text)) {
+ if (isRecordJson(span)) output = output.replace(span, "");
+ }
+ return output.replace(/\n{3,}/gu, "\n\n");
</file context>
| if (isRecordJson(span)) output = output.replace(span, ""); | |
| if ( | |
| isRecordJson(span) && | |
| /"(?:success|summary|finalAnswer)"\s*:/u.test(span) | |
| ) { | |
| output = output.replace(span, ""); | |
| } |
| mcpServers: toolAdapter?.mcpServers, | ||
| tools: toolAdapter?.tools, | ||
| mcpTimeoutMs: readPositiveIntEnv("EVAL_MASTRA_MCP_TIMEOUT_MS"), | ||
| providerOptions: openAiReasoningProviderOptions(model), |
There was a problem hiding this comment.
P2: When Mastra runs with model = "mastra/default", this helper returns undefined even though Mastra normalizes the alias to OpenAI, so the run omits the default reasoning summary. Pass the normalized model to openAiReasoningProviderOptions and cover this alias.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/mastraRunner.ts, line 100:
<comment>When Mastra runs with `model = "mastra/default"`, this helper returns `undefined` even though Mastra normalizes the alias to OpenAI, so the run omits the default reasoning summary. Pass the normalized model to `openAiReasoningProviderOptions` and cover this alias.</comment>
<file context>
@@ -96,6 +97,7 @@ export async function runMastraAgent({
mcpServers: toolAdapter?.mcpServers,
tools: toolAdapter?.tools,
mcpTimeoutMs: readPositiveIntEnv("EVAL_MASTRA_MCP_TIMEOUT_MS"),
+ providerOptions: openAiReasoningProviderOptions(model),
},
onToolResult: toolAdapter?.onToolResult,
</file context>
| if (!Number.isInteger(clientTimeout) || clientTimeout <= 0) { | ||
| throw new RangeError( | ||
| "stagehand.experimentalBatch() clientTimeoutMs must be a positive integer", | ||
| ); | ||
| } |
There was a problem hiding this comment.
P2: When callers set clientTimeoutMs above MAX_TIMER_DELAY_MS, this validation accepts it but RPCClient schedules a 1 ms timer. Reject explicit values above MAX_TIMER_DELAY_MS so oversized deadlines do not fail immediately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-ts/src/stagehand.ts, line 165:
<comment>When callers set `clientTimeoutMs` above `MAX_TIMER_DELAY_MS`, this validation accepts it but `RPCClient` schedules a 1 ms timer. Reject explicit values above `MAX_TIMER_DELAY_MS` so oversized deadlines do not fail immediately.</comment>
<file context>
@@ -151,18 +159,35 @@ export class Stagehand {
+ const clientTimeout =
+ options.clientTimeoutMs ??
+ Math.min(timeout + CALLBACK_BATCH_CLIENT_GRACE_MS, MAX_TIMER_DELAY_MS);
+ if (!Number.isInteger(clientTimeout) || clientTimeout <= 0) {
+ throw new RangeError(
+ "stagehand.experimentalBatch() clientTimeoutMs must be a positive integer",
</file context>
| if (!Number.isInteger(clientTimeout) || clientTimeout <= 0) { | |
| throw new RangeError( | |
| "stagehand.experimentalBatch() clientTimeoutMs must be a positive integer", | |
| ); | |
| } | |
| if ( | |
| !Number.isInteger(clientTimeout) || | |
| clientTimeout <= 0 || | |
| clientTimeout > MAX_TIMER_DELAY_MS | |
| ) { | |
| throw new RangeError( | |
| `stagehand.experimentalBatch() clientTimeoutMs must be a positive integer not exceeding ${MAX_TIMER_DELAY_MS} milliseconds`, | |
| ); | |
| } |
| ...(toolAdapter?.env && { env: toolAdapter.env }), | ||
| ...(toolAdapter?.mcpServers && { mcpServers: toolAdapter.mcpServers }), | ||
| systemPrompt: buildDeepagentsSystemPrompt(toolAdapter?.toolSurface), | ||
| ...(isOpenAiModel(model) && { reasoningSummary: readReasoningSummary() }), |
There was a problem hiding this comment.
P2: When model is a bare OpenAI ID such as gpt-5.4-mini, this branch sends reasoningSummary, but the deepagents runner leaves the ID bare and ignores it. Canonicalize the model consistently before this check and the runner gate, then add regression coverage for bare and prefixed IDs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/deepagentsRunner.ts, line 114:
<comment>When `model` is a bare OpenAI ID such as `gpt-5.4-mini`, this branch sends `reasoningSummary`, but the deepagents runner leaves the ID bare and ignores it. Canonicalize the model consistently before this check and the runner gate, then add regression coverage for bare and prefixed IDs.</comment>
<file context>
@@ -110,6 +111,7 @@ export async function runDeepagentsAgent({
...(toolAdapter?.env && { env: toolAdapter.env }),
...(toolAdapter?.mcpServers && { mcpServers: toolAdapter.mcpServers }),
systemPrompt: buildDeepagentsSystemPrompt(toolAdapter?.toolSurface),
+ ...(isOpenAiModel(model) && { reasoningSummary: readReasoningSummary() }),
recursionLimit: readDeepagentsRecursionLimit(),
maxToolSteps: readDeepagentsMaxToolSteps(),
</file context>
| const cause = sessionLossCause(error); | ||
| if (cause === undefined) throw error; | ||
| this.loss = { cause, tool, at: new Date().toISOString() }; | ||
| this.options.onSessionLost?.(this.loss); |
There was a problem hiding this comment.
P2: When onSessionLost throws, enqueue propagates the hook error instead of StagehandFacadeSessionLostError, so the first lost-session call is not terminal. Invoke the hook defensively and always rethrow the session-loss error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/tools.ts, line 324:
<comment>When `onSessionLost` throws, `enqueue` propagates the hook error instead of `StagehandFacadeSessionLostError`, so the first lost-session call is not terminal. Invoke the hook defensively and always rethrow the session-loss error.</comment>
<file context>
@@ -275,8 +312,20 @@ export class StagehandFacadeTools {
+ const cause = sessionLossCause(error);
+ if (cause === undefined) throw error;
+ this.loss = { cause, tool, at: new Date().toISOString() };
+ this.options.onSessionLost?.(this.loss);
+ throw new StagehandFacadeSessionLostError(this.loss);
+ }
</file context>
| this.options.onSessionLost?.(this.loss); | |
| try { | |
| this.options.onSessionLost?.(this.loss); | |
| } catch { | |
| // Keep telemetry hooks from replacing the terminal session-loss error. | |
| } |
| function sessionLossCause(error: unknown): string | undefined { | ||
| if (!(error instanceof Error)) return undefined; | ||
| if ((error as { facadeExecutionError?: boolean }).facadeExecutionError) return undefined; | ||
| switch (error.name) { |
There was a problem hiding this comment.
P2: When an RPC stops responding before snapshot or screenshot starts, sessionLossCause returns undefined for RPCResponseTimeoutError, so dead-session calls keep retrying instead of becoming terminal. Handle this timeout as session loss and preserve its timeout duration in the cause.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/tools.ts, line 371:
<comment>When an RPC stops responding before `snapshot` or `screenshot` starts, `sessionLossCause` returns `undefined` for `RPCResponseTimeoutError`, so dead-session calls keep retrying instead of becoming terminal. Handle this timeout as session loss and preserve its timeout duration in the cause.</comment>
<file context>
@@ -285,6 +334,57 @@ export class StagehandFacadeTools {
+function sessionLossCause(error: unknown): string | undefined {
+ if (!(error instanceof Error)) return undefined;
+ if ((error as { facadeExecutionError?: boolean }).facadeExecutionError) return undefined;
+ switch (error.name) {
+ case "StagehandBatchTimeoutError": {
+ const { clientTimeout } = error as Error & { clientTimeout?: number };
</file context>
| switch (error.name) { | |
| if (error.name === "RPCResponseTimeoutError") { | |
| const { method, timeoutMs } = error as Error & { | |
| method?: string; | |
| timeoutMs?: number; | |
| }; | |
| return typeof timeoutMs === "number" | |
| ? `${method ?? "RPC"} received no response within ${timeoutMs}ms` | |
| : "RPC response timed out"; | |
| } | |
| switch (error.name) { |
eve's generated agent hardcoded openai/anthropic/google and threw for every
other creator, so AI-Gateway OSS models (alibaba/qwen3.8-flash, zai/glm-*,
deepseek/*) never reached a model. Ids of any other creator now resolve to
gateway("creator/model") from ai; an explicit gateway/ prefix forces the
gateway for first-party creators too.
…catalog eve derives its compaction threshold from AI Gateway context-window metadata bundled with the release; models newer than eve 0.29.4 (alibaba/qwen3.8-flash) are absent and the agent fails to compile. Gateway models now set modelContextWindowTokens (default 128k, EVAL_EVE_MODEL_CONTEXT_WINDOW_TOKENS to override), which bypasses the lookup. Verified with eve build.
A judge pass overridden by a deterministic gate previously surfaced the agent's confident self-report as the row error, which read as a verifier contradiction in Braintrust. The error now leads with the gate and its reason, with the agent's claim attached.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 4/5
- In
packages/evals/framework/verifierAdapter.ts, process mode can report an outcome-gate failure even when a strict process score below 0.8 is the actual failure condition; this makes evaluation diagnostics misleading. Update the failure-cause selection so it reflects the process-mode rule.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/framework/verifierAdapter.ts">
<violation number="1" location="packages/evals/framework/verifierAdapter.ts:301">
P2: When `EVAL_SUCCESS_MODE=process` and a judge pass has an outcome gate plus a strict process score below 0.8, this branch reports the gate as the failure cause even though process mode ignores outcome gates. Only use this diagnostic when the selected success mode includes outcome evaluation.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| processScore: evaluationResult.processScore, | ||
| error: verifiedSuccess | ||
| ? undefined | ||
| : gates.outcomeGates.length > 0 && gates.judgeOutcomeSuccess |
There was a problem hiding this comment.
P2: When EVAL_SUCCESS_MODE=process and a judge pass has an outcome gate plus a strict process score below 0.8, this branch reports the gate as the failure cause even though process mode ignores outcome gates. Only use this diagnostic when the selected success mode includes outcome evaluation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/verifierAdapter.ts, line 301:
<comment>When `EVAL_SUCCESS_MODE=process` and a judge pass has an outcome gate plus a strict process score below 0.8, this branch reports the gate as the failure cause even though process mode ignores outcome gates. Only use this diagnostic when the selected success mode includes outcome evaluation.</comment>
<file context>
@@ -296,7 +296,14 @@ export async function gradeExternalTrajectory({
- error: verifiedSuccess ? undefined : (baseResult.error ?? errorMessage),
+ error: verifiedSuccess
+ ? undefined
+ : gates.outcomeGates.length > 0 && gates.judgeOutcomeSuccess
+ ? // The judge passed this row; a deterministic gate flipped it. Say
+ // so where the row error is read, instead of echoing the agent's
</file context>
A factually correct answer sourced from a search snippet counts as a pass (owner decision on the F1 race-time row). The check still runs and is recorded as `grounding` + metric answer_grounded so snippet-sourced passes remain filterable; EVAL_REQUIRE_GROUNDING=1 opts into gating.
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Confidence score: 3/5
packages/evals/framework/verifierGates.tschangesresolveRequireGroundingso an unsetEVAL_REQUIRE_GROUNDINGdisables grounding for every dataset, conflicting with the function contract and verifier-gates documentation and potentially weakening evaluation checks; restore the documented default-on behavior or update the contract and all dependent expectations.packages/evals/tests/framework/gradeExternalTrajectory.test.tssilently depends onEVAL_REQUIRE_GROUNDINGnot being set, so developers with it enabled may not exercise the advisory path as intended; explicitly control the environment in the test and cover both configuration modes.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/tests/framework/gradeExternalTrajectory.test.ts">
<violation number="1" location="packages/evals/tests/framework/gradeExternalTrajectory.test.ts:184">
P3: The advisory half of this test silently depends on EVAL_REQUIRE_GROUNDING not being set to "1"/"true" in the environment. When a developer runs the suite with EVAL_REQUIRE_GROUNDING=1 configured, the first `run("hardbenchmark")` gates, `_success` is false, and the advisory assertions fail; the finally block then deletes the variable instead of restoring it. Save and clear the variable (or set it to "0") before the advisory run alongside the existing beforeEach env handling, and restore it in afterEach.</violation>
</file>
<file name="packages/evals/framework/verifierGates.ts">
<violation number="1" location="packages/evals/framework/verifierGates.ts:179">
P3: When `EVAL_REQUIRE_GROUNDING` is unset, `resolveRequireGrounding` now always disables grounding for every dataset, but the function contract and verifier-gates documentation still promise default-on behavior for precomputed-rubric and HardBench/WebTailBench tasks. Update the JSDoc and documentation to state the advisory default, or restore the documented dataset-based default.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| logger: new EvalLogger(false), | ||
| }); | ||
|
|
||
| // Default: a correct answer sourced from a search snippet still passes; |
There was a problem hiding this comment.
P3: The advisory half of this test silently depends on EVAL_REQUIRE_GROUNDING not being set to "1"/"true" in the environment. When a developer runs the suite with EVAL_REQUIRE_GROUNDING=1 configured, the first run("hardbenchmark") gates, _success is false, and the advisory assertions fail; the finally block then deletes the variable instead of restoring it. Save and clear the variable (or set it to "0") before the advisory run alongside the existing beforeEach env handling, and restore it in afterEach.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/tests/framework/gradeExternalTrajectory.test.ts, line 184:
<comment>The advisory half of this test silently depends on EVAL_REQUIRE_GROUNDING not being set to "1"/"true" in the environment. When a developer runs the suite with EVAL_REQUIRE_GROUNDING=1 configured, the first `run("hardbenchmark")` gates, `_success` is false, and the advisory assertions fail; the finally block then deletes the variable instead of restoring it. Save and clear the variable (or set it to "0") before the advisory run alongside the existing beforeEach env handling, and restore it in afterEach.</comment>
<file context>
@@ -181,20 +181,21 @@ describe("gradeExternalTrajectory", () => {
- );
-
- process.env.EVAL_REQUIRE_GROUNDING = "0";
+ // Default: a correct answer sourced from a search snippet still passes;
+ // the grounding result is recorded so snippet-sourced passes stay filterable.
+ const advisory = await run("hardbenchmark");
</file context>
| // remain filterable; opt into gating with EVAL_REQUIRE_GROUNDING=1. | ||
| void dataset; | ||
| void hasPrecomputedRubric; | ||
| return false; |
There was a problem hiding this comment.
P3: When EVAL_REQUIRE_GROUNDING is unset, resolveRequireGrounding now always disables grounding for every dataset, but the function contract and verifier-gates documentation still promise default-on behavior for precomputed-rubric and HardBench/WebTailBench tasks. Update the JSDoc and documentation to state the advisory default, or restore the documented dataset-based default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/verifierGates.ts, line 179:
<comment>When `EVAL_REQUIRE_GROUNDING` is unset, `resolveRequireGrounding` now always disables grounding for every dataset, but the function contract and verifier-gates documentation still promise default-on behavior for precomputed-rubric and HardBench/WebTailBench tasks. Update the JSDoc and documentation to state the advisory default, or restore the documented dataset-based default.</comment>
<file context>
@@ -170,7 +169,14 @@ export function resolveRequireGrounding(
+ // remain filterable; opt into gating with EVAL_REQUIRE_GROUNDING=1.
+ void dataset;
+ void hasPrecomputedRubric;
+ return false;
}
</file context>
runExternalHarnessTask now times the agent session, the terminal evidence capture and the verifier separately (agent_wall_ms, evidence_ms, verifier_wall_ms, total_wall_ms metrics; agent_wall_ms on the row) so agent speed is never confounded with grading time. The trace result line carries agent=NNs and a timing line follows the verifier.
Each harness SDK lays out token buckets differently: OpenAI-style SDKs (codex, mastra, eve, deepagents, fx) report cached tokens as a subset of input, the Claude Agent SDK reports cache reads/writes outside input_tokens, pi reports only the uncached remainder as input, and cursor reports nothing. normalizeUsage maps every harness onto input_total / input_cached / input_uncached / output / reasoning with the convention recorded, and the runner emits usage_input_total, usage_input_cached, usage_output and usage_reasoning on every row (harness_* metrics unchanged). The trace result line now reads in=<total> (cached <n>) out=<n>.
…ce map pricing/pricing.json snapshots USD-per-million list prices for the curated eval model set (scripts/update-pricing.ts pulls them from the public Vercel AI Gateway model list, OpenRouter as fallback). EAP / codename models are kept at null with "needs owner input" so they surface as unpriced, never $0. estimateCost prices the normalized buckets: uncached at input, cached at the cache-read rate, cache writes at the write rate where the provider bills one, output (with reasoning inside it) at output. Model ids resolve through aliases (gateway/ prefix, codex/default, dashed vs dotted versions, -preview/date/-eap suffixes, xai/spacexai), never onto a sibling model. Rows get cost_source and cost_usd_estimated; metrics carry cost_usd_estimated next to the harness-reported cost_usd_reported. Unpriced models log a level-1 line naming the model.
…a on FP-prone tasks Two-sided verdict audits (65 + 39 + 10 rows across 8 harnesses incl. the native v3 agent) found every false positive on five tasks where the judge credited constraint claims (gluten-free, diploma-only, Best Sellers, 'check the actual product page', seat fee) that the trajectory never confirmed. Ten criteria now state the evidence requirement and that an unverified claim presented as confirmed earns no credit.
…als (native-path parity) The native Stagehand agent path creates Browserbase sessions with proxies:true and browserSettings.verified:true; the facade path passed neither, so every external-harness browser was unproxied and unverified. Macy's blocked 7/7 facade runs while the native browser (and the verified reachability probe) got through — an environment failure that was really a harness confound. Facade launch options now pass proxies/verified through (STAGEHAND_BROWSERBASE_PROXIES/_VERIFIED); evals set both on by default (EVAL_BROWSERBASE_PROXIES/_VERIFIED=0 to disable). Macy's leaves quarantine (n 42 → 43).
…t abort codex exec only reports usage on turn.completed, which never arrives when the tool-step budget (or the caller's signal) aborts the turn, so max_turns runs recorded 0/0 tokens (7e1047f4: 100 steps, 2M+ tokens unrecorded). The session now reads the last cumulative token_count from the thread's rollout file under the per-run CODEX_HOME and reports usageSource (turn_completed | rollout | none); the runner emits codex_usage_recovered. The implementation itself was swept into 2acb9ab from the shared working tree; this commit adds its tests.
The Codex SDK (0.147) streams codex exec --json, whose only usage record is turn.completed; aborting on the tool-step budget kills the process before it arrives, so budget-exhausted rows showed 0 tokens and a $0 estimate. The session now recovers the cumulative token_count from the thread's rollout under the per-run CODEX_HOME (landed in 2acb9ab / c337309); this commit closes the remaining hole: when nothing is recoverable the row's usage is flagged reported:false, normalizes to the unreported convention, carries no usage_* metrics and prices as cost_source no_usage instead of real zeros. cursor's never-reported usage rides the same flag.
There was a problem hiding this comment.
9 issues found across 43 files (changes from recent commits).
Confidence score: 3/5
packages/evals/core/tools/stagehand_facade.tsexposes raw environment-variable values throughevalBooleanEnverrors, potentially leaking configuration data; sanitize the value before raising the error.- The pricing and usage paths can produce materially incorrect cost data:
packages/integrations/codex-sdk/src/session.tstreats all-zero usage as measured, whilepackages/evals/pricing/pricing.jsondisagrees with the UI’s Grok 4.5 cached-token rate; reject unknown totals and align the pricing source. packages/evals/scripts/update-pricing.tscan replace the existing snapshot with an all-unpriced map when catalogs are empty or malformed, and its alias/conversion paths lack focused coverage; validate catalogs before writing and add mocked gateway/OpenRouter tests.packages/evals/framework/stepBudget.tsaccepts malformed numeric values, whilepackages/evals/framework/harnesses/externalRunner.tsundercounts wall time and omitsprices_as_of; validate full integers, measure from run completion, and preserve pricing provenance.packages/evals/tests/framework/codexRunner.test.tsalso leaves temporary directories behind, so clean them up infinally.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/codex-sdk/src/session.ts">
<violation number="1" location="packages/integrations/codex-sdk/src/session.ts:325">
P2: When a rollout contains an all-zero `total_token_usage`, this return marks unknown usage as `rollout`; the eval runner then reports it as measured and can estimate a zero cost. Reject all-zero extracted totals so the result remains `usageSource: "none"`.</violation>
</file>
<file name="packages/evals/pricing/pricing.json">
<violation number="1" location="packages/evals/pricing/pricing.json:18">
P2: When a Grok 4.5 run has cached tokens, this map estimates them at $0.30/M while the UI publisher recalculates the same run at $0.50/M. Align this entry with the existing table or make the UI publisher consume this versioned map so reported costs agree.</violation>
</file>
<file name="packages/evals/scripts/update-pricing.ts">
<violation number="1" location="packages/evals/scripts/update-pricing.ts:62">
P3: The new catalog parsing and alias paths have no focused tests, so an upstream schema or alias change can silently rewrite the shipped map. Add mocked gateway/OpenRouter tests covering unit conversion, aliases, missing records, and EAP redaction.
(Based on your team's feedback about adding focused tests for new behavior.)</violation>
<violation number="2" location="packages/evals/scripts/update-pricing.ts:67">
P2: When either catalog returns a successful but empty or malformed model list, this path writes an all-unpriced `pricing.json` and discards the previous snapshot. Reject invalid catalogs before generating the file, and preserve the last snapshot until a valid catalog is available.</violation>
</file>
<file name="packages/evals/tests/framework/codexRunner.test.ts">
<violation number="1" location="packages/evals/tests/framework/codexRunner.test.ts:233">
P3: Each run leaks a `codex-home-*` directory under os.tmpdir(). The finally blocks restore the env var but never remove the temp dir, so repeated suite runs accumulate junk in the OS temp. Remove the directory in the finally block after the assertions.</violation>
</file>
<file name="packages/evals/framework/stepBudget.ts">
<violation number="1" location="packages/evals/framework/stepBudget.ts:48">
P2: When a step-budget environment variable contains a malformed value such as `12junk` or `1.5`, `readPositiveInt` silently truncates it and runs with an unintended budget. Validate the entire value as a positive integer before accepting it, so invalid configuration falls through to the next precedence level.</violation>
</file>
<file name="packages/evals/framework/harnesses/externalRunner.ts">
<violation number="1" location="packages/evals/framework/harnesses/externalRunner.ts:322">
P2: When an estimate is emitted, the runner drops `cost.prices_as_of`, so result rows cannot be reproduced against the versioned price map. Preserve the price-map date alongside `priced_with`.</violation>
<violation number="2" location="packages/evals/framework/harnesses/externalRunner.ts:482">
P2: When verifier timing is reported, `total_wall_ms` undercounts end-to-end latency because it omits processing between agent exit and evidence capture. Measure total from a run-level end timestamp, or rename this metric to the phase sum.</violation>
</file>
<file name="packages/evals/core/tools/stagehand_facade.ts">
<violation number="1" location="packages/evals/core/tools/stagehand_facade.ts:284">
P2: Custom agent: **Exception and error message sanitization**
`evalBooleanEnv` reflects the raw environment-variable value in a raised error message. Rule 31c8a33a requires sanitized errors that never expose env vars; use a fixed message instead of interpolating `raw`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const total = payload.info.total_token_usage; | ||
| if (isRecord(total)) latest = total; | ||
| } | ||
| return latest ? extractCodexTokenUsage(latest) : undefined; |
There was a problem hiding this comment.
P2: When a rollout contains an all-zero total_token_usage, this return marks unknown usage as rollout; the eval runner then reports it as measured and can estimate a zero cost. Reject all-zero extracted totals so the result remains usageSource: "none".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/codex-sdk/src/session.ts, line 325:
<comment>When a rollout contains an all-zero `total_token_usage`, this return marks unknown usage as `rollout`; the eval runner then reports it as measured and can estimate a zero cost. Reject all-zero extracted totals so the result remains `usageSource: "none"`.</comment>
<file context>
@@ -209,16 +233,98 @@ export async function runCodexSession(input: {
+ const total = payload.info.total_token_usage;
+ if (isRecord(total)) latest = total;
+ }
+ return latest ? extractCodexTokenUsage(latest) : undefined;
+}
+
</file context>
| return latest ? extractCodexTokenUsage(latest) : undefined; | |
| if (!latest) return undefined; | |
| const usage = extractCodexTokenUsage(latest); | |
| return Object.values(usage).some((value) => value > 0) ? usage : undefined; |
| }, | ||
| "anthropic/claude-sonnet-4.6": { | ||
| "input_per_m": 3, | ||
| "cached_input_per_m": 0.3, |
There was a problem hiding this comment.
P2: When a Grok 4.5 run has cached tokens, this map estimates them at $0.30/M while the UI publisher recalculates the same run at $0.50/M. Align this entry with the existing table or make the UI publisher consume this versioned map so reported costs agree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/pricing/pricing.json, line 18:
<comment>When a Grok 4.5 run has cached tokens, this map estimates them at $0.30/M while the UI publisher recalculates the same run at $0.50/M. Align this entry with the existing table or make the UI publisher consume this versioned map so reported costs agree.</comment>
<file context>
@@ -0,0 +1,146 @@
+ },
+ "anthropic/claude-sonnet-4.6": {
+ "input_per_m": 3,
+ "cached_input_per_m": 0.3,
+ "cache_write_input_per_m": 3.75,
+ "output_per_m": 15,
</file context>
| if (!response.ok) throw new Error(`gateway models: HTTP ${response.status}`); | ||
| const body = (await response.json()) as { data?: Array<Record<string, unknown>> }; | ||
| const prices = new Map<string, FetchedPrice>(); | ||
| for (const model of body.data ?? []) { |
There was a problem hiding this comment.
P2: When either catalog returns a successful but empty or malformed model list, this path writes an all-unpriced pricing.json and discards the previous snapshot. Reject invalid catalogs before generating the file, and preserve the last snapshot until a valid catalog is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/scripts/update-pricing.ts, line 67:
<comment>When either catalog returns a successful but empty or malformed model list, this path writes an all-unpriced `pricing.json` and discards the previous snapshot. Reject invalid catalogs before generating the file, and preserve the last snapshot until a valid catalog is available.</comment>
<file context>
@@ -0,0 +1,183 @@
+ if (!response.ok) throw new Error(`gateway models: HTTP ${response.status}`);
+ const body = (await response.json()) as { data?: Array<Record<string, unknown>> };
+ const prices = new Map<string, FetchedPrice>();
+ for (const model of body.data ?? []) {
+ const pricing = model.pricing as Record<string, unknown> | undefined;
+ const id = typeof model.id === "string" ? model.id : undefined;
</file context>
| } | ||
|
|
||
| function readPositiveInt(raw: string | undefined): number | undefined { | ||
| const parsed = Number.parseInt(raw ?? "", 10); |
There was a problem hiding this comment.
P2: When a step-budget environment variable contains a malformed value such as 12junk or 1.5, readPositiveInt silently truncates it and runs with an unintended budget. Validate the entire value as a positive integer before accepting it, so invalid configuration falls through to the next precedence level.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/stepBudget.ts, line 48:
<comment>When a step-budget environment variable contains a malformed value such as `12junk` or `1.5`, `readPositiveInt` silently truncates it and runs with an unintended budget. Validate the entire value as a positive integer before accepting it, so invalid configuration falls through to the next precedence level.</comment>
<file context>
@@ -0,0 +1,50 @@
+}
+
+function readPositiveInt(raw: string | undefined): number | undefined {
+ const parsed = Number.parseInt(raw ?? "", 10);
+ return Number.isFinite(parsed) && parsed > 0 ? parsed : undefined;
+}
</file context>
| agent_wall_ms: metricValue(timing.agentWallMs), | ||
| evidence_ms: metricValue(timing.evidenceMs), | ||
| verifier_wall_ms: metricValue(timing.verifierWallMs), | ||
| total_wall_ms: metricValue(timing.agentWallMs + timing.evidenceMs + timing.verifierWallMs), |
There was a problem hiding this comment.
P2: When verifier timing is reported, total_wall_ms undercounts end-to-end latency because it omits processing between agent exit and evidence capture. Measure total from a run-level end timestamp, or rename this metric to the phase sum.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/externalRunner.ts, line 482:
<comment>When verifier timing is reported, `total_wall_ms` undercounts end-to-end latency because it omits processing between agent exit and evidence capture. Measure total from a run-level end timestamp, or rename this metric to the phase sum.</comment>
<file context>
@@ -382,6 +469,50 @@ function isSessionLostToolOutput(
+ agent_wall_ms: metricValue(timing.agentWallMs),
+ evidence_ms: metricValue(timing.evidenceMs),
+ verifier_wall_ms: metricValue(timing.verifierWallMs),
+ total_wall_ms: metricValue(timing.agentWallMs + timing.evidenceMs + timing.verifierWallMs),
+ };
+}
</file context>
| agent_wall_ms: Math.round(agentWallMs), | ||
| cost_source: cost.cost_source, | ||
| ...(cost.cost_usd_estimated !== undefined && { cost_usd_estimated: cost.cost_usd_estimated }), | ||
| ...(cost.priced_with && { priced_with: cost.priced_with }), |
There was a problem hiding this comment.
P2: When an estimate is emitted, the runner drops cost.prices_as_of, so result rows cannot be reproduced against the versioned price map. Preserve the price-map date alongside priced_with.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/externalRunner.ts, line 322:
<comment>When an estimate is emitted, the runner drops `cost.prices_as_of`, so result rows cannot be reproduced against the versioned price map. Preserve the price-map date alongside `priced_with`.</comment>
<file context>
@@ -263,39 +315,52 @@ export async function runExternalHarnessTask<TRaw>({
+ agent_wall_ms: Math.round(agentWallMs),
+ cost_source: cost.cost_source,
+ ...(cost.cost_usd_estimated !== undefined && { cost_usd_estimated: cost.cost_usd_estimated }),
+ ...(cost.priced_with && { priced_with: cost.priced_with }),
// Deprecated compatibility aliases; consumers should use the normalized
// harnessStatus / harnessStopReason fields for newly registered harnesses.
</file context>
| ...(cost.priced_with && { priced_with: cost.priced_with }), | |
| ...(cost.priced_with && { priced_with: cost.priced_with }), | |
| ...(cost.prices_as_of && { prices_as_of: cost.prices_as_of }), |
| if (!v) return fallback; | ||
| if (v === "1" || v === "true" || v === "yes" || v === "on") return true; | ||
| if (v === "0" || v === "false" || v === "no" || v === "off") return false; | ||
| throw new EvalsError(`Expected a boolean env value, got "${raw}".`); |
There was a problem hiding this comment.
P2: Custom agent: Exception and error message sanitization
evalBooleanEnv reflects the raw environment-variable value in a raised error message. Rule 31c8a33a requires sanitized errors that never expose env vars; use a fixed message instead of interpolating raw.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/core/tools/stagehand_facade.ts, line 284:
<comment>`evalBooleanEnv` reflects the raw environment-variable value in a raised error message. Rule 31c8a33a requires sanitized errors that never expose env vars; use a fixed message instead of interpolating `raw`.</comment>
<file context>
@@ -269,3 +275,11 @@ export class StagehandFacadeLegacyTool extends StagehandFacadeTool {
+ if (!v) return fallback;
+ if (v === "1" || v === "true" || v === "yes" || v === "on") return true;
+ if (v === "0" || v === "false" || v === "no" || v === "off") return false;
+ throw new EvalsError(`Expected a boolean env value, got "${raw}".`);
+}
</file context>
|
|
||
| type FetchedPrice = Omit<ModelPrice, "source"> & { source: string }; | ||
|
|
||
| async function fetchGatewayPrices(): Promise<Map<string, FetchedPrice>> { |
There was a problem hiding this comment.
P3: The new catalog parsing and alias paths have no focused tests, so an upstream schema or alias change can silently rewrite the shipped map. Add mocked gateway/OpenRouter tests covering unit conversion, aliases, missing records, and EAP redaction.
(Based on your team's feedback about adding focused tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/scripts/update-pricing.ts, line 62:
<comment>The new catalog parsing and alias paths have no focused tests, so an upstream schema or alias change can silently rewrite the shipped map. Add mocked gateway/OpenRouter tests covering unit conversion, aliases, missing records, and EAP redaction.
(Based on your team's feedback about adding focused tests for new behavior.) </comment>
<file context>
@@ -0,0 +1,183 @@
+
+type FetchedPrice = Omit<ModelPrice, "source"> & { source: string };
+
+async function fetchGatewayPrices(): Promise<Map<string, FetchedPrice>> {
+ const response = await fetch(GATEWAY_MODELS_URL);
+ if (!response.ok) throw new Error(`gateway models: HTTP ${response.status}`);
</file context>
| }); | ||
|
|
||
| it("recovers usage from the isolated CODEX_HOME rollout when the step budget aborts the turn", async () => { | ||
| const codexHome = await fsp.mkdtemp(path.join(os.tmpdir(), "codex-home-")); |
There was a problem hiding this comment.
P3: Each run leaks a codex-home-* directory under os.tmpdir(). The finally blocks restore the env var but never remove the temp dir, so repeated suite runs accumulate junk in the OS temp. Remove the directory in the finally block after the assertions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/tests/framework/codexRunner.test.ts, line 233:
<comment>Each run leaks a `codex-home-*` directory under os.tmpdir(). The finally blocks restore the env var but never remove the temp dir, so repeated suite runs accumulate junk in the OS temp. Remove the directory in the finally block after the assertions.</comment>
<file context>
@@ -225,4 +228,115 @@ describe("codex runner helpers", () => {
});
+
+ it("recovers usage from the isolated CODEX_HOME rollout when the step budget aborts the turn", async () => {
+ const codexHome = await fsp.mkdtemp(path.join(os.tmpdir(), "codex-home-"));
+ const sessionsDir = path.join(codexHome, "sessions", "2026", "08", "31");
+ await fsp.mkdir(sessionsDir, { recursive: true });
</file context>
… rate)
One cost column, cost_usd = what the cell actually billed, with cost_source
("reported" | "computed" | "unavailable") and billing_channel on the row.
Precedence: harness-reported dollars (claude_code total_cost_usd →
anthropic_api, eve costUsd → ai_gateway, pi cost → pi_catalog, fx total_cost
→ fx_gateway); otherwise, for harnesses that call the provider API directly
with our key (codex, mastra, deepagents, and eve/pi when they did not report),
normalized tokens × provider list price from pricing.json → "computed",
channel "<provider>_api"; otherwise (cursor / claude_code on subscription,
unpriced model, unreported usage) no cost_usd metric and "unavailable".
No aliases, ratios or duplicate emissions; token efficiency stays on the
usage_* metrics.
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
Confidence score: 2/5
packages/evals/framework/costEstimate.tscan misclassify Eve gateway or non-first-party traffic as direct-provider spend and emit an incorrect computed cost, which risks inaccurate billing; determine Eve’s actual route before applying provider-cost estimation.packages/evals/framework/costEstimate.tscan emit a misleading $0 cost when direct-provider usage is absent, obscuring unreported spend; propagate an explicit usage-present signal and preserve the unreported state.packages/evals/framework/costEstimate.tslabels FX’s unreported gateway bill as<provider>_api, weakening billing attribution; returnfx_gatewayfor FX’s unreported path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/framework/costEstimate.ts">
<violation number="1" location="packages/evals/framework/costEstimate.ts:66">
P1: When Eve uses `gateway/...` or a non-first-party creator and no gateway cost is reported, this classifies gateway spend as direct provider spend and emits a computed cost. Determine Eve's actual route before adding it to the direct-provider set; gateway-routed cells must remain unavailable without a reported gateway bill.</violation>
<violation number="2" location="packages/evals/framework/costEstimate.ts:101">
P2: When FX has no reported `total_cost`, `resolveBilledCost` labels the unavailable bill as `<provider>_api` even though FX always uses its gateway. Return `fx_gateway` for FX's unreported path so billing metadata identifies the actual channel.</violation>
<violation number="3" location="packages/evals/framework/costEstimate.ts:106">
P2: When a direct-provider harness returns no usage, this guard still computes and emits a zero-dollar cost because `unreported` is not set for Eve, Mastra, or Deep Agents. Propagate an explicit usage-present signal and return `unavailable` when the SDK supplied no usage.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "codex", | ||
| "mastra", | ||
| "deepagents", | ||
| "eve", |
There was a problem hiding this comment.
P1: When Eve uses gateway/... or a non-first-party creator and no gateway cost is reported, this classifies gateway spend as direct provider spend and emits a computed cost. Determine Eve's actual route before adding it to the direct-provider set; gateway-routed cells must remain unavailable without a reported gateway bill.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/costEstimate.ts, line 66:
<comment>When Eve uses `gateway/...` or a non-first-party creator and no gateway cost is reported, this classifies gateway spend as direct provider spend and emits a computed cost. Determine Eve's actual route before adding it to the direct-provider set; gateway-routed cells must remain unavailable without a reported gateway bill.</comment>
<file context>
@@ -19,49 +19,120 @@ export interface PriceMap {
+ "codex",
+ "mastra",
+ "deepagents",
+ "eve",
+ "pi",
+]);
</file context>
| const channel = SUBSCRIPTION_HARNESSES.has(harness) | ||
| ? "subscription" | ||
| : provider | ||
| ? `${provider}_api` | ||
| : "none"; |
There was a problem hiding this comment.
P2: When FX has no reported total_cost, resolveBilledCost labels the unavailable bill as <provider>_api even though FX always uses its gateway. Return fx_gateway for FX's unreported path so billing metadata identifies the actual channel.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/costEstimate.ts, line 101:
<comment>When FX has no reported `total_cost`, `resolveBilledCost` labels the unavailable bill as `<provider>_api` even though FX always uses its gateway. Return `fx_gateway` for FX's unreported path so billing metadata identifies the actual channel.</comment>
<file context>
@@ -19,49 +19,120 @@ export interface PriceMap {
+ };
}
+ const provider = providerOf(model);
+ const channel = SUBSCRIPTION_HARNESSES.has(harness)
+ ? "subscription"
+ : provider
</file context>
| const channel = SUBSCRIPTION_HARNESSES.has(harness) | |
| ? "subscription" | |
| : provider | |
| ? `${provider}_api` | |
| : "none"; | |
| const channel = SUBSCRIPTION_HARNESSES.has(harness) | |
| ? "subscription" | |
| : harness === "fx" | |
| ? "fx_gateway" | |
| : provider | |
| ? `${provider}_api` | |
| : "none"; |
| : provider | ||
| ? `${provider}_api` | ||
| : "none"; | ||
| if (!DIRECT_PROVIDER_HARNESSES.has(harness) || usage.convention === "unreported") { |
There was a problem hiding this comment.
P2: When a direct-provider harness returns no usage, this guard still computes and emits a zero-dollar cost because unreported is not set for Eve, Mastra, or Deep Agents. Propagate an explicit usage-present signal and return unavailable when the SDK supplied no usage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/costEstimate.ts, line 106:
<comment>When a direct-provider harness returns no usage, this guard still computes and emits a zero-dollar cost because `unreported` is not set for Eve, Mastra, or Deep Agents. Propagate an explicit usage-present signal and return `unavailable` when the SDK supplied no usage.</comment>
<file context>
@@ -19,49 +19,120 @@ export interface PriceMap {
+ : provider
+ ? `${provider}_api`
+ : "none";
+ if (!DIRECT_PROVIDER_HARNESSES.has(harness) || usage.convention === "unreported") {
+ return { cost_source: "unavailable", billing_channel: channel };
+ }
</file context>
3cd9df6 to
745e229
Compare
|
This draft is superseded by the published PR stack below. All 17 remote heads and immediate bases were verified against consolidated tip
All 132 changed paths from this draft have an explicit disposition, and six preserved copies of the two excluded historical reports match their recorded hashes. The corpus intentionally retains 122 active core/extended/holdout tasks without the duplicate corpus manifests. No unique deferred work is being claimed as implemented. This completes the replacement linkage for the already-closed draft. CI and automated review follow-ups remain in progress, and the verifier judge-default decision remains pending. |
Stacked on #2866 (← #2812, top of the harness wave). Everything found and fixed while smoke-gating all 8 external harnesses on HardBenchmark (Browserbase, 5 tasks/cell).
Tool surface
stagehand_facadeis the Playwright-batch surface. The shipped facaderunalready executed viaexperimentalBatch; this syncs its runtime with the experiment branch's improvements (a11y-tree fallback whengetByRolemisses in DOM, shadow-root XPath hops, 10 s default locator timeout) and rewrites the prompt/description to the Playwright idiom (page/context/browserin scope). snapshot/screenshot always on, no env knobs.stagehand_facade_legacypreserves the previous prompt contract (--surface=legacy), wired into all 8 harness surface lists, so runs are never silently mixed.page.frameLocator/locator.frameLocator/contentFrame()implemented across cross-origin iframes (deep-locator hop notation; a11y-routed frame-scopedgetByRole/getByLabel/getByText). Clearer strict-mode errors (lists candidates + disambiguation), layout-error wording, lenient bare snapshot ids.HardBenchmark validity audit → n = 45
scripts/audit-hardbenchmark.ts(reusable): Browserbase verified-mode reachability of every start URL + the deep pages each task needs, proxy retry, rubric schema, achievability vs prior trajectories, stop-before-purchase flags.valid:false, builder skips + logs):heb_comparison_shopping_1(PerimeterX wall through verified+proxy). 20 flaggedverdict_review: stop-before-purchase(carried into row metadata). Review list:datasets/hardbenchmark/AUDIT-REVIEW.md.Harness fixes (each gate-verified live)
google/gemini-2.5-flash) → every criterion "Fused judgment call failed", runs silently unscored. Now defaults togoogle/gemini-3.5-flash; explicitEVAL_VERIFIER_MODELstill fails loudly on a missing key.readOnlyHintneed approval and headless there is no reviewer → "user cancelled MCP tool call"; the model fell back to the operator's global~/.codextools and "passed" via curl. Fix:default_tools_approval_mode="approve"on runner-mounted servers + per-run isolatedCODEX_HOME. New metricsfacade_tool_calls/facade_tool_call_failuresand apassesWithoutBrowserUsegate so browserless passes are visible."*"in the deny list hid fx's ownmcp_select_toolmeta-tools; workspace was created beside (not below) the throwaway$HOME, soAGENTS.md/.fx.jsonnever loaded;web_fetchis not permission-gated in fx 0.0.3 (guidance added).function_callJSON (flatten_text).tool_surface,model,provider,dataset,task_count.Smoke results (gate = trajectories paired, usage present, live verdicts, facade actually driven)
eve ✅ · deepagents ✅ · claude_code ✅ · mastra ✅ · pi ✅ · fx ✅ · codex ✅ (post-fix) · cursor pending login.
Post-smoke hardening (from two-sided verdict audits of 114 rows across all 9 harnesses)
output.logsfor every harness (step N · tool · ok|ERR · code → result,summary ·,answer ·,result ·,timing ·), Browserbase session URL as the first line, raw SDK streams demoted.judgeOutcomeSuccess; strict vs lenient process score (blocker-credited criteria flagged); answer grounding recorded (advisory by default,EVAL_REQUIRE_GROUNDING=1to gate); gated outcome written intoscores/result.json; gated rows name the gate in the row error.browser_session_loststop reason; explicit 60-min Browserbase session timeout (project default was 15 min); proxied + verified sessions by default — parity with the native agent path, which explained the Macy's/recreation.gov walls on external harnesses.step_budget+terminationReasonrecorded.tool_surface/model/provider/dataset;agent_wall_ms/verifier_wall_mssplit; normalized usage (usage_input_total/cached/output/reasoning, one convention per harness); versioned price map (pricing/pricing.json) →cost_usd_estimated; EAP models intentionally unpriced.Summary by cubic
Makes Playwright-style batch runs on the facade reliable and auditable for HardBenchmark smoke gates:
stagehand_facadebecomes the Playwright-batch surface, andstagehand_facade_legacypreserves the previous contract so runs are never mixed. Harnesses now agree on step budgets, token usage, billed cost, and gated verdicts across all external runners.Tool surface and audit
Harness fixes and gates
google/gemini-3.5-flash; the retired default silently left runs unscored.EVAL_REQUIRE_GROUNDING=1.cost_usdper row — harness-reported dollars first, provider-rate computation from a versioned price map second, otherwise "unavailable".CODEX_HOMErollout file when a turn aborts on budget; unrecoverable or never-reported usage (cursor) carries no usage metrics and is priced as no_usage instead of zero.scores/result.json, keeping the judge's original verdict alongside.Written for commit 745e229. Summary will update on new commits.