Skip to content

feat: Playwright-batch facade surface, HardBench audit, and harness fixes from the smoke gates - #2867

Closed
miguelg719 wants to merge 49 commits into
bench/hardbenchmark-portfrom
feat/facade-batch-surface
Closed

feat: Playwright-batch facade surface, HardBench audit, and harness fixes from the smoke gates#2867
miguelg719 wants to merge 49 commits into
bench/hardbenchmark-portfrom
feat/facade-batch-surface

Conversation

@miguelg719

@miguelg719 miguelg719 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

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_facade is the Playwright-batch surface. The shipped facade run already executed via experimentalBatch; this syncs its runtime with the experiment branch's improvements (a11y-tree fallback when getByRole misses in DOM, shadow-root XPath hops, 10 s default locator timeout) and rewrites the prompt/description to the Playwright idiom (page/context/browser in scope). snapshot/screenshot always on, no env knobs.
  • stagehand_facade_legacy preserves 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-scoped getByRole/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.
  • 1 quarantined (valid:false, builder skips + logs): heb_comparison_shopping_1 (PerimeterX wall through verified+proxy). 20 flagged verdict_review: stop-before-purchase (carried into row metadata). Review list: datasets/hardbenchmark/AUDIT-REVIEW.md.

Harness fixes (each gate-verified live)

  • Verifier default was a retired model (google/gemini-2.5-flash) → every criterion "Fused judgment call failed", runs silently unscored. Now defaults to google/gemini-3.5-flash; explicit EVAL_VERIFIER_MODEL still fails loudly on a missing key.
  • codex never reached the facade: under the read-only sandbox, MCP tools without readOnlyHint need approval and headless there is no reviewer → "user cancelled MCP tool call"; the model fell back to the operator's global ~/.codex tools and "passed" via curl. Fix: default_tools_approval_mode="approve" on runner-mounted servers + per-run isolated CODEX_HOME. New metrics facade_tool_calls / facade_tool_call_failures and a passesWithoutBrowserUse gate so browserless passes are visible.
  • fx never reached the facade: "*" in the deny list hid fx's own mcp_select_tool meta-tools; workspace was created beside (not below) the throwaway $HOME, so AGENTS.md/.fx.json never loaded; web_fetch is not permission-gated in fx 0.0.3 (guidance added).
  • mastra/pi OOM at concurrency 5: step-finish chunks carried the full request body (quadratic) and were retained + stringified; pi retained base64 screenshots. Compacted events, redacted logs.
  • deepagents reasoning polluted with raw function_call JSON (flatten_text).
  • runner end-of-run summary crashed when a Braintrust row had no output (lost experiment.json).
  • Experiment metadata always carries 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)

  • Logs: unified step trace in output.logs for every harness (step N · tool · ok|ERR · code → result, summary ·, answer ·, result ·, timing ·), Browserbase session URL as the first line, raw SDK streams demoted.
  • Verifier gates (deterministic, post-judge): no final answer / trajectory error / no browser use → fail; judge verdict preserved as judgeOutcomeSuccess; strict vs lenient process score (blocker-credited criteria flagged); answer grounding recorded (advisory by default, EVAL_REQUIRE_GROUNDING=1 to gate); gated outcome written into scores/result.json; gated rows name the gate in the row error.
  • Session death: typed client deadline on callback batches; facade returns a terminal "Browser session lost" instead of dead-session loops; browser_session_lost stop 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.
  • Harness fixes: pi keeps its stock system prompt + thinking on (22% → 70% audited); fx final answer is the conclusion, not narration; eve final answer/reasoning pairing; eve routes non-first-party creators via the AI Gateway (+ context-window pin); codex usage recovered from the rollout on budget abort; reasoning summaries requested/captured on every harness that can emit them (fx cannot).
  • Budgets: dataset-aware step budgets, HardBench = 75 for all harnesses; step_budget + terminationReason recorded.
  • Metrics: experiment metadata always carries tool_surface/model/provider/dataset; agent_wall_ms/verifier_wall_ms split; 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.
  • Dataset: n = 43 (H-E-B, imgur, Best Buy trade-in quarantined with evidence; Macy's restored after the proxy/verified fix); constraint criteria on the 5 FP-prone tasks now require on-site evidence.
  • History note: the codex rollout-usage implementation was swept into 2acb9ab (facade parity commit); its tests are in c337309.

Summary by cubic

Makes Playwright-style batch runs on the facade reliable and auditable for HardBenchmark smoke gates: stagehand_facade becomes the Playwright-batch surface, and stagehand_facade_legacy preserves 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

  • Adds accessibility-tree role fallback, shadow-root XPath and cross-origin frame support, a 10-second locator timeout, and clearer strict-mode and layout errors.
  • Adds an explicit-snapshot-actions prompt variant that restores ref-action routing, and keeps the frame retry from masking the layout error that triggered it.
  • Adds a reusable HardBenchmark audit leaving 43 of 46 tasks valid, quarantining three broken tasks, and flagging 20 for verdict review.
  • Hardens ten rubric criteria on five false-positive-prone tasks to require on-site evidence for constraint claims (gluten-free, diploma-only, Best Sellers, product-page checks, seat fee).
  • The facade returns a terminal browser-session-lost error instead of looping on dead sessions; Browserbase sessions get an explicit 1-hour timeout and run proxied and verified by default, matching the native path.

Harness fixes and gates

  • Defaults rubric verification to google/gemini-3.5-flash; the retired default silently left runs unscored.
  • Ensures Codex and fx reach the mounted facade, records facade calls and failures, and flags passes that never call the browser.
  • Adds a deterministic strict-process-score gate that only downgrades a judge pass; answer grounding is recorded but advisory unless EVAL_REQUIRE_GROUNDING=1.
  • Requests model reasoning summaries by default so step reasoning is comparable across harnesses.
  • Restores pi's scoring by appending the eval brief to its stock prompt, fixes fx to grade the committed conclusion, and routes non-openai/anthropic/google eve models through the Vercel AI Gateway with a pinned context window.
  • Applies per-dataset step budgets (HardBench defaults to 75), splits agent vs verifier wall-clock, normalizes token usage, and reports one cost_usd per row — harness-reported dollars first, provider-rate computation from a versioned price map second, otherwise "unavailable".
  • Recovers Codex token usage from the CODEX_HOME rollout 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.
  • Trims Mastra/pi memory growth, removes raw Deepagents tool-call JSON, and keeps summaries after failed rows.
  • Persists termination reasons and gated verdicts in scores/result.json, keeping the judge's original verdict alongside.
  • Emits one shared per-step trace with summary, answer or missing-answer status, and result; logs the Browserbase session URL first in each task.

Written for commit 745e229. Summary will update on new commits.

Review in cubic

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.
@changeset-bot

changeset-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 745e229

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

24 issues found across 53 files

Confidence score: 2/5

  • packages/integrations/core/src/facade/runtime.ts forwards arbitrary browser/runtime text through describeActionFailure, which can expose unsanitized external content in errors—sanitize failure messages before wrapping them in Error instances.
  • packages/evals/scripts/audit-hardbenchmark.ts can mark skipped or failed reachability checks as valid and clear existing quarantines, while AUDIT-REVIEW.md assumes 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.ts may leave the browser/session alive after browser.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.ts can resolve nested frames incorrectly and ignore role-state or visibility options, while packages/evals/framework/codexToolAdapter.ts can 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
Loading

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 => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]!);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Comment on lines +294 to +305
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(() => {});
},
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 } : {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 issues found across 51 files (changes from recent commits).

Confidence score: 2/5

  • packages/evals/framework/harnesses/externalRunner.ts and packages/evals/framework/harnesses/traceLog.ts can persist credential-bearing stopReason values in trace logs and result.logs, creating a concrete sensitive-data exposure risk — pass the sanitized stop reason through every logging path.
  • packages/integrations/core/src/facade/stdio-server.ts still accepts session_info requests despite omitting the method from tools/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, and cursor-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.ts can construct the wrong URL when a session ID contains reserved characters, while packages/evals/framework/harnesses/traceLog.ts can 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 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

View Feedback

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}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)] : []),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

