feat: Playwright-batch facade surface, HardBench audit, and harness fixes from the smoke gates - #2867
Open
miguelg719 wants to merge 20 commits into
Open
feat: Playwright-batch facade surface, HardBench audit, and harness fixes from the smoke gates#2867miguelg719 wants to merge 20 commits into
miguelg719 wants to merge 20 commits into
Conversation
Sync facade/runtime.ts with the evals playwrightCompatRuntime experiment:
accessibility-tree fallback for getByRole misses, shadow-root XPath
resolution for snapshot-derived paths, 10 s default locator timeout.
Keep the facade's browser.close() -> closeRequested semantics.
run now returns a full batch envelope (telemetry, screenshot artifacts,
closeRequested, batch runtime), writes page.screenshot({ path }) files,
retries without a page target when the batch page vanished, and reports
telemetry to stderr as stagehand_playwright_compat lines.
Tool description and FACADE_AGENT_INSTRUCTIONS move to the Playwright
idiom (page/context/browser in scope, no AI methods). The previous text
is preserved verbatim as LEGACY_* and selectable with --surface=legacy.
…agehand_facade_legacy stagehand_facade_legacy starts the same facade server with --surface=legacy and mounts LEGACY_FACADE_AGENT_INSTRUCTIONS so runs on the earlier prompt are never mixed with the Playwright-idiom surface. Registered on every harness that mounts the facade (claude_code, codex, mastra, pi, eve, deepagents, fx, cursor).
V3Evaluator's built-in default (google/gemini-2.5-flash) was retired on 2026-07-09; without EVAL_VERIFIER_MODEL every rubric criterion failed with "Fused judgment call failed" and whole runs were silently unscored. The verifier now defaults to google/gemini-3.5-flash; an explicit override still fails loudly when its provider key is missing.
…flag 20 for verdict review Add scripts/audit-hardbenchmark.ts, a reusable four-check audit for the suite: live reachability in a Browserbase verified-mode session (start URL plus the deep pages each task needs, proxy retry on captcha/WAF; only a block on both attempts counts), rubric shape, achievability against past trajectory runs (model dirs aliased), and a stop-before-purchase heuristic. `--apply` writes `valid: false` + `invalid_reason` / `verdict_review` into the jsonl without deleting rows and can clear its own earlier quarantines. Suite builder now skips rows with valid === false and logs how many, and carries verdict_review into testcase metadata. Audit 2026-08-30: 45/46 valid. - quarantined: heb_comparison_shopping_1 (PerimeterX wall on both attempts) - 20 tasks flagged stop-before-purchase (see AUDIT-REVIEW.md) - all rubrics well-formed; no task showed missing content
The fx settings.json carried a "*": "deny" catch-all. In fx 0.0.3 a deny rule hides the tool from the model, and "*" also hid fx's own mcp_search_tools / mcp_select_tool, so no dynamic MCP tool could ever be selected: the trace showed tool_schema_count=1 and every HardBench step was web_fetch (which is not permission-gated in 0.0.3, so its deny rule is inert). Drop the catch-all; explicit per-tool denies still hide the built-ins. The per-run workspace also sat beside the throwaway $HOME instead of below it, so fx skipped AGENTS.md (project_rules_omitted: workspace is not below home) and the exact-tool-name guidance never reached the model. Nest the workspace under home via resolveFxRuntimePaths and spell out in AGENTS.md that web_fetch is not a browser. Verified: 1-task HardBench probe now runs mcp_select_tool -> 9x mcp_stagehand_run (Playwright idiom), zero web_fetch, verifier pass.
…nd duplicate screenshots Both in-process harnesses exhausted Node's default heap at concurrency 5 on HardBenchmark. Mastra's step-finish chunks carry the full request body under payload.metadata, which summarizeMastraEvent stringified into an unclipped log detail every step while events[] kept every chunk; pi kept each screenshot as base64 in tool_execution_end, again in the toolResult message_end, and again in both log details. Mastra now retains only the fields the trajectory adapter and usage accounting read (compactMastraEvent) and clips fallback details. pi decodes screenshots to one Buffer on the tool event, drops image payloads from non-assistant messages, and redacts image data from log details; the pi trajectory adapter accepts the Buffer form.
With langchain_openai on the Responses API, AIMessage.content carries function_call (and reasoning) blocks beside the text blocks. flatten_text json-dumped anything that was not text or an image, so every step's reasoning and evidence text became the raw function_call JSON. Skip tool-call block types (they are already reported via message.tool_calls) and reduce reasoning blocks to their summary text.
…utput Braintrust leaves result.output undefined when the task function threw or its span failed after the task returned, and the end-of-run mapping dereferenced output._success, so one such row crashed the CLI before the summary, per-model table, and experiment.json link were written for the whole run. Such rows now count as failures carrying the Braintrust error message.
…ade tool calls Codex 0.147 treats MCP tools without a readOnlyHint as needing approval under the read-only sandbox. Headless runs have no reviewer, so every stagehand facade call failed as "user cancelled MCP tool call" and gpt-5.4-mini fell back to whatever the operator's ~/.codex/config.toml exposed (node_repl, bundled plugins) — three HardBench tasks "passed" without touching the browser. - Pre-approve tools on every runner-mounted MCP server (default_tools_approval_mode = "approve"); the sandbox stays read-only. - Run each Codex session with CODEX_HOME pointed at a per-run directory so the global config can never leak extra servers or plugins into a cell. - Record facade_tool_calls / facade_tool_call_failures on external-harness results so a pass with zero browser use is visible in Braintrust.
… layout errors; lenient snapshot ids
page.frameLocator(selector) (plus locator.frameLocator/contentFrame) returns
a locator factory scoped to that iframe. css/xpath/text= tails compile to
Stagehand hop selectors ("iframe >> tail"), which the extension's deepLocator
resolves across cross-origin frames; getByRole/getByLabel/exact getByText
resolve through the includeIframes accessibility snapshot, scoped to nodes
under the first hop's iframe XPath, and act via deep XPaths. Supports nested
frameLocator chaining, first/nth/last, click/fill/type/press/hover/
selectOption/check/setInputFiles, count/isVisible/waitFor/text getters.
Unsupported operations fail with a targeted message.
Strict-mode violations now list up to five candidates (tag, text, visibility)
plus how to disambiguate; check()/uncheck() report their own method name.
A matched element with no layout box is reported as not rendered instead of
the bare -32000 CDP error after the retry window.
Snapshot actions accept a backend id without its frame-ordinal prefix when
it is unambiguous (models frequently pass "7812" for "0-7812"), and retry
once after 250 ms on layout-object errors.
Per-arm verifiability now counts successful runs whose facade_tool_calls metric is 0 and prints them next to the unverifiable-criteria summary. When EVAL_MAX_UNVERIFIABLE_CRITERIA gates the batch, such passes fail it too: the rubric verifier cannot tell an answer fetched with curl from one found in the browser.
…provider, dataset Bench experiments previously exposed only environment/harness/tier (and model only when --model was passed), so cells could not be grouped by surface or model in Braintrust/LangSmith. Derive them from the planned rows.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
24 issues found across 53 files
Confidence score: 2/5
packages/integrations/core/src/facade/runtime.tsforwards arbitrary browser/runtime text throughdescribeActionFailure, which can expose unsanitized external content in errors—sanitize failure messages before wrapping them inErrorinstances.packages/evals/scripts/audit-hardbenchmark.tscan mark skipped or failed reachability checks as valid and clear existing quarantines, whileAUDIT-REVIEW.mdassumes proxies the runner does not enable; gate unquarantining on completed audits and align campaign proxy configuration or task documentation.packages/integrations/core/src/facade/tools.tsmay leave the browser/session alive afterbrowser.close()and replay earlier side-effecting actions after a later snapshot failure, risking leaked resources and duplicate clicks or input—route close requests to the host and retry only unexecuted work.packages/integrations/core/src/facade/runtime.tscan resolve nested frames incorrectly and ignore role-state or visibility options, whilepackages/evals/framework/codexToolAdapter.tscan hide existing Codex login credentials in its isolated home; resolve every frame hop, preserve query options, and copy login credentials into the run environment.
Not reviewed (too large): packages/evals/datasets/hardbenchmark/audit-2026-08-30.json (~2,831 lines), packages/evals/datasets/hardbenchmark/HardBenchmark_data.jsonl (~42 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/integrations/core/src/facade/runtime.ts">
<violation number="1" location="packages/integrations/core/src/facade/runtime.ts:248">
P2: When the accessibility fallback sees a hidden role node, `getByRole` can return it even without `includeHidden: true` because this conversion discards the visibility option. Preserve `includeHidden` in the fallback step and reject non-visible candidates unless it is enabled.</violation>
<violation number="2" location="packages/integrations/core/src/facade/runtime.ts:949">
P1: Custom agent: **Exception and error message sanitization**
When a facade action fails, `describeActionFailure` forwards arbitrary browser/runtime text and wraps it in generic `new Error()` instances. Sanitize external details and raise a dedicated typed facade error instead; apply the same treatment to the new strict-mode and frame-locator errors.</violation>
<violation number="3" location="packages/integrations/core/src/facade/runtime.ts:1640">
P2: When code uses state options with `frame.getByRole`, the facade silently ignores them and may act on the wrong control. Carry the role state options through `FrameQuery` and enforce them during accessibility-tree matching.</violation>
<violation number="4" location="packages/integrations/core/src/facade/runtime.ts:1787">
P1: Nested `frameLocator()` calls are not scoped to their nested iframe: the accessibility route filters only by the first hop. Resolve and apply every hop before accepting a candidate, so `frameLocator("#outer").frameLocator("#inner")` cannot select an element elsewhere under `#outer`.</violation>
</file>
<file name="packages/integrations/core/src/facade/contract.ts">
<violation number="1" location="packages/integrations/core/src/facade/contract.ts:157">
P2: Updating the canonical prompt here leaves CrewAI's copied `FACADE_AGENT_INSTRUCTIONS` stale. Its contract test compares the copy with this template, so the integration test fails; update the CrewAI copy alongside this change or generate it from the canonical constant.</violation>
<violation number="2" location="packages/integrations/core/src/facade/contract.ts:159">
P2: When an agent follows this prompt and calls `page.waitForURL`, the facade rejects the call because the runtime does not implement that method. Remove this example or implement `waitForURL` before advertising it.</violation>
</file>
<file name="packages/evals/framework/runner.ts">
<violation number="1" location="packages/evals/framework/runner.ts:385">
P2: Default `stagehand` benchmark runs omit `tool_surface` from experiment metadata, so those experiments cannot be grouped by the promised surface metadata. Propagate a stable default surface for these runs, or explicitly revise the metadata contract to allow this field to be absent.</violation>
</file>
<file name="packages/evals/tui/commands/verify.ts">
<violation number="1" location="packages/evals/tui/commands/verify.ts:51">
P2: When `evals verify` runs without `--model`, `handleVerify` still uses V3Evaluator's built-in default, so this help advertises a model the command does not select. Pass `DEFAULT_VERIFIER_MODEL` here or reuse `createVerifierEvaluator` before documenting 3.5.</violation>
</file>
<file name="packages/evals/framework/harnesses/piAdapter.ts">
<violation number="1" location="packages/evals/framework/harnesses/piAdapter.ts:139">
P2: The new retained-`Buffer` path is not covered by the adapter tests, so a regression could silently drop screenshots from trajectories while the existing test still passes through the base64 fallback. Add a focused adapter test with `{ bytes: image, mimeType: "image/png" }` and assert the resulting image modality preserves `image`.</violation>
</file>
<file name="packages/evals/evals.config.json">
<violation number="1" location="packages/evals/evals.config.json:27">
P2: Because this marker is committed, every fresh checkout skips the first-run welcome. Remove the runtime-generated `_meta` block from the baseline config and let `markFirstRunComplete` persist it locally.</violation>
</file>
<file name="packages/evals/framework/codexToolAdapter.ts">
<violation number="1" location="packages/evals/framework/codexToolAdapter.ts:114">
P1: When an eval uses an existing `codex login` instead of `OPENAI_API_KEY`, this empty per-run home removes the login credentials and Codex fails before the task starts. Copy the login credentials into the isolated home, or isolate configuration while preserving the authenticated home.</violation>
</file>
<file name="packages/evals/framework/verifierAdapter.ts">
<violation number="1" location="packages/evals/framework/verifierAdapter.ts:35">
P2: The new default selection and credential policy have no focused tests, so a future model or key-resolution change can silently ungrade verifier runs. Add tests for the default model, explicit missing-key failure, and keyless providers.
(Based on your team's feedback about unit coverage for changed logic.)</violation>
</file>
<file name="packages/integrations/core/src/facade/tools.ts">
<violation number="1" location="packages/integrations/core/src/facade/tools.ts:200">
P1: When a later snapshot action fails with a layout error, this retry replays every earlier action in the batch, causing duplicate clicks, typing, or other side effects. Retry only the failed or unexecuted action, or remove the whole-batch retry.</violation>
<violation number="2" location="packages/integrations/core/src/facade/tools.ts:219">
P1: When facade code calls `browser.close()`, `runNow` only reports `closeRequested`; the stdio host logs that report and never closes Stagehand or the browser, leaving the persistent browser/session alive. Route this flag to the host cleanup lifecycle after the batch.</violation>
</file>
<file name="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md">
<violation number="1" location="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md:1">
P3: The report says 2026-08-30, but the linked audit JSON records 2026-08-31. Align the report and artifact dates so campaign provenance is unambiguous.</violation>
<violation number="2" location="packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md:50">
P1: These tasks are not guaranteed reachable in campaigns: the runner does not enable Browserbase proxies, despite this note claiming it always does. Enable proxies for campaign sessions or document these tasks as proxy-dependent.</violation>
</file>
<file name="packages/integrations/pi-sdk/src/session.ts">
<violation number="1" location="packages/integrations/pi-sdk/src/session.ts:324">
P2: When a detail contains a secret near the 20,000-character boundary, `clip` runs before `sanitizeErrorMessage` and can leave an unmatched secret fragment in logs and transcripts. Sanitize the complete detail first, then apply the length cap at all three detail sites.</violation>
</file>
<file name="packages/evals/scripts/audit-hardbenchmark.ts">
<violation number="1" location="packages/evals/scripts/audit-hardbenchmark.ts:294">
P2: If Stagehand initialization or page discovery fails after `browserbase.launch`, this helper leaks the live Browserbase session because cleanup only exists on the returned `Session`. Wrap attachment and page acquisition in `try/catch`, close Stagehand and the browser, then rethrow.</violation>
<violation number="2" location="packages/evals/scripts/audit-hardbenchmark.ts:349">
P2: Raw Browserbase/Stagehand errors are persisted in `audit-<date>.json`, where they can expose credential-bearing URLs or session details. Sanitize these messages or record a generic probe error before serialization.
(Based on your team's feedback about sanitized Browserbase session errors.)</violation>
<violation number="3" location="packages/evals/scripts/audit-hardbenchmark.ts:635">
P2: When `--only` reruns a task without `--trajectories`, this replacement discards the prior achievability record from the existing report. Preserve the previous task’s achievability whenever the optional trajectory check was not requested.</violation>
<violation number="4" location="packages/evals/scripts/audit-hardbenchmark.ts:663">
P1: When reachability is skipped or the Browserbase session fails, `a.valid` remains true and this branch clears existing bot-wall/dead quarantines. Guard the cleanup so only a completed reachability audit can unquarantine a row.</violation>
</file>
<file name="packages/evals/suites/hardbenchmark.ts">
<violation number="1" location="packages/evals/suites/hardbenchmark.ts:166">
P3: The new manual-review marker is not part of the `Testcase.metadata` type. Add `verdict_review?: string` to that metadata contract so downstream reporting and review tooling can consume it without casts.</violation>
</file>
<file name="packages/integrations/deepagents/runner/run_eval.py">
<violation number="1" location="packages/integrations/deepagents/runner/run_eval.py:146">
P2: If a provider emits a content block with a non-hashable `type`, this membership test raises `TypeError` before fallback serialization and fails the run; validate the type before set membership.</violation>
<violation number="2" location="packages/integrations/deepagents/runner/run_eval.py:148">
P2: When an Anthropic response contains a native `thinking` block, this branch is skipped and the entire block is serialized into `assistant.text` and `final.text; handle `thinking` blocks here.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Suite as Suite Builder
participant Audit as Audit Script
participant Runner as External Runner
participant Adapter as Tool Adapter
participant Server as Facade MCP Server
participant Runtime as Playwright Runtime
participant Browser as Browser Session
participant Verifier as Rubric Verifier
participant Metrics as Metrics/Verifiability
Note over Suite,Audit: Dataset Validity Pipeline
Suite->>Audit: Read HardBenchmark rows
Audit->>Audit: Reachability probe (verified mode)
Audit->>Audit: Rubric schema validation
Audit->>Audit: Store audit results
Audit-->>Suite: Quarantine invalid rows (valid:false)
Suite->>Runner: Build testcase list (skip quarantined)
Note over Runner,Adapter: Harness Startup & Facade Mount
Runner->>Adapter: Prepare harness (eve/deepagents/codex/fx)
Adapter->>Server: Spawn facade stdio server
Adapter->>Adapter: Select surface (playwright/legacy)
Adapter->>Server: Mount MCP server
Server->>Runtime: Initialize Stagehand/Playwright
Note over Adapter,Runtime: Browser Session & Facade Calls
Adapter->>Server: Set up MCP config
Server->>Runtime: Execute agent code (playwright fallback)
Runtime->>Browser: Launch browser (Browserbase/LOCAL)
Browser-->>Runtime: Active page/context
alt Playwright-surface run
Adapter->>Server: run() with Playwright API
else Legacy-surface run
Adapter->>Server: run() with legacy prompt contract
end
Runtime->>Runtime: getByRole a11y-tree fallback
Runtime->>Runtime: frameLocator / contentFrame
Runtime->>Browser: Navigate, locate, interact
Runtime-->>Adapter: Return results + telemetry
Note over Runner,Verifier: Verdict & Metrics
Adapter->>Runner: Trajectory + tool call data
Runner->>Verifier: Grade trajectory
Verifier->>Runner: Return graded result
Runner->>Metrics: Compute facade_tool_calls/failures
Metrics->>Runner: Check passesWithoutBrowserUse
Note over Adapter,Verifier: Harness-specific Fixes
opt codex harness
Adapter->>Adapter: Set default_tools_approval_mode=approve
Adapter->>Adapter: Isolate CODEX_HOME per-run
end
opt fx harness
Adapter->>Adapter: Nested workspace under home
Adapter->>Adapter: Remove "*" deny rule
end
opt mastra/pi harness
Adapter->>Adapter: Compact events (drop request bodies)
Adapter->>Adapter: Decode screenshots once
end
Runner->>Runner: Build summary (survive missing Braintrust output)
Runner-->>Suite: Task result with metrics/gates
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * "-32000 Node does not have a layout object". After the retry window that | ||
| * is what the agent saw; say what it means instead. | ||
| */ | ||
| const describeActionFailure = (method: string, error: unknown, timeout: number): Error => { |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
When a facade action fails, describeActionFailure forwards arbitrary browser/runtime text and wraps it in generic new Error() instances. Sanitize external details and raise a dedicated typed facade error instead; apply the same treatment to the new strict-mode and frame-locator errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/runtime.ts, line 949:
<comment>When a facade action fails, `describeActionFailure` forwards arbitrary browser/runtime text and wraps it in generic `new Error()` instances. Sanitize external details and raise a dedicated typed facade error instead; apply the same treatment to the new strict-mode and frame-locator errors.</comment>
<file context>
@@ -650,6 +932,30 @@ export async function createPlaywrightCompatRuntime(
+ * "-32000 Node does not have a layout object". After the retry window that
+ * is what the agent saw; say what it means instead.
+ */
+ const describeActionFailure = (method: string, error: unknown, timeout: number): Error => {
+ const message = error instanceof Error ? error.message : String(error);
+ if (!LAYOUT_ERROR_RE.test(message)) return error instanceof Error ? error : new Error(message);
</file context>
| xpaths: string[]; | ||
| names: string[]; | ||
| }> { | ||
| const prefix = await iframeHostXPath(this.state, this.hops[0]!); |
Contributor
There was a problem hiding this comment.
P1: Nested frameLocator() calls are not scoped to their nested iframe: the accessibility route filters only by the first hop. Resolve and apply every hop before accepting a candidate, so frameLocator("#outer").frameLocator("#inner") cannot select an element elsewhere under #outer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/runtime.ts, line 1787:
<comment>Nested `frameLocator()` calls are not scoped to their nested iframe: the accessibility route filters only by the first hop. Resolve and apply every hop before accepting a candidate, so `frameLocator("#outer").frameLocator("#inner")` cannot select an element elsewhere under `#outer`.</comment>
<file context>
@@ -1145,6 +1489,638 @@ export async function createPlaywrightCompatRuntime(
+ xpaths: string[];
+ names: string[];
+ }> {
+ const prefix = await iframeHostXPath(this.state, this.hops[0]!);
+ if (!prefix) return { xpaths: [], names: [] };
+ const snapshot = (await this.state.rawPage.snapshot({ includeIframes: true })) as {
</file context>
| for (const [key, value] of Object.entries(baseEnv)) { | ||
| if (value !== undefined) env[key] = value; | ||
| } | ||
| env.CODEX_HOME = codexHome; |
Contributor
There was a problem hiding this comment.
P1: When an eval uses an existing codex login instead of OPENAI_API_KEY, this empty per-run home removes the login credentials and Codex fails before the task starts. Copy the login credentials into the isolated home, or isolate configuration while preserving the authenticated home.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/codexToolAdapter.ts, line 114:
<comment>When an eval uses an existing `codex login` instead of `OPENAI_API_KEY`, this empty per-run home removes the login credentials and Codex fails before the task starts. Copy the login credentials into the isolated home, or isolate configuration while preserving the authenticated home.</comment>
<file context>
@@ -60,30 +60,73 @@ export const CODEX_TOOL_SURFACES: ToolSurface[] = [
+ for (const [key, value] of Object.entries(baseEnv)) {
+ if (value !== undefined) env[key] = value;
+ }
+ env.CODEX_HOME = codexHome;
+ return env;
+}
</file context>
| telemetry: envelope.telemetry, | ||
| batchRoundTripMs: performance.now() - startedAt, | ||
| batchRuntimeMs: envelope.batchRuntimeMs, | ||
| closeRequested: envelope.closeRequested, |
Contributor
There was a problem hiding this comment.
P1: When facade code calls browser.close(), runNow only reports closeRequested; the stdio host logs that report and never closes Stagehand or the browser, leaving the persistent browser/session alive. Route this flag to the host cleanup lifecycle after the batch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/tools.ts, line 219:
<comment>When facade code calls `browser.close()`, `runNow` only reports `closeRequested`; the stdio host logs that report and never closes Stagehand or the browser, leaving the persistent browser/session alive. Route this flag to the host cleanup lifecycle after the batch.</comment>
<file context>
@@ -166,11 +210,15 @@ export class StagehandFacadeTools {
+ telemetry: envelope.telemetry,
+ batchRoundTripMs: performance.now() - startedAt,
+ batchRuntimeMs: envelope.batchRuntimeMs,
+ closeRequested: envelope.closeRequested,
+ });
+ await this.writeScreenshotArtifacts(envelope.artifacts);
</file context>
| // A freshly hydrated element with no layout box is usually mid-render | ||
| // (menus, lazy lists); give it one beat before reporting. | ||
| if (!isLayoutError(error)) throw error; | ||
| await page.waitForTimeout(250); |
Contributor
There was a problem hiding this comment.
P1: When a later snapshot action fails with a layout error, this retry replays every earlier action in the batch, causing duplicate clicks, typing, or other side effects. Retry only the failed or unexecuted action, or remove the whole-batch retry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/tools.ts, line 200:
<comment>When a later snapshot action fails with a layout error, this retry replays every earlier action in the batch, causing duplicate clicks, typing, or other side effects. Retry only the failed or unexecuted action, or remove the whole-batch retry.</comment>
<file context>
@@ -147,15 +180,26 @@ export class StagehandFacadeTools {
+ // A freshly hydrated element with no layout box is usually mid-render
+ // (menus, lazy lists); give it one beat before reporting.
+ if (!isLayoutError(error)) throw error;
+ await page.waitForTimeout(250);
+ result = await runBatch();
+ }
</file context>
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(() => {}); | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
P2: If Stagehand initialization or page discovery fails after browserbase.launch, this helper leaks the live Browserbase session because cleanup only exists on the returned Session. Wrap attachment and page acquisition in try/catch, close Stagehand and the browser, then rethrow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/scripts/audit-hardbenchmark.ts, line 294:
<comment>If Stagehand initialization or page discovery fails after `browserbase.launch`, this helper leaks the live Browserbase session because cleanup only exists on the returned `Session`. Wrap attachment and page acquisition in `try/catch`, close Stagehand and the browser, then rethrow.</comment>
<file context>
@@ -0,0 +1,680 @@
+ },
+ userMetadata: { stagehand: "true", evals: "true", audit: "hardbenchmark" },
+ } as Parameters<typeof browserbase.launch>[0]);
+ const stagehand = await Stagehand.create({ browser });
+ const pages = await browser.context.pages();
+ const page = pages[0] ?? (await browser.context.newPage());
</file context>
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: |
Contributor
There was a problem hiding this comment.
P2: If a provider emits a content block with a non-hashable type, this membership test raises TypeError before fallback serialization and fails the run; validate the type before set membership.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/deepagents/runner/run_eval.py, line 146:
<comment>If a provider emits a content block with a non-hashable `type`, this membership test raises `TypeError` before fallback serialization and fails the run; validate the type before set membership.</comment>
<file context>
@@ -122,6 +143,11 @@ def flatten_text(content: object) -> str:
and isinstance(block.get("text"), str)
):
parts.append(block["text"])
+ elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES:
+ continue
+ elif isinstance(block, dict) and block.get("type") == "reasoning":
</file context>
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": |
Contributor
There was a problem hiding this comment.
P2: When an Anthropic response contains a native thinking block, this branch is skipped and the entire block is serialized into assistant.text and final.text; handle thinking` blocks here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/deepagents/runner/run_eval.py, line 148:
<comment>When an Anthropic response contains a native `thinking` block, this branch is skipped and the entire block is serialized into `assistant.text` and `final.text; handle `thinking` blocks here.</comment>
<file context>
@@ -122,6 +143,11 @@ def flatten_text(content: object) -> str:
parts.append(block["text"])
+ elif isinstance(block, dict) and block.get("type") in _TOOL_CALL_BLOCK_TYPES:
+ continue
+ elif isinstance(block, dict) and block.get("type") == "reasoning":
+ if text := _reasoning_text(block):
+ parts.append(text)
</file context>
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 | |||
Contributor
There was a problem hiding this comment.
P3: The report says 2026-08-30, but the linked audit JSON records 2026-08-31. Align the report and artifact dates so campaign provenance is unambiguous.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/datasets/hardbenchmark/AUDIT-REVIEW.md, line 1:
<comment>The report says 2026-08-30, but the linked audit JSON records 2026-08-31. Align the report and artifact dates so campaign provenance is unambiguous.</comment>
<file context>
@@ -0,0 +1,58 @@
+# HardBenchmark validity audit — 2026-08-30
+
+Source: `scripts/audit-hardbenchmark.ts`, full record in `audit-2026-08-30.json`. Live probes ran in Browserbase `verified` mode with a proxy retry on captcha/WAF; achievability cross-referenced two frontier-model trajectory sets from 2026-08-24 (aliased `model-1`/`model-2`).
</file context>
| source_suite: row.source_suite, | ||
| failure_mode: row.failure_mode, | ||
| capability_axis: row.capability_axis, | ||
| ...(row.verdict_review ? { verdict_review: row.verdict_review } : {}), |
Contributor
There was a problem hiding this comment.
P3: The new manual-review marker is not part of the Testcase.metadata type. Add verdict_review?: string to that metadata contract so downstream reporting and review tooling can consume it without casts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/suites/hardbenchmark.ts, line 166:
<comment>The new manual-review marker is not part of the `Testcase.metadata` type. Add `verdict_review?: string` to that metadata contract so downstream reporting and review tooling can consume it without casts.</comment>
<file context>
@@ -144,6 +163,7 @@ export const buildHardBenchmarkTestcases = (models: string[] | AgentModelEntry[]
source_suite: row.source_suite,
failure_mode: row.failure_mode,
capability_axis: row.capability_axis,
+ ...(row.verdict_review ? { verdict_review: row.verdict_review } : {}),
},
expected: true,
</file context>
…eltas Every session layer (claude-agent-sdk, codex-sdk, mastra-sdk, pi-sdk, eve-sdk, deepagents-sdk, fx-sdk, cursor-sdk) now classifies its raw events through a shared harnessEventLogLevel helper: stream fragments (*-delta, *_update, item.updated, stream_event) and bare lifecycle markers (*-start/*-end without content) are not logged at all, failures stay at level 1, everything else drops to level 2. The readable per-step trace is emitted from the normalized trajectory instead.
…starts
The facade stdio server gains a runner-only session_info tool (absent
from tools/list) that launches the browser if needed and reports
{ provider, sessionId }. The bridge exposes it as sessionInfo() with a
launch-sized timeout, and StagehandFacadeTool calls it for BROWSERBASE
runs so browserbaseSessionId/Url land on the start metadata; LOCAL runs
keep the lazy launch. stagehand_code publishes its session/debug URLs on
metadata the same way.
runExternalHarnessTask emits one 'trace' log line per normalized trajectory step (think / tool · ok|ERR · code → result) plus a final 'result · status · steps · facade_calls · tokens' line, so claude_code, codex, mastra, pi, eve, deepagents, fx and cursor rows all read the same. Full code/results ride in auxiliary, capped at 16 KB. EvalLogger.getLogs now filters to level <= 1 by default so debug lines stay out of output.logs; the runner re-reads logs after grading so trace and verifier lines ship with the row.
…ry harness task startAgentToolRuntime folds runner-provided target metadata and tool-published metadata into a BrowserSessionInfo that every prepared adapter (claude_code, codex, mastra, pi, eve, deepagents, fx, cursor) carries as browserSession. defineExternalHarness logs 'Browserbase session: <url>' (level 0, category session) right after adapter preparation and stamps sessionUrl / browserbaseSessionId / browserProvider on the TaskResult; the OTEL task span carries them too. Adapter setup chatter moves to level 2 so the session line heads output.logs. browse_cli logs the bare provider because the daemon does not report its session id.
Contributor
There was a problem hiding this comment.
11 issues found across 51 files (changes from recent commits).
Confidence score: 2/5
packages/evals/framework/harnesses/externalRunner.tsandpackages/evals/framework/harnesses/traceLog.tscan persist credential-bearingstopReasonvalues in trace logs andresult.logs, creating a concrete sensitive-data exposure risk — pass the sanitized stop reason through every logging path.packages/integrations/core/src/facade/stdio-server.tsstill acceptssession_inforequests despite omitting the method fromtools/list, so an agent can obtain the Browserbase session ID — move the authorization or dispatch guard to the server-side handler.- Failure classification is incomplete across
packages/integrations/eve-sdk/src/session.ts,fx-sdk/src/session.ts,pi-sdk/src/session.ts,claude-agent-sdk/src/session.ts, andcursor-sdk/src/session.ts, which can hide real SDK/tool errors at debug level — classify nested and explicit error fields as failures and add coverage for each path. packages/evals/framework/browserSession.tscan construct the wrong URL when a session ID contains reserved characters, whilepackages/evals/framework/harnesses/traceLog.tscan emit inconsistent shared tool names and formats — encode IDs as one path segment and align trace output with the advertised surface.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/framework/browserSession.ts">
<violation number="1" location="packages/evals/framework/browserSession.ts:15">
P2: When a session ID contains a reserved path character, this helper generates a URL for a different path or query. Encode the ID as one URL path segment before interpolation.
(Based on your team's feedback about encoding session IDs in URL path segments.)</violation>
</file>
<file name="packages/evals/framework/harnesses/traceLog.ts">
<violation number="1" location="packages/evals/framework/harnesses/traceLog.ts:106">
P2: When an SDK stop reason contains credentials or other sensitive error text, this line writes the unsanitized value into persisted trace logs. Use the sanitized stop reason when constructing the trace input, or sanitize `outcome.stopReason` before adding it to the message.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging and returning them.)</violation>
<violation number="2" location="packages/evals/framework/harnesses/traceLog.ts:134">
P2: When the Fx or Eve harness emits its native tool name, the trace keeps the harness prefix instead of producing the advertised shared surface name. This also prints run code as JSON and skips snapshot/screenshot formatting; normalize `mcp_stagehand_*` and `stagehand__*` before the existing stagehand regex.</violation>
</file>
<file name="packages/integrations/eve-sdk/src/session.ts">
<violation number="1" location="packages/integrations/eve-sdk/src/session.ts:374">
P2: When an Eve tool returns `action.result` with a non-`completed` status or `result.isError === true`, this predicate leaves it at debug level 2, hiding the failure from normal harness logs. Classify failed `action.result` payloads as errors as well.</violation>
</file>
<file name="packages/integrations/cursor-sdk/src/session.ts">
<violation number="1" location="packages/integrations/cursor-sdk/src/session.ts:402">
P2: When a Cursor `result` carries `is_error: true` with no non-success subtype, this classifier logs it at debug level even though `resolveCursorStatus` treats it as an SDK failure. Include `event.is_error === true` in the result error predicate.</violation>
</file>
<file name="packages/integrations/fx-sdk/src/session.ts">
<violation number="1" location="packages/integrations/fx-sdk/src/session.ts:663">
P2: When fx ends a committed turn as cancelled/interrupted or returns an ask error, `logFxEvent` records the failure at level 2. Classify the turn and ask failure fields as `isError` so SDK failures remain visible at level 1.</violation>
</file>
<file name="packages/integrations/pi-sdk/src/session.ts">
<violation number="1" location="packages/integrations/pi-sdk/src/session.ts:308">
P2: When Pi reports a provider failure through `message_end.message.stopReason === "error"`, this predicate classifies it as level 2. `runPiSession` records the stop reason without logging a warning on that path, so the default persisted logs hide the failure; classify that assistant message as an error too.</violation>
</file>
<file name="packages/integrations/claude-agent-sdk/src/session.ts">
<violation number="1" location="packages/integrations/claude-agent-sdk/src/session.ts:289">
P2: When a Claude tool returns an error, the SDK places `is_error` on the nested `user.message.content` tool-result block, not necessarily on the outer message. Inspect those blocks before classifying the event, otherwise the failure is logged at level 2 and omitted from default persisted logs.</violation>
</file>
<file name="packages/evals/tests/core/stagehand-facade.test.ts">
<violation number="1" location="packages/evals/tests/core/stagehand-facade.test.ts:238">
P2: The stderr-based assertions can race: start() resolves as soon as the session_info stdout reply is processed (routeServerLine settles the call), while the "fake tools/call session_info" line is delivered by the separate child.stderr 'data' handler. Nothing waits for stderr to drain, so the assertion on logger.lines can run before the line is captured. Wait for the expected line (bounded) instead of asserting synchronously after start().</violation>
</file>
<file name="packages/evals/framework/harnesses/externalRunner.ts">
<violation number="1" location="packages/evals/framework/harnesses/externalRunner.ts:251">
P1: When a verifier runs and the SDK supplies a credential-bearing `outcome.stopReason`, this call sends the unsanitized value to the trace logger and `result.logs`. Pass `sanitizedStopReason` to the trace emitter so the trace cannot bypass the existing redaction.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging them.)</violation>
</file>
<file name="packages/integrations/core/src/facade/stdio-server.ts">
<violation number="1" location="packages/integrations/core/src/facade/stdio-server.ts:111">
P2: Because the agent relay forwards every `tools/call` to this dispatcher, omitting `session_info` from `tools/list` does not keep it runner-only; an agent can request it and receive the Browserbase session ID. Move this metadata exchange to an authenticated out-of-band runner channel, or reject this name for agent-originated calls instead of relying on discovery omission.
(Based on your team's feedback about runner-side facade session operations.)</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // every harness logs the same shape; a formatting bug must never fail | ||
| // the grade. | ||
| try { | ||
| emitTrajectoryTrace(logger, { trajectory, outcome, isFacadeTool }); |
Contributor
There was a problem hiding this comment.
P1: When a verifier runs and the SDK supplies a credential-bearing outcome.stopReason, this call sends the unsanitized value to the trace logger and result.logs. Pass sanitizedStopReason to the trace emitter so the trace cannot bypass the existing redaction.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging them.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/externalRunner.ts, line 251:
<comment>When a verifier runs and the SDK supplies a credential-bearing `outcome.stopReason`, this call sends the unsanitized value to the trace logger and `result.logs`. Pass `sanitizedStopReason` to the trace emitter so the trace cannot bypass the existing redaction.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging them.) </comment>
<file context>
@@ -242,6 +244,18 @@ export async function runExternalHarnessTask<TRaw>({
+ // every harness logs the same shape; a formatting bug must never fail
+ // the grade.
+ try {
+ emitTrajectoryTrace(logger, { trajectory, outcome, isFacadeTool });
+ } catch (traceError) {
+ logger.warn({
</file context>
| } | ||
|
|
||
| export function browserbaseSessionUrl(sessionId: string): string { | ||
| return `https://www.browserbase.com/sessions/${sessionId}`; |
Contributor
There was a problem hiding this comment.
P2: When a session ID contains a reserved path character, this helper generates a URL for a different path or query. Encode the ID as one URL path segment before interpolation.
(Based on your team's feedback about encoding session IDs in URL path segments.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/browserSession.ts, line 15:
<comment>When a session ID contains a reserved path character, this helper generates a URL for a different path or query. Encode the ID as one URL path segment before interpolation.
(Based on your team's feedback about encoding session IDs in URL path segments.) </comment>
<file context>
@@ -0,0 +1,89 @@
+}
+
+export function browserbaseSessionUrl(sessionId: string): string {
+ return `https://www.browserbase.com/sessions/${sessionId}`;
+}
+
</file context>
| } else if (name.includes(".")) { | ||
| name = name.slice(name.lastIndexOf(".") + 1); | ||
| } | ||
| if (/^stagehand_(?:browser_)?(run|snapshot|screenshot)$/u.test(name)) { |
Contributor
There was a problem hiding this comment.
P2: When the Fx or Eve harness emits its native tool name, the trace keeps the harness prefix instead of producing the advertised shared surface name. This also prints run code as JSON and skips snapshot/screenshot formatting; normalize mcp_stagehand_* and stagehand__* before the existing stagehand regex.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/traceLog.ts, line 134:
<comment>When the Fx or Eve harness emits its native tool name, the trace keeps the harness prefix instead of producing the advertised shared surface name. This also prints run code as JSON and skips snapshot/screenshot formatting; normalize `mcp_stagehand_*` and `stagehand__*` before the existing stagehand regex.</comment>
<file context>
@@ -0,0 +1,239 @@
+ } else if (name.includes(".")) {
+ name = name.slice(name.lastIndexOf(".") + 1);
+ }
+ if (/^stagehand_(?:browser_)?(run|snapshot|screenshot)$/u.test(name)) {
+ name = name.replace(/^stagehand_(?:browser_)?/u, "");
+ }
</file context>
| const message = [ | ||
| "result", | ||
| outcome.status, | ||
| ...(outcome.stopReason ? [singleLine(outcome.stopReason)] : []), |
Contributor
There was a problem hiding this comment.
P2: When an SDK stop reason contains credentials or other sensitive error text, this line writes the unsanitized value into persisted trace logs. Use the sanitized stop reason when constructing the trace input, or sanitize outcome.stopReason before adding it to the message.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging and returning them.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/traceLog.ts, line 106:
<comment>When an SDK stop reason contains credentials or other sensitive error text, this line writes the unsanitized value into persisted trace logs. Use the sanitized stop reason when constructing the trace input, or sanitize `outcome.stopReason` before adding it to the message.
(Based on your team's feedback about sanitizing SDK-derived error strings before logging and returning them.) </comment>
<file context>
@@ -0,0 +1,239 @@
+ const message = [
+ "result",
+ outcome.status,
+ ...(outcome.stopReason ? [singleLine(outcome.stopReason)] : []),
+ `steps=${trajectory.steps.length}`,
+ ...(facadeCalls !== undefined ? [`facade_calls=${facadeCalls}`] : []),
</file context>
|
|
||
| export function logEveEvent(logger: HarnessLogger, event: EveEvent): void { | ||
| const level = harnessEventLogLevel(event.type, { | ||
| isError: event.type.endsWith(".failed"), |
Contributor
There was a problem hiding this comment.
P2: When an Eve tool returns action.result with a non-completed status or result.isError === true, this predicate leaves it at debug level 2, hiding the failure from normal harness logs. Classify failed action.result payloads as errors as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/eve-sdk/src/session.ts, line 374:
<comment>When an Eve tool returns `action.result` with a non-`completed` status or `result.isError === true`, this predicate leaves it at debug level 2, hiding the failure from normal harness logs. Classify failed `action.result` payloads as errors as well.</comment>
<file context>
@@ -369,13 +370,18 @@ export function buildEveTranscript(events: EveEvent[]): string {
export function logEveEvent(logger: HarnessLogger, event: EveEvent): void {
+ const level = harnessEventLogLevel(event.type, {
+ isError: event.type.endsWith(".failed"),
+ hasContent: event.type.endsWith(".completed") || event.type === "action.result",
+ });
</file context>
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 ?? ""))), |
Contributor
There was a problem hiding this comment.
P2: When fx ends a committed turn as cancelled/interrupted or returns an ask error, logFxEvent records the failure at level 2. Classify the turn and ask failure fields as isError so SDK failures remain visible at level 1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/fx-sdk/src/session.ts, line 663:
<comment>When fx ends a committed turn as cancelled/interrupted or returns an ask error, `logFxEvent` records the failure at level 2. Classify the turn and ask failure fields as `isError` so SDK failures remain visible at level 1.</comment>
<file context>
@@ -658,11 +659,19 @@ export function buildFxTranscript(events: FxEvent[]): string {
export function logFxEvent(logger: HarnessLogger, event: FxEvent): void {
+ const level = harnessEventLogLevel(event.type, {
+ isError:
+ (event.type === "stderr" && /\b(?:error|fatal|failed|panic)\b/iu.test(event.line)) ||
+ (event.type === "tool_step" &&
</file context>
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), |
Contributor
There was a problem hiding this comment.
P2: When Pi reports a provider failure through message_end.message.stopReason === "error", this predicate classifies it as level 2. runPiSession records the stop reason without logging a warning on that path, so the default persisted logs hide the failure; classify that assistant message as an error too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/pi-sdk/src/session.ts, line 308:
<comment>When Pi reports a provider failure through `message_end.message.stopReason === "error"`, this predicate classifies it as level 2. `runPiSession` records the stop reason without logging a warning on that path, so the default persisted logs hide the failure; classify that assistant message as an error too.</comment>
<file context>
@@ -302,11 +303,17 @@ export function buildPiTranscript(events: PiEvent[]): string {
export function logPiEvent(logger: HarnessLogger, event: PiEvent): void {
+ const type = String(event.type ?? "unknown");
+ const level = harnessEventLogLevel(type, {
+ isError: type === "error" || (type === "tool_execution_end" && event.isError === true),
+ hasContent: type === "message_end" || type === "tool_execution_end",
+ });
</file context>
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, |
Contributor
There was a problem hiding this comment.
P2: When a Claude tool returns an error, the SDK places is_error on the nested user.message.content tool-result block, not necessarily on the outer message. Inspect those blocks before classifying the event, otherwise the failure is logged at level 2 and omitted from default persisted logs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/claude-agent-sdk/src/session.ts, line 289:
<comment>When a Claude tool returns an error, the SDK places `is_error` on the nested `user.message.content` tool-result block, not necessarily on the outer message. Inspect those blocks before classifying the event, otherwise the failure is logged at level 2 and omitted from default persisted logs.</comment>
<file context>
@@ -277,11 +282,19 @@ export function buildClaudeCodeTranscript(messages: ClaudeSdkMessage[]): string
+ const level = harnessEventLogLevel(type, {
+ isError:
+ (type === "result" && message.subtype !== undefined && message.subtype !== "success") ||
+ message.is_error === true,
+ hasContent: type === "assistant" || type === "user" || type === "result",
+ });
</file context>
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( |
Contributor
There was a problem hiding this comment.
P2: The stderr-based assertions can race: start() resolves as soon as the session_info stdout reply is processed (routeServerLine settles the call), while the "fake tools/call session_info" line is delivered by the separate child.stderr 'data' handler. Nothing waits for stderr to drain, so the assertion on logger.lines can run before the line is captured. Wait for the expected line (bounded) instead of asserting synchronously after start().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/tests/core/stagehand-facade.test.ts, line 238:
<comment>The stderr-based assertions can race: start() resolves as soon as the session_info stdout reply is processed (routeServerLine settles the call), while the "fake tools/call session_info" line is delivered by the separate child.stderr 'data' handler. Nothing waits for stderr to drain, so the assertion on logger.lines can run before the line is captured. Wait for the expected line (bounded) instead of asserting synchronously after start().</comment>
<file context>
@@ -174,6 +212,58 @@ describe("stagehand facade tool surface", () => {
+ expect(
+ logger.lines.find((line) => line.message.startsWith("Started runner-owned"))?.level,
+ ).toBe(2);
+ expect(logger.lines.some((line) => line.message === "fake tools/call session_info")).toBe(
+ true,
+ );
</file context>
| // Runner-side only (absent from tools/list): launches the browser if | ||
| // needed and reports where it lives so the harness can log the | ||
| // Browserbase session URL before the agent's first call. | ||
| const browser = (await ensureResources()).browser; |
Contributor
There was a problem hiding this comment.
P2: Because the agent relay forwards every tools/call to this dispatcher, omitting session_info from tools/list does not keep it runner-only; an agent can request it and receive the Browserbase session ID. Move this metadata exchange to an authenticated out-of-band runner channel, or reject this name for agent-originated calls instead of relying on discovery omission.
(Based on your team's feedback about runner-side facade session operations.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/facade/stdio-server.ts, line 111:
<comment>Because the agent relay forwards every `tools/call` to this dispatcher, omitting `session_info` from `tools/list` does not keep it runner-only; an agent can request it and receive the Browserbase session ID. Move this metadata exchange to an authenticated out-of-band runner channel, or reject this name for agent-originated calls instead of relying on discovery omission.
(Based on your team's feedback about runner-side facade session operations.) </comment>
<file context>
@@ -103,6 +104,18 @@ server.server.setRequestHandler(CallToolRequestSchema, async (request) => {
+ // Runner-side only (absent from tools/list): launches the browser if
+ // needed and reports where it lives so the harness can log the
+ // Browserbase session URL before the agent's first call.
+ const browser = (await ensureResources()).browser;
+ return textResult(
+ JSON.stringify({
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2866 (← #2812, top of the harness wave). Everything found and fixed while smoke-gating all 8 external harnesses on HardBenchmark (Browserbase, 5 tasks/cell).
Tool surface
stagehand_facadeis the Playwright-batch surface. The shipped facaderunalready executed viaexperimentalBatch; this syncs its runtime with the experiment branch's improvements (a11y-tree fallback whengetByRolemisses in DOM, shadow-root XPath hops, 10 s default locator timeout) and rewrites the prompt/description to the Playwright idiom (page/context/browserin scope). snapshot/screenshot always on, no env knobs.stagehand_facade_legacypreserves the previous prompt contract (--surface=legacy), wired into all 8 harness surface lists, so runs are never silently mixed.page.frameLocator/locator.frameLocator/contentFrame()implemented across cross-origin iframes (deep-locator hop notation; a11y-routed frame-scopedgetByRole/getByLabel/getByText). Clearer strict-mode errors (lists candidates + disambiguation), layout-error wording, lenient bare snapshot ids.HardBenchmark validity audit → n = 45
scripts/audit-hardbenchmark.ts(reusable): Browserbase verified-mode reachability of every start URL + the deep pages each task needs, proxy retry, rubric schema, achievability vs prior trajectories, stop-before-purchase flags.valid:false, builder skips + logs):heb_comparison_shopping_1(PerimeterX wall through verified+proxy). 20 flaggedverdict_review: stop-before-purchase(carried into row metadata). Review list:datasets/hardbenchmark/AUDIT-REVIEW.md.Harness fixes (each gate-verified live)
google/gemini-2.5-flash) → every criterion "Fused judgment call failed", runs silently unscored. Now defaults togoogle/gemini-3.5-flash; explicitEVAL_VERIFIER_MODELstill fails loudly on a missing key.readOnlyHintneed approval and headless there is no reviewer → "user cancelled MCP tool call"; the model fell back to the operator's global~/.codextools and "passed" via curl. Fix:default_tools_approval_mode="approve"on runner-mounted servers + per-run isolatedCODEX_HOME. New metricsfacade_tool_calls/facade_tool_call_failuresand apassesWithoutBrowserUsegate so browserless passes are visible."*"in the deny list hid fx's ownmcp_select_toolmeta-tools; workspace was created beside (not below) the throwaway$HOME, soAGENTS.md/.fx.jsonnever loaded;web_fetchis not permission-gated in fx 0.0.3 (guidance added).function_callJSON (flatten_text).tool_surface,model,provider,dataset,task_count.Smoke results (gate = trajectories paired, usage present, live verdicts, facade actually driven)
eve ✅ · deepagents ✅ · claude_code ✅ · mastra ✅ · pi ✅ · fx ✅ · codex ✅ (post-fix) · cursor pending login.
In flight on this branch
Unified readable step trace in
output.logs(one format for all harnesses) + Browserbase session URL as the first log line.Summary by cubic
Turns
stagehand_facadeinto the Playwright-batch surface for all external harnesses and fixes the harness bugs found while smoke-gating on HardBenchmark.Tool surface
stagehand_facadenow targets the Playwright idiom (page/context/browserin scope);stagehand_facade_legacykeeps the old prompt contract and is wired into every harness so runs never mix.getByRolemisses, shadow-root XPath hops, a 10 s default locator timeout, cross-originframeLocator, and clearer strict-mode and layout errors.Harness fixes
google/gemini-3.5-flash; the previous default was retired and silently unscored every run.default_tools_approval_mode="approve"and per-runCODEX_HOME; newfacade_tool_callsandfacade_tool_call_failuresmetrics plus apassesWithoutBrowserUsegate expose browserless passes.AGENTS.mdguidance by nesting the workspace under the run home and stops hiding its own MCP meta-tools behind the deny catch-all.function_callJSON into assistant text.stagehand_codereport their Browserbase session before the agent starts; the session URL is the first line of every task log and lands on task results and spans.Written for commit d307b1a. Summary will update on new commits.