[STG-1573] Add providerOptions for extensible model auth#1822
Merged
[STG-1573] Add providerOptions for extensible model auth#1822
Conversation
🦋 Changeset detectedLatest commit: 10835ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
✱ Stainless preview buildsThis PR will update the
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 21 files
Confidence score: 3/5
- There is some regression risk from configuration merge behavior changes, so this is not a no-risk merge despite looking contained to option-preparation paths (all flagged issues are medium severity at 6/10).
- Most severe impact is in
packages/server-v4/src/lib/header.ts:getRequestModelConfigcan ignorebody.modelNamewhenbody.options.modelis an object, which may select the wrong model/provider for clients using mixed old/new payload fields. packages/core/lib/v3/api.tsandpackages/server-v3/src/lib/InMemorySessionStore.tsboth have spread-order override hazards (apiKeyfallback being clobbered, andmodelClientOptions.modelNameoverriding explicitmodelName), which could cause auth/config mismatches at runtime.- Pay close attention to
packages/server-v4/src/lib/header.ts,packages/core/lib/v3/api.ts,packages/server-v3/src/lib/InMemorySessionStore.ts- merge-order and fallback precedence can silently change effective model configuration.
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-v4/src/lib/header.ts">
<violation number="1" location="packages/server-v4/src/lib/header.ts:49">
P2: `getRequestModelConfig` short-circuits on `body.options.model` existing as an object, dropping the old fallback to `body.modelName`. If a client sends `{ options: { model: { provider: "bedrock" } }, modelName: "some-model" }`, the old `getModelName` would find `"some-model"` via the `body.modelName` fallback, but the new code returns `undefined` because `getRequestModelConfig` returns the model object (which lacks `modelName`) without ever checking `body.modelName`.</violation>
</file>
<file name="packages/core/lib/v3/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:660">
P2: Spread order in `prepareModelConfig` can clobber the resolved `apiKey` fallback. `resolveModelClientOptions` already merges `model` into its result and then adds a fallback `apiKey`—but `...model` is re-spread on top, overwriting that key if `model` carries an explicit `apiKey: undefined`. Swap the spread order so `resolveModelClientOptions` output takes precedence:
```ts
return {
...model,
...this.resolveModelClientOptions(provider, model),
};
This ensures the fallback apiKey (and any Bedrock credentials) aren't silently clobbered.
Architecture diagram
sequenceDiagram
participant SDK as Stagehand SDK (V3)
participant API as API Client
participant Srv as Server (API Route)
participant Store as Session Store
participant AWS as AWS Bedrock
Note over SDK, AWS: Session Initialization (Start)
SDK->>API: init(modelClientOptions)
API->>API: NEW: toSessionStartModelClientOptions()<br/>(Serializes Bedrock region/keys)
API->>Srv: POST /sessions/start (JSON body)
Note right of API: Body includes NEW: modelClientOptions<br/>(accessKeyId, secretAccessKey, etc.)
Srv->>Store: NEW: createSession(modelClientOptions)
Store->>Store: Persist provider settings for session
Store-->>Srv: sessionId
Srv-->>API: 200 OK
API-->>SDK: Session Ready
Note over SDK, AWS: Action Execution (e.g., act / extract)
SDK->>API: act({ input, options })
API->>API: NEW: prepareModelConfig()<br/>(Resolves default vs override creds)
API->>Srv: POST /act (JSON body)
Note right of API: Body contains options.model object<br/>with AWS credentials
Srv->>Srv: NEW: getRequestModelConfig()<br/>(Parses body for Bedrock keys)
alt NEW: Credentials in body
Srv->>Srv: Use body credentials
else CHANGED: Credentials missing in body
Srv->>Srv: Fallback to x-model-api-key header
end
Srv->>Store: getV3Options(requestContext)
Store->>Store: NEW: Merge Session settings + Request overrides
Store-->>Srv: V3Options (with resolved BedrockProviderSettings)
Srv->>AWS: Execute Model Call
Note right of Srv: Request authorized using passed-through<br/>AWS Access Key / Session Token
AWS-->>Srv: Model Response
Srv-->>API: Streamed Action Results
API-->>SDK: Result
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
Greptile SummaryThis PR adds AWS Bedrock as a first-class model provider by threading provider-specific credential objects ( Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant SDK as Stagehand SDK (v3.ts)
participant Client as StagehandAPIClient
participant Server as Server (start.ts)
participant Store as InMemorySessionStore
participant AI as AI Provider (Bedrock / OpenAI)
Note over SDK,AI: Session Initialization
SDK->>Client: init({ modelName, modelApiKey?, modelClientOptions? })
Client->>Client: merge modelApiKey into this.modelClientOptions
Client->>Server: POST /sessions/start<br/>body: { modelName, modelClientOptions }<br/>header: x-model-api-key (if apiKey present)
Server->>Store: createSession({ modelName, modelClientOptions })
Store-->>Server: sessionId
Server-->>Client: { sessionId, available }
Note over SDK,AI: Per-Request Tool Call (e.g. act)
SDK->>Client: act({ input, options? })
Client->>Client: getDefaultModelConfig()<br/>→ { modelName, ...modelClientOptions }
Client->>Server: POST /act<br/>body: { options: { model: { modelName, credentials... } } }<br/>header: x-model-api-key (if apiKey present)
Server->>Server: getRequestModelConfig(request)<br/>→ ctx.modelConfig
Server->>Store: getOrCreateStagehand(sessionId, ctx)
Store->>Store: buildV3Options:<br/>merge params.modelClientOptions + ctx.modelConfig<br/>apply ctx.modelApiKey if no explicit creds
Store->>AI: Stagehand call with merged credentials
AI-->>Store: result
Store-->>Server: result
Server-->>Client: streaming response
Last reviewed commit: a15158e |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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/core/lib/v3/api.ts">
<violation number="1">
P2: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
These API client changes (removing default model injection on follow-up requests and simplifying `prepareModelConfig` to drop Bedrock credential merging) alter the wire format sent by act/extract/observe/agent calls. Per project rules, breaking changes to `packages/core/lib/v3/api.ts` must be covered by at least one integration test under `packages/server-v3/test/` or `packages/server-v4/test/`. Only a unit test (`packages/core/tests/unit/api-client-model-config.test.ts`) was added — consider adding an integration test that verifies end-to-end model config forwarding (e.g., session-start with `modelClientOptions` followed by an act/extract call that confirms the server receives the expected model configuration).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
shrey150
changed the title
Contributor
There was a problem hiding this comment.
3 issues found across 8 files (changes from recent commits).
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/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:80">
P2: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
This schema change (removing `region`/`accessKeyId`/`secretAccessKey`/`sessionToken` in favor of `providerOptions`) is a breaking change to the public API surface. No integration test under `packages/server-v4/test/integration/` validates that the new `providerOptions` field (or the session-level `modelClientOptions` carrying it) round-trips correctly through the server routes. Add at least one integration test (e.g., in `start.test.ts` or a new file) that sends a session-start request with `modelClientOptions.providerOptions` and verifies it is preserved end-to-end.</violation>
</file>
<file name="packages/server-v3/src/lib/InMemorySessionStore.ts">
<violation number="1" location="packages/server-v3/src/lib/InMemorySessionStore.ts:219">
P2: Removing the `hasExplicitAwsCredentials` guard means `ctx.modelApiKey` will now be injected into `modelClientOptions` even when AWS credentials (`accessKeyId`, `secretAccessKey`, `sessionToken`) are present. If a hosted environment sets a default model API key via the `x-model-api-key` header and a request supplies Bedrock AWS credentials without an explicit `apiKey`, the injected key could confuse the model client's auth path. Consider either keeping the guard or adding a comment explaining why it's safe to remove (e.g., the downstream client now correctly prioritizes AWS credentials over `apiKey`).</violation>
</file>
<file name="packages/core/lib/v3/llm/LLMProvider.ts">
<violation number="1" location="packages/core/lib/v3/llm/LLMProvider.ts:121">
P2: Shallow spread of `providerOptions` over `restOptions` silently drops the top-level `headers` when both objects carry a `headers` key (e.g., `GoogleVertexProviderSettings` also has `headers`). Consider merging the `headers` fields explicitly to avoid silent data loss.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
shrey150
changed the title
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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/core/lib/v3/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:223">
P2: Redundant call to `toSessionStartModelClientOptions` — `serializedMco` computed three lines above is the same result and should be reused in the request body.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
3 issues found across 13 files (changes from recent commits).
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-v3/openapi.v3.yaml">
<violation number="1" location="packages/server-v3/openapi.v3.yaml:94">
P2: Copy-paste error: `baseURL` example in all four Bedrock-specific schemas shows `https://api.openai.com/v1` instead of a Bedrock endpoint. This will confuse consumers of the API docs / generated SDK examples.</violation>
<violation number="2" location="packages/server-v3/openapi.v3.yaml:202">
P2: `GenericModelConfigObject.provider` enum includes `bedrock`, which overlaps with the dedicated Bedrock schemas in the `anyOf`. A Bedrock request supplying only `modelName` validates against the generic branch, silently skipping the Bedrock-required `providerOptions`. Consider removing `bedrock` from the generic enum (it is covered by the two Bedrock-specific schemas) or adding a `discriminator` on the `provider` field.</violation>
</file>
<file name="packages/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:339">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
Breaking API schema changes (`ModelConfigObjectSchema` → union, `ModelClientOptionsSchema` → union, `providerOptions` narrowed from open record to strict typed union) have no corresponding integration tests in `packages/server-v3/test/` or `packages/server-v4/test/`. Add integration tests to `start.test.ts` (or a new test file) in both server packages covering at least: (1) session start with Bedrock API-key auth, (2) session start with Bedrock AWS credentials, and (3) session start with generic/non-Bedrock provider options.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
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-v3/src/lib/InMemorySessionStore.ts">
<violation number="1" location="packages/server-v3/src/lib/InMemorySessionStore.ts:223">
P2: This check hardcodes AWS credential field names in the vendor-agnostic session store, contradicting the PR's extensibility design. Consider a generic signal instead—e.g. a `skipApiKey` boolean in `providerOptions`—so future non-API-key providers don't require server-side changes.
Additionally, this non-obvious interaction between `providerOptions` and `apiKey` injection deserves an inline comment explaining why AWS credentials suppress the API key fallback.
(Based on your team's feedback about documenting complex logic and edge-case interactions inline.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
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-v3/openapi.v3.yaml">
<violation number="1">
P1: The provider enum is missing `vertex` and several other supported AI SDK providers, which will cause OpenAPI validation to incorrectly reject valid models.</violation>
<violation number="2" location="packages/server-v3/openapi.v3.yaml:83">
P2: The `ProviderOptions` schema will reject arbitrary fields for new providers, contradicting the PR's claim that future providers require "zero schema changes".</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
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/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:262">
P2: Use a nested apiKey path when validating `SessionStartRequestSchema`; the current hardcoded `['apiKey']` points to the wrong field.</violation>
<violation number="2" location="packages/core/lib/v3/types/public/api.ts:289">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
Rule 2 requires integration coverage for changed public API request-shape behavior, but these new Bedrock validation branches were added without corresponding server integration tests (especially the new rejection paths). Add `packages/server-*/test/integration/*` cases that exercise the new Bedrock validation outcomes.</violation>
</file>
<file name="packages/core/tests/unit/model-config-schema.test.ts">
<violation number="1" location="packages/core/tests/unit/model-config-schema.test.ts:114">
P3: This test is scoped to `SessionStartRequestSchema` but validates `ModelClientOptionsSchema`, so it doesn't actually verify session-start acceptance for generic provider options.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
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-v3/test/integration/v3/agentExecute.test.ts">
<violation number="1" location="packages/server-v3/test/integration/v3/agentExecute.test.ts:335">
P2: Ensure browser harness cleanup still runs when `endSession` fails; the current `await` order can skip `browserHarness.close()` and leak test resources.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
monadoid
requested changes
Mar 26, 2026
Contributor
|
For strictness, could add a superRefine that requires: This would better follow "parse, don't validate", but maybe falls under "taste". Otherwise, LGTM. |
shrey150
force-pushed
the
shrey/bedrock-auth-passthrough
branch
from
2eca76a to
e287c46
Compare
Contributor
|
Addressed in 5f06f70c — extended the existing |
Contributor
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
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/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:61">
P2: Mutual-exclusion validation for `apiKey` and Bedrock credentials is only enforced for `ModelConfig`, not for `SessionStartRequest.modelClientOptions`, so conflicting auth can still be accepted at session start.</violation>
<violation number="2" location="packages/core/lib/v3/types/public/api.ts:70">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
Rule 3 violation: this new Bedrock auth mutual-exclusion validation changes public API request behavior, but there is no integration test covering the new rejection case. Add a server-v3 integration test that sends `model` with `apiKey` plus Bedrock `providerConfig.options.accessKeyId/secretAccessKey` and asserts a 400 with the validation error.
(Based on your team's feedback about checking providerConfig-related coverage in server-v3 integration tests first.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
monadoid
approved these changes
Mar 26, 2026
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:176">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
Add a server integration test for these new Bedrock validation rules. This change now rejects previously accepted request shapes, but the existing integration suite only covers typed providerConfig acceptance and provider mismatch.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
shrey150
force-pushed
the
shrey/bedrock-auth-passthrough
branch
from
09e48df to
a62a332
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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-v3/tests/integration/v3/start.test.ts">
<violation number="1" location="packages/server-v3/tests/integration/v3/start.test.ts:658">
P2: Don't require `/v1/sessions/start` to reject `modelClientOptions.apiKey` mixed with provider-specific auth fields. On session start, `modelClientOptions` is a transport bag; codifying this 400 response would force the wrong validation layer.
(Based on your team's feedback about limiting apiKey/providerConfig mutual-exclusion checks to ModelConfigSchema, not SessionStartRequestSchema.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
The server already stores modelClientOptions from session start and merges them into subsequent requests. This removes the redundant client-side persistence (getDefaultModelConfig, resolveModelClientOptions, modelName/modelClientOptions fields) so that language SDKs don't each need to reimplement this logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of adding AWS-specific fields (region, accessKeyId, secretAccessKey, sessionToken) as top-level fields in the shared ModelConfigObjectSchema, move them into a providerOptions bag that is passed through to the AI SDK provider constructor. This keeps the shared schema generic (apiKey, baseURL, headers, providerOptions) and avoids vendor-specific logic in the server. TypeScript types provide autocomplete for known providers (Bedrock, Vertex) via a union type on ClientOptions.providerOptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wrap endSession in nested try/finally so browserHarness.close() always runs even if endSession throws, preventing test resource leaks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use delete instead of underscore-prefixed destructuring to exclude properties from rest spreads, avoiding no-unused-vars lint errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevents Bedrock requests with accessKeyId/secretAccessKey from getting a spurious apiKey added via the x-model-api-key header fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shrey150
force-pushed
the
shrey/bedrock-auth-passthrough
branch
from
576f342 to
c77d1fa
Compare
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Contributor
|
@cubic-dev-ai review |
Contributor
@shrey150 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
4 issues found across 33 files
Confidence score: 2/5
- There is a high-confidence functional regression risk in
packages/core/lib/v3/api.ts:providerConfigis not mapped toproviderOptions, and the strictModelClientOptionsSchemacan reject payloads, which is likely to break provider initialization/auth flows. - Error-handling concerns in
packages/core/examples/bedrock_aws_credentials.tsandpackages/core/examples/bedrock_api_key.tsare user-facing/compliance-relevant: genericnew Error()messages include configuration-specific details instead of typed, sanitized errors. - Tests in
packages/core/tests/unit/llm-provider.test.tscurrently assert onlytoBeDefined(), so they are unlikely to catch the config-mapping regression; this keeps risk elevated despite test additions. - Pay close attention to
packages/core/lib/v3/api.ts,packages/core/examples/bedrock_aws_credentials.ts,packages/core/examples/bedrock_api_key.ts,packages/core/tests/unit/llm-provider.test.ts- mapping correctness, sanitized typed errors, and stronger regression assertions are needed.
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/core/tests/unit/llm-provider.test.ts">
<violation number="1" location="packages/core/tests/unit/llm-provider.test.ts:86">
P2: These new tests only check `toBeDefined()` and do not validate providerConfig mapping/normalization, so regressions in the new auth-option behavior can pass undetected.</violation>
</file>
<file name="packages/core/lib/v3/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:713">
P1: `providerConfig` (SDK field) is never converted to `providerOptions` (wire field). The deep copy keeps the key as `providerConfig`, but `ModelClientOptionsSchema` expects `providerOptions` and is `.strict()` — so the server will reject the payload. Additionally, since `requestOptions.providerOptions` is always `undefined`, the `skipApiKeyFallback` guard is dead and Bedrock AWS credential auth won't suppress API-key backfill.
The fix should flatten `providerConfig.options` into `providerOptions` and delete the `providerConfig` key.</violation>
</file>
<file name="packages/core/examples/bedrock_aws_credentials.ts">
<violation number="1" location="packages/core/examples/bedrock_aws_credentials.ts:28">
P1: Custom agent: **Exception and error message sanitization**
Rule 1 violation: this uses generic `new Error()` with env-var-specific text. Replace with a typed error and a sanitized message that does not expose configuration variable details.</violation>
</file>
<file name="packages/core/examples/bedrock_api_key.ts">
<violation number="1" location="packages/core/examples/bedrock_api_key.ts:24">
P2: Custom agent: **Exception and error message sanitization**
Replace generic `new Error()` throws with a typed, sanitized error class as required by the error sanitization rule.</violation>
</file>
Architecture diagram
sequenceDiagram
participant SDK as Stagehand SDK
participant API as Server (API)
participant Store as Session Store
participant AISDK as AI SDK Provider
Note over SDK, AISDK: Session Initialization Flow
SDK->>SDK: NEW: normalize model config
SDK->>API: POST /v1/sessions/start
Note right of SDK: Includes modelClientOptions<br/>(providerOptions: region, creds, etc.)
API->>API: CHANGED: getRequestModelConfig()
API->>Store: NEW: Store modelClientOptions in session
Store-->>API: Success
API-->>SDK: 201 Created (sessionId)
Note over SDK, AISDK: Runtime Request Flow (Act/Observe/Extract)
SDK->>SDK: NEW: ensureModelConfig()<br/>(Injects session defaults into request)
SDK->>API: POST /v1/sessions/:id/act
Note right of SDK: Includes model config override or session defaults
API->>Store: Get session config
Store-->>API: Session Data (stored options)
API->>API: NEW: buildV3Options()<br/>(Merges stored + per-request config)
alt NEW: providerOptions present (Bedrock/Vertex)
API->>API: skipApiKeyFallback = true
API->>AISDK: NEW: getProviderConstructorOptions()
Note right of API: Maps providerOptions to AI SDK constructor
else Standard API Key auth
API->>API: Use x-model-api-key or model.apiKey
end
API->>AISDK: createAmazonBedrock / createVertexAI(options)
AISDK-->>API: Provider Instance
API->>AISDK: generateText / streamText
AISDK-->>API: Model Response
API-->>SDK: Action Result
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
monadoid
approved these changes
Apr 8, 2026
This was referenced Apr 8, 2026
tkattkat
added a commit
that referenced
this pull request
Apr 9, 2026
…)" This reverts commit 4fe9a72.
tkattkat
added a commit
that referenced
this pull request
Apr 9, 2026
…)" (#1987) ## Summary - Reverts PR #1822 which added `providerOptions` for extensible model auth (Bedrock/Vertex) - Removes `providerOptions` bag from model config schema, Bedrock examples, provider config, and server-side `modelClientOptions` storage/passthrough ## Note - One test from PR #1980 (`accepts verified Browserbase session settings` in `model-config-schema.test.ts`) was removed as part of this revert since the test file was originally created by #1822. That test may need to be re-added in a separate file. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Reverts the `providerOptions`-based model auth from #1822 (STG-1573) and restores the previous auth flow: no provider options passthrough; model auth is via the `x-model-api-key` header. - **Refactors** - Removed `providerOptions` and server-side `modelClientOptions` passthrough from core and server. - Deleted Bedrock examples and provider config; pruned related schemas and tests. - Simplified OpenAPI: removed ProviderOptions/ModelClientOptions components and helpers extracting model config from request bodies. - **Migration** - Remove `providerOptions` from all model configs and requests. - Do not send `modelClientOptions` in session start; the server ignores it. - Send model API keys via the `x-model-api-key` header; remove any `skipApiKeyFallback` usage. - If you adopted `vertex/*` names from #1822, switch to `google/*`. <sup>Written for commit 1a3bbcb. Summary will update on new commits. <a href="https://cubic.dev/pr/browserbase/stagehand/pull/1987">Review in cubic</a></sup> <!-- End of auto-generated description by cubic. -->
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
3 participants
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.
Summary
providerOptionsbag to model config schema for provider-specific auth (Bedrock, Vertex, future providers) without polluting the shared schema with vendor-specific fieldsmodelApiKeyoptional so providers with non-API-key auth (e.g. AWS SigV4) can work without itmodelClientOptionsfrom session start and reuses them on subsequent act/observe/extract/execute calls — SDKs don't need to reimplement this logicmodelClientOptions(includingproviderOptions) in the session start request body; server merges stored + per-request configWhat this enables
region,accessKeyId,secretAccessKey,sessionTokenviaproviderOptionsproject,location,googleAuthOptionsviaproviderOptionsproviderOptions— zero schema or server changes neededArchitecture
Linked PRs
Testing