View Feedback

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"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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)),

Comment on lines +663 to +666
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 ?? ""))),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

View Feedback

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).

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files (changes from recent commits).

Confidence score: 3/5

  • In packages/integrations/pi-sdk/src/session.ts, runPiSession overwrites a resolver-provided thinking level with "medium" before loadPiSdk can apply it, so configured model reasoning behavior may be ignored — forward thinkingLevel only 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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Confidence score: 5/5

  • packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md contains 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 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts reasoning summaries and raw errors rethrown by packages/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.ts can 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.ts and packages/evals/core/tools/stagehandFacadeBridge.ts, while packages/integrations/core/src/facade/tools.ts misses 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.ts and packages/integrations/deepagents-sdk/src/session.ts can 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 : "";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
extraConfig: buildEvalCodexConfig(extraConfig),
extraConfig: buildEvalCodexConfig(extraConfig, env),

Comment on lines +354 to +356
answer_grounded: flag(!gates.grounding.gatesOutcome),
}),
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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, "");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Comment on lines +165 to +169
if (!Number.isInteger(clientTimeout) || clientTimeout <= 0) {
throw new RangeError(
"stagehand.experimentalBatch() clientTimeoutMs must be a positive integer",
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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() }),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files (changes from recent commits).

Confidence score: 3/5

  • packages/evals/framework/verifierGates.ts changes resolveRequireGrounding so an unset EVAL_REQUIRE_GROUNDING disables 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.ts silently depends on EVAL_REQUIRE_GROUNDING not 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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 issues found across 43 files (changes from recent commits).

Confidence score: 3/5

  • packages/evals/core/tools/stagehand_facade.ts exposes raw environment-variable values through evalBooleanEnv errors, 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.ts treats all-zero usage as measured, while packages/evals/pricing/pricing.json disagrees with the UI’s Grok 4.5 cached-token rate; reject unknown totals and align the pricing source.
  • packages/evals/scripts/update-pricing.ts can 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.ts accepts malformed numeric values, while packages/evals/framework/harnesses/externalRunner.ts undercounts wall time and omits prices_as_of; validate full integers, measure from run completion, and preserve pricing provenance. packages/evals/tests/framework/codexRunner.test.ts also leaves temporary directories behind, so clean them up in finally.
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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?? []) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
...(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}".`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

View Feedback

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-"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 5 files (changes from recent commits).

Confidence score: 2/5

  • packages/evals/framework/costEstimate.ts can 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.ts can 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.ts labels FX’s unreported gateway bill as <provider>_api, weakening billing attribution; return fx_gateway for 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",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Comment on lines +101 to +105
const channel = SUBSCRIPTION_HARNESSES.has(harness)
? "subscription"
: provider
? `${provider}_api`
: "none";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

@miguelg719
miguelg719 marked this pull request as draft August 31, 2026 23:20
@miguelg719
miguelg719 force-pushed the feat/facade-batch-surface branch from 3cd9df6 to 745e229 Compare September 1, 2026 00:00
@miguelg719 miguelg719 closed this Sep 8, 2026
@miguelg719

Copy link
Copy Markdown
Collaborator Author

This draft is superseded by the published PR stack below. All 17 remote heads and immediate bases were verified against consolidated tip d08dab9b8541f4cc5be08fb352fd05fa0eb8db5f; every replacement is open and non-draft.

  1. Add HardBench and rubric v1.2 on the current evaluator #2891 — HardBench and rubric v1.2.
  2. Make browser session ownership and facade mounts explicit #2892 — Browser session ownership and facade mounts.
  3. Recover bounded capture deadlines and preserve terminal session loss #2893 — Capture-deadline recovery and terminal loss handling.
  4. Add bounded CDP heartbeat and sanitized disconnect diagnostics #2894 — CDP heartbeat and disconnect diagnostics.
  5. Align label, strict locator, and URL wait semantics #2895 — Locator and URL-wait semantics.
  6. Resolve facade locators and snapshot refs across frames #2896 — Frame locators and snapshot references.
  7. Bound stalled DeepAgents sessions and cleanup #2897 — Deepagents watchdogs and cleanup.
  8. Route existing harness models through native providers #2898 — Native provider routing.
  9. Bound SDK event logs while preserving screenshot evidence #2899 — Bounded event retention and screenshot evidence.
  10. Preserve SDK usage reporting and interrupted Codex usage #2900 — Usage reporting and interrupted Codex usage.
  11. Keep verifier outcomes, uncertainty, and execution failures auditable #2901 — Auditable verifier outcomes and ungraded failures.
  12. Standardize eval policy, budgets, session metadata, traces, and cost accounting #2902 — Shared eval policy, budgets, metadata, traces, and accounting.
  13. Apply the shared eval contract across existing harnesses #2903 — Shared contract adoption across existing harnesses.
  14. Run Cursor SDK through the shared facade in evals and the example #2889 — Reused Cursor SDK contribution, eval integration, and example.
  15. Expose one typed facade bridge for native computer-use adapters #2904 — Shared facade bridge for computer use.
  16. Add Claude native browser use through the shared facade runtime #2905 — Claude native browser use.
  17. Add Gemini native computer use with model-visible evidence #2906 — Gemini native computer use.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant