2/2 Implement explicit model gateway - #2440
Conversation
|
d2b2604 to
7f253fa
Compare
There was a problem hiding this comment.
12 issues found and verified against the latest diff
Confidence score: 2/5
packages/server/llm/gatewayClient.tsis pointed at a non-deployed.../v1/llm/responsesroute, so Browserbase model generations will 404 at runtime and effectively take the gateway path down — switch provider configuration to the deployed Model Gateway Responses endpoint before merging.packages/sdk-python/src/stagehand/_generated/models.pyintroduced incompatible model config changes (KnownModelConfig.api_keynow required andbrowserbase/autostring construction failing), which can block SDK initialization and make gateway access unusable for Python clients — restore optionalapi_keyhandling in validation and routebrowserbase/strings through the Browserbase config types.packages/server/services/llmService.tsalways selecting.responses(...)means Chat Completions-compatible custom endpoints cannot generate, creating user-facing regressions for advertised OpenAI-compatible integrations — branch to.chat(...)for those providers.- Coverage gaps across
openAiCompatibleClient.ts,stagehandApi.ts,stagehandController.ts, and theact/observe/extractservices raise follow-on regression risk around baseURL/apiKey/headers, region routing, and gateway-context forwarding — add focused tests on these new paths to lock behavior before future refactors.
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/server/llm/openAiCompatibleClient.ts">
<violation number="1" location="packages/server/llm/openAiCompatibleClient.ts:6">
P3: Custom endpoint connection settings lack coverage, so regressions that drop or misroute `baseURL`, optional `apiKey`, or custom `headers` will still pass the gateway test. Add focused tests that mock `createOpenAI` and verify these settings for authenticated and header-only endpoints.
(Based on your team's feedback about unit tests for new behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/server/clients/stagehandApi.ts">
<violation number="1" location="packages/server/clients/stagehandApi.ts:14">
P3: Regional endpoint routing has no executable coverage. Add focused cases for each `BrowserbaseRegion` and `undefined`, since a typo or fallback regression sends sessions to a different region and the API rejects the request.</violation>
</file>
<file name="packages/sdk-python/src/stagehand/_generated/models.py">
<violation number="1" location="packages/sdk-python/src/stagehand/_generated/models.py:228">
P1: `Stagehand(model="browserbase/auto")` cannot initialize, leaving the gateway unusable through the SDK's string-model constructor. Route `browserbase/` model strings through `BrowserbaseModelConfig`/`BrowserbaseUnlistedModelConfig` instead of always constructing `KnownModelConfig`.</violation>
<violation number="2" location="packages/sdk-python/src/stagehand/_generated/models.py:772">
P0: `KnownModelConfig.api_key` changed from optional (`Optional[StrictStr] = None`) to required (`StrictStr`), but the SDK's `_model_config()` helper in `client_models.py` explicitly omits `api_key` from the validation dict when no API key is provided. This means any existing code path that creates a model config without an API key — such as calling `Stagehand(model="openai/gpt-5")` without `model_api_key` (line 331 of `stagehand.py` passes `api_key=model_api_key` which defaults to `None`) — will now raise a `ValidationError` at runtime. Recommend keeping `api_key` optional on `KnownModelConfig` or updating `_model_config()` to handle the required field.</violation>
</file>
<file name="packages/server/services/actService.ts">
<violation number="1" location="packages/server/services/actService.ts:243">
P3: Act gateway forwarding has no service-level regression test; add focused coverage that `act()` with a Browserbase model forwards its gateway context (including the two-step/self-heal paths if applicable).
(Based on your team's feedback about unit tests for changed behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/server/controllers/stagehandController.ts">
<violation number="1" location="packages/server/controllers/stagehandController.ts:50">
P3: Gateway routing through the RPC `act`, `observe`, and `extract` paths is untested; current tests only cover `llmService.generate` directly. Add focused controller coverage so a future omitted or misplaced `gateway` argument does not silently break gateway models for these public operations.
(Based on your team's feedback about unit tests for new behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/server/services/extractService.ts">
<violation number="1" location="packages/server/services/extractService.ts:112">
P3: Gateway-backed extraction has no service-level regression coverage, so a future change can silently drop the context and break Browserbase extraction. Add focused tests for forwarding `GatewayContext` and for the missing-context failure path.
(Based on your team's feedback about adding unit tests for changed behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/server/services/observeService.ts">
<violation number="1" location="packages/server/services/observeService.ts:89">
P3: Gateway-backed `observe()` routing has no service-level regression test. Add a focused observation test with a Browserbase model and gateway context so this new forwarding path remains covered.
(Based on your team's feedback about adding unit tests for new behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/server/llm/aiSdkClient.ts">
<violation number="1" location="packages/server/llm/aiSdkClient.ts:125">
P3: Unlisted direct models now reach provider factories without catalog validation, but no AI SDK-client test covers that new path. Add focused unlisted-ID coverage (including a slash-containing ID) so forwarding behavior cannot regress.
(Based on your team's feedback about unit tests for new behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/server/services/llmService.ts">
<violation number="1" location="packages/server/services/llmService.ts:33">
P2: Custom endpoints that expose the common Chat Completions-compatible API cannot generate here: this route always selects the helper's `.responses(...)` model. Use `.chat(...)` for the advertised OpenAI-compatible contract, or explicitly restrict and document custom endpoints as Responses-compatible.</violation>
</file>
<file name="packages/server/llm/gatewayClient.ts">
<violation number="1" location="packages/server/llm/gatewayClient.ts:25">
P3: Gateway transport wiring has no focused test, so an invalid endpoint or missing cloud headers reaches production undetected. Add tests that mock the provider request and assert the resolved URL plus both Browserbase headers.
(Based on your team's feedback about unit tests for new behavior.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/server/llm/gatewayClient.ts:31">
P0: All Browserbase model generations will 404 because this produces `<regional API>/v1/llm/responses`, which is not a deployed route. Configure the provider with the deployed Model Gateway Responses endpoint before dispatching requests.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ): LanguageModel { | ||
| return createOpenAI({ | ||
| apiKey: gateway.apiKey, | ||
| baseURL: `${gateway.apiUrl}/llm`, |
There was a problem hiding this comment.
P0: All Browserbase model generations will 404 because this produces <regional API>/v1/llm/responses, which is not a deployed route. Configure the provider with the deployed Model Gateway Responses endpoint before dispatching requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/llm/gatewayClient.ts, line 31:
<comment>All Browserbase model generations will 404 because this produces `<regional API>/v1/llm/responses`, which is not a deployed route. Configure the provider with the deployed Model Gateway Responses endpoint before dispatching requests.</comment>
<file context>
@@ -0,0 +1,37 @@
+): LanguageModel {
+ return createOpenAI({
+ apiKey: gateway.apiKey,
+ baseURL: `${gateway.apiUrl}/llm`,
+ headers: {
+ "x-bb-api-key": gateway.apiKey,
</file context>
| api_key: Annotated[ | ||
| Optional[StrictStr], Field(examples=["sk-some-openai-api-key"], min_length=1) | ||
| ] = None | ||
| StrictStr, Field(examples=["sk-some-openai-api-key"], min_length=1) |
There was a problem hiding this comment.
P0: KnownModelConfig.api_key changed from optional (Optional[StrictStr] = None) to required (StrictStr), but the SDK's _model_config() helper in client_models.py explicitly omits api_key from the validation dict when no API key is provided. This means any existing code path that creates a model config without an API key — such as calling Stagehand(model="openai/gpt-5") without model_api_key (line 331 of stagehand.py passes api_key=model_api_key which defaults to None) — will now raise a ValidationError at runtime. Recommend keeping api_key optional on KnownModelConfig or updating _model_config() to handle the required field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-python/src/stagehand/_generated/models.py, line 772:
<comment>`KnownModelConfig.api_key` changed from optional (`Optional[StrictStr] = None`) to required (`StrictStr`), but the SDK's `_model_config()` helper in `client_models.py` explicitly omits `api_key` from the validation dict when no API key is provided. This means any existing code path that creates a model config without an API key — such as calling `Stagehand(model="openai/gpt-5")` without `model_api_key` (line 331 of `stagehand.py` passes `api_key=model_api_key` which defaults to `None`) — will now raise a `ValidationError` at runtime. Recommend keeping `api_key` optional on `KnownModelConfig` or updating `_model_config()` to handle the required field.</comment>
<file context>
@@ -725,8 +769,8 @@ class KnownModelConfig(WireModel):
api_key: Annotated[
- Optional[StrictStr], Field(examples=["sk-some-openai-api-key"], min_length=1)
- ] = None
+ StrictStr, Field(examples=["sk-some-openai-api-key"], min_length=1)
+ ]
"""
</file context>
| min_width: Optional[StrictFloat] = None | ||
|
|
||
|
|
||
| class BrowserbaseModelConfig(WireModel): |
There was a problem hiding this comment.
P1: Stagehand(model="browserbase/auto") cannot initialize, leaving the gateway unusable through the SDK's string-model constructor. Route browserbase/ model strings through BrowserbaseModelConfig/BrowserbaseUnlistedModelConfig instead of always constructing KnownModelConfig.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-python/src/stagehand/_generated/models.py, line 228:
<comment>`Stagehand(model="browserbase/auto")` cannot initialize, leaving the gateway unusable through the SDK's string-model constructor. Route `browserbase/` model strings through `BrowserbaseModelConfig`/`BrowserbaseUnlistedModelConfig` instead of always constructing `KnownModelConfig`.</comment>
<file context>
@@ -225,6 +225,31 @@ class BrowserbaseFingerprintScreen(WireModel):
min_width: Optional[StrictFloat] = None
+class BrowserbaseModelConfig(WireModel):
+ model_config = ConfigDict(
+ extra="forbid",
</file context>
| case "direct": | ||
| return await generateWithAiSdk(createAiSdkLanguageModel(target), params); | ||
| case "openai-compatible": | ||
| return await generateWithAiSdk(createOpenAICompatibleLanguageModel(target), params); |
There was a problem hiding this comment.
P2: Custom endpoints that expose the common Chat Completions-compatible API cannot generate here: this route always selects the helper's .responses(...) model. Use .chat(...) for the advertised OpenAI-compatible contract, or explicitly restrict and document custom endpoints as Responses-compatible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/services/llmService.ts, line 33:
<comment>Custom endpoints that expose the common Chat Completions-compatible API cannot generate here: this route always selects the helper's `.responses(...)` model. Use `.chat(...)` for the advertised OpenAI-compatible contract, or explicitly restrict and document custom endpoints as Responses-compatible.</comment>
<file context>
@@ -7,27 +7,34 @@ import type {
+ case "direct":
+ return await generateWithAiSdk(createAiSdkLanguageModel(target), params);
+ case "openai-compatible":
+ return await generateWithAiSdk(createOpenAICompatibleLanguageModel(target), params);
+ case "browserbase":
+ if (!gateway) {
</file context>
| import type { OpenAICompatibleModelTarget } from "./modelTarget.js"; | ||
|
|
||
| /** Creates an OpenAI Responses-compatible model for a caller-controlled endpoint. */ | ||
| export function createOpenAICompatibleLanguageModel( |
There was a problem hiding this comment.
P3: Custom endpoint connection settings lack coverage, so regressions that drop or misroute baseURL, optional apiKey, or custom headers will still pass the gateway test. Add focused tests that mock createOpenAI and verify these settings for authenticated and header-only endpoints.
(Based on your team's feedback about unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/llm/openAiCompatibleClient.ts, line 6:
<comment>Custom endpoint connection settings lack coverage, so regressions that drop or misroute `baseURL`, optional `apiKey`, or custom `headers` will still pass the gateway test. Add focused tests that mock `createOpenAI` and verify these settings for authenticated and header-only endpoints.
(Based on your team's feedback about unit tests for new behavior.) </comment>
<file context>
@@ -0,0 +1,14 @@
+import type { OpenAICompatibleModelTarget } from "./modelTarget.js";
+
+/** Creates an OpenAI Responses-compatible model for a caller-controlled endpoint. */
+export function createOpenAICompatibleLanguageModel(
+ target: OpenAICompatibleModelTarget,
+): LanguageModel {
</file context>
| selfHeal: state.initParams.selfHeal, | ||
| domSettleTimeoutMs: state.initParams.domSettleTimeoutMs, | ||
| cache: cacheService.buildCacheContext(state.initParams), | ||
| gateway: buildGatewayContext(state.initParams), |
There was a problem hiding this comment.
P3: Gateway routing through the RPC act, observe, and extract paths is untested; current tests only cover llmService.generate directly. Add focused controller coverage so a future omitted or misplaced gateway argument does not silently break gateway models for these public operations.
(Based on your team's feedback about unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/controllers/stagehandController.ts, line 50:
<comment>Gateway routing through the RPC `act`, `observe`, and `extract` paths is untested; current tests only cover `llmService.generate` directly. Add focused controller coverage so a future omitted or misplaced `gateway` argument does not silently break gateway models for these public operations.
(Based on your team's feedback about unit tests for new behavior.) </comment>
<file context>
@@ -46,6 +47,7 @@ export function createStagehandController(runtime: StagehandRuntime) {
selfHeal: state.initParams.selfHeal,
domSettleTimeoutMs: state.initParams.domSettleTimeoutMs,
cache: cacheService.buildCacheContext(state.initParams),
+ gateway: buildGatewayContext(state.initParams),
});
}
</file context>
| domElements: combinedTree, | ||
| schema: transformedSchema as z.ZodObject, | ||
| generate: (input) => llmService.generate(model, input, clientLLMGenerate), | ||
| generate: (input) => llmService.generate(model, input, clientLLMGenerate, gateway), |
There was a problem hiding this comment.
P3: Gateway-backed extraction has no service-level regression coverage, so a future change can silently drop the context and break Browserbase extraction. Add focused tests for forwarding GatewayContext and for the missing-context failure path.
(Based on your team's feedback about adding unit tests for changed behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/services/extractService.ts, line 112:
<comment>Gateway-backed extraction has no service-level regression coverage, so a future change can silently drop the context and break Browserbase extraction. Add focused tests for forwarding `GatewayContext` and for the missing-context failure path.
(Based on your team's feedback about adding unit tests for changed behavior.) </comment>
<file context>
@@ -106,7 +109,7 @@ export async function extract({
domElements: combinedTree,
schema: transformedSchema as z.ZodObject,
- generate: (input) => llmService.generate(model, input, clientLLMGenerate),
+ generate: (input) => llmService.generate(model, input, clientLLMGenerate, gateway),
userProvidedInstructions: systemPrompt,
});
</file context>
| instruction: effectiveInstruction, | ||
| domElements: combinedTree, | ||
| generate: (input) => llmService.generate(model, input, clientLLMGenerate), | ||
| generate: (input) => llmService.generate(model, input, clientLLMGenerate, gateway), |
There was a problem hiding this comment.
P3: Gateway-backed observe() routing has no service-level regression test. Add a focused observation test with a Browserbase model and gateway context so this new forwarding path remains covered.
(Based on your team's feedback about adding unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/services/observeService.ts, line 89:
<comment>Gateway-backed `observe()` routing has no service-level regression test. Add a focused observation test with a Browserbase model and gateway context so this new forwarding path remains covered.
(Based on your team's feedback about adding unit tests for new behavior.) </comment>
<file context>
@@ -83,7 +86,7 @@ export async function observe({
instruction: effectiveInstruction,
domElements: combinedTree,
- generate: (input) => llmService.generate(model, input, clientLLMGenerate),
+ generate: (input) => llmService.generate(model, input, clientLLMGenerate, gateway),
userProvidedInstructions: systemPrompt,
supportedActions: Object.values(SupportedUnderstudyAction),
</file context>
|
|
||
| /** Creates a direct AI SDK model from a validated Stagehand model configuration. */ | ||
| export function createAiSdkLanguageModel(config: KnownModelConfig): LanguageModel { | ||
| export function createAiSdkLanguageModel(config: DirectModelConnection): LanguageModel { |
There was a problem hiding this comment.
P3: Unlisted direct models now reach provider factories without catalog validation, but no AI SDK-client test covers that new path. Add focused unlisted-ID coverage (including a slash-containing ID) so forwarding behavior cannot regress.
(Based on your team's feedback about unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/llm/aiSdkClient.ts, line 125:
<comment>Unlisted direct models now reach provider factories without catalog validation, but no AI SDK-client test covers that new path. Add focused unlisted-ID coverage (including a slash-containing ID) so forwarding behavior cannot regress.
(Based on your team's feedback about unit tests for new behavior.) </comment>
<file context>
@@ -128,7 +122,7 @@ const AiSdkGenerationSchema = z
/** Creates a direct AI SDK model from a validated Stagehand model configuration. */
-export function createAiSdkLanguageModel(config: KnownModelConfig): LanguageModel {
+export function createAiSdkLanguageModel(config: DirectModelConnection): LanguageModel {
const separator = config.modelName.indexOf("/");
const provider = ModelProviderSchema.parse(config.modelName.slice(0, separator));
</file context>
| } | ||
|
|
||
| /** Creates an AI SDK Responses model backed by Browserbase Model Gateway. */ | ||
| export function createGatewayLanguageModel( |
There was a problem hiding this comment.
P3: Gateway transport wiring has no focused test, so an invalid endpoint or missing cloud headers reaches production undetected. Add tests that mock the provider request and assert the resolved URL plus both Browserbase headers.
(Based on your team's feedback about unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/llm/gatewayClient.ts, line 25:
<comment>Gateway transport wiring has no focused test, so an invalid endpoint or missing cloud headers reaches production undetected. Add tests that mock the provider request and assert the resolved URL plus both Browserbase headers.
(Based on your team's feedback about unit tests for new behavior.) </comment>
<file context>
@@ -0,0 +1,37 @@
+}
+
+/** Creates an AI SDK Responses model backed by Browserbase Model Gateway. */
+export function createGatewayLanguageModel(
+ target: BrowserbaseModelTarget,
+ gateway: GatewayContext,
</file context>
7f253fa to
63fbb89
Compare
beb241b to
5fba8ca
Compare
Why
#2438 makes model routing explicit in the protocol. This PR implements that contract in the runtime.
Model IDs from integrated providers are intentionally open:
openai/gpt-future-previewis valid direct-provider input without a Stagehand catalog update. The runtime still validates the stable configuration choices it owns: direct credentials, Browserbase gateway selection, and custom endpoint configuration.What changed
Adds one server-side resolver with three execution targets:
Dispatches on the resolved target instead of treating a missing provider API key as Browserbase routing.
Adds the Browserbase gateway client, which:
Implements the OpenAI-compatible endpoint path.
Updates direct-provider adapters to forward an open provider model ID.
Removes all runtime handling of
type: "unlisted".Test plan