feat(integrations): add shared harness contract module to core - #2746
feat(integrations): add shared harness contract module to core#2746miguelg719 wants to merge 3 commits into
Conversation
Adds @browserbasehq/stagehand-integrations/harness with: - the harness mount contract (AgentMount, AgentRunToolSpec, AGENT_RUN_TOOL_* constants) moved verbatim from evals core/contracts/tool.ts, which now re-exports them - StartedSurface / HarnessTask / HarnessLogger / HarnessAdapterError, the narrow seam types for upcoming harness adapter packages - sanitizeErrorMessage: single merged copy (the codexCodeBridge variant was a 2-rule subset of the stdio-server's 5; both callers now share the superset) - buildAllowlistedEnv: single copy replacing 4 identical ones (vercel-ai, mastra, claude-code, codex examples) No behavior change except bridge redaction widening to the superset.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 21 files
Confidence score: 5/5
- In
packages/integrations/core/src/harness/index.ts, usingexport *can unintentionally exposesanitizeErrorMessageandbuildAllowlistedEnvas part of the public package API, which risks accidental external coupling and future breaking changes when internals evolve — replace wildcard re-exports with an explicit export list (or keep these utilities unexported) to constrain the public 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/integrations/core/src/harness/index.ts">
<violation number="1" location="packages/integrations/core/src/harness/index.ts:3">
P3: `harness/index.ts` uses `export *` to re-export `sanitizeErrorMessage` (redact.ts) and `buildAllowlistedEnv` (env.ts), which makes these generic runtime utilities part of the public `@browserbasehq/stagehand-integrations/harness` surface — the same surface also exposing the contract types for external adapter packages. This widens the stable API with utility functions outside the contract's scope and, because of `export *`, automatically promotes any future export added to redact.ts/env.ts to public API without a deliberate decision. Note the sibling `facade/index.ts` entry deliberately narrows its surface with explicit named exports; the new harness entry does the opposite. Narrow the re-exports to just the two helpers the callers need (e.g. `export { sanitizeErrorMessage } from "./redact.js"; export { buildAllowlistedEnv } from "./env.js";`), or keep the utilities in a separate internal module and re-export them explicitly.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Evals as Evals Package
participant Harness as Integrations Harness
participant Bridge as Codex Code Bridge
participant Facade as Facade Stdio Server
participant Adapters as Adapter Clients (Vercel-AI, Mastra, Claude-Code, Codex)
Note over Evals,Harness: Contract Module Boundary
Evals->>Harness: Import AgentMount, AgentRunToolSpec, AGENT_RUN_TOOL_*
Harness-->>Evals: Re-exported contracts (evals depends on integrations)
Note over Bridge,Facade: Shared Redaction Utility
Bridge->>Harness: sanitizeErrorMessage(message)
Facade->>Harness: sanitizeErrorMessage(message)
Harness-->>Bridge: Redacted message (5-rule superset)
Harness-->>Facade: Redacted message (5-rule superset)
Note over Adapters,Harness: Shared Environment Allowlist
Adapters->>Harness: buildAllowlistedEnv()
Harness->>Harness: Filter env (STAGEHAND_*, BROWSERBASE_*)
Harness-->>Adapters: Allowlisted env object
Note over Evals,Harness: New Seam Types (future adapters)
Harness->>Harness: StartedSurface, HarnessTask, HarnessLogger, HarnessAdapterError
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,3 @@ | |||
| export * from "./contract.js"; | |||
| export * from "./env.js"; | |||
| export * from "./redact.js"; | |||
There was a problem hiding this comment.
P3: harness/index.ts uses export * to re-export sanitizeErrorMessage (redact.ts) and buildAllowlistedEnv (env.ts), which makes these generic runtime utilities part of the public @browserbasehq/stagehand-integrations/harness surface — the same surface also exposing the contract types for external adapter packages. This widens the stable API with utility functions outside the contract's scope and, because of export *, automatically promotes any future export added to redact.ts/env.ts to public API without a deliberate decision. Note the sibling facade/index.ts entry deliberately narrows its surface with explicit named exports; the new harness entry does the opposite. Narrow the re-exports to just the two helpers the callers need (e.g. export { sanitizeErrorMessage } from "./redact.js"; export { buildAllowlistedEnv } from "./env.js";), or keep the utilities in a separate internal module and re-export them explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/src/harness/index.ts, line 3:
<comment>`harness/index.ts` uses `export *` to re-export `sanitizeErrorMessage` (redact.ts) and `buildAllowlistedEnv` (env.ts), which makes these generic runtime utilities part of the public `@browserbasehq/stagehand-integrations/harness` surface — the same surface also exposing the contract types for external adapter packages. This widens the stable API with utility functions outside the contract's scope and, because of `export *`, automatically promotes any future export added to redact.ts/env.ts to public API without a deliberate decision. Note the sibling `facade/index.ts` entry deliberately narrows its surface with explicit named exports; the new harness entry does the opposite. Narrow the re-exports to just the two helpers the callers need (e.g. `export { sanitizeErrorMessage } from "./redact.js"; export { buildAllowlistedEnv } from "./env.js";`), or keep the utilities in a separate internal module and re-export them explicitly.</comment>
<file context>
@@ -0,0 +1,3 @@
+export * from "./contract.js";
+export * from "./env.js";
+export * from "./redact.js";
</file context>
There was a problem hiding this comment.
These utilities are deliberately part of the harness surface — this module is the designated shared home for cross-package harness plumbing (contract + redaction + env allowlisting), consumed by the adapter packages and evals. All packages involved are private/unpublished, so there's no external API-stability concern.
cubic P2 on #2746 — core uses explicit .js relative ESM specifiers.
… utilities The AgentMount/AgentRunToolSpec contract abstracts over evals' tool surface registry — an evals concern, not an integrations one. It moves back to evals/core/contracts/tool.ts verbatim. The shared harness module keeps only what thin SDK adapters genuinely need: the merged sanitizeErrorMessage, buildAllowlistedEnv, HarnessLogger, and HarnessAdapterError. Also ignore evals run artifacts (.trajectories, rubric cache) — generated output that was dirtying trees and fmt scans.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Confidence score: 5/5
- In
packages/integrations/core/tests/harness.test.ts, removing the run-tool derivation test removes the check thatAGENT_RUN_TOOL_NAMEstays aligned withAGENT_RUN_TOOL_SERVER, which could allow a silent naming drift and break run-tool wiring at runtime—restore an equivalent assertion near the newAGENT_RUN_TOOL_NAMElocation inpackages/evals/cor...(or add a cross-package contract test).
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/tests/harness.test.ts">
<violation number="1" location="packages/integrations/core/tests/harness.test.ts:2">
P3: Removing the run-tool derivation test drops the guard that keeps AGENT_RUN_TOOL_NAME (`mcp__${AGENT_RUN_TOOL_SERVER}__run`) in sync with AGENT_RUN_TOOL_SERVER. Since AGENT_RUN_TOOL_NAME now lives in packages/evals/core/contracts/tool.ts, re-add the assertion as an evals test (e.g. in tests/framework/harnessObservations.test.ts) so the contract is still covered where it is defined.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,46 @@ | |||
| import { afterEach, describe, expect, it, vi } from "vitest"; | |||
| import { buildAllowlistedEnv, sanitizeErrorMessage } from "../src/harness/index.js"; | |||
There was a problem hiding this comment.
P3: Removing the run-tool derivation test drops the guard that keeps AGENT_RUN_TOOL_NAME (mcp__${AGENT_RUN_TOOL_SERVER}__run) in sync with AGENT_RUN_TOOL_SERVER. Since AGENT_RUN_TOOL_NAME now lives in packages/evals/core/contracts/tool.ts, re-add the assertion as an evals test (e.g. in tests/framework/harnessObservations.test.ts) so the contract is still covered where it is defined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/core/tests/harness.test.ts, line 2:
<comment>Removing the run-tool derivation test drops the guard that keeps AGENT_RUN_TOOL_NAME (`mcp__${AGENT_RUN_TOOL_SERVER}__run`) in sync with AGENT_RUN_TOOL_SERVER. Since AGENT_RUN_TOOL_NAME now lives in packages/evals/core/contracts/tool.ts, re-add the assertion as an evals test (e.g. in tests/framework/harnessObservations.test.ts) so the contract is still covered where it is defined.</comment>
<file context>
@@ -1,20 +1,11 @@
- buildAllowlistedEnv,
- sanitizeErrorMessage,
-} from "../src/harness/index.js";
+import { buildAllowlistedEnv, sanitizeErrorMessage } from "../src/harness/index.js";
describe("harness contract", () => {
</file context>
|
Reshaped after review feedback (see latest commit): the |
1 participant
Stacked on #2743. Part 1 of the harness consolidation stack (see plan).
What
Adds
@browserbasehq/stagehand-integrations/harness— the shared contract module for harness adapters:AgentMount,AgentRunToolSpec,AGENT_RUN_TOOL_SERVER/_NAME/_RESERVED_HANDLESmoved verbatim frompackages/evals/core/contracts/tool.ts, which now re-exports them (every existing evals import compiles unchanged). Integrations never imports from evals — dependency direction is evals → integrations.StartedSurface,HarnessTask,HarnessLogger(deliberately nogetLogs),HarnessAdapterError.sanitizeErrorMessagededupe: the codexCodeBridge copy was a 2-rule subset of the facade stdio-server's 5 rules (drifted). Single merged superset inharness/redact.ts; both callers rewired. Only behavior change in the PR: bridge redaction widens to the superset.buildAllowlistedEnvdedupe: one copy replaces 4 identical ones (vercel-ai, mastra, claude-code, codex examples). All four verified identical before merging.Verification
core/tests/harness.test.ts: per-rule redaction assertions (both source rule sets survive the merge), env allowlist behavior, run-tool constant derivationcodex exec reviewsecond-opinion pass: no source findingsSummary by cubic
Creates
@browserbasehq/stagehand-integrations/harnessto share harness utilities and centralize error redaction and env allowlisting. The codex bridge’s error redaction changes from a 2‑rule subset to the 5‑rule superset, so more secrets are redacted; no other behavior changes.packages/evals/core/contracts/tool.ts;@browserbasehq/stagehand-integrations/harnessnow exportssanitizeErrorMessage,buildAllowlistedEnv,HarnessLogger, andHarnessAdapterErroronly.buildAllowlistedEnvacrossvercel-ai,mastra,claude-code, andcodex; example/tests update import paths to@browserbasehq/stagehand-integrations/harness.sanitizeErrorMessage; both the facade stdio server andcodexCodeBridgeimport the shared superset (explicit.jsspecifier used in core).packages/integrations/core/tests/harness.test.tsto cover redaction and env allowlist; expandspackages/evals/.gitignoreto exclude run artifacts.Written for commit 9c7eaa0. Summary will update on new commits.