Skip to content

refactor(openai): sign Bedrock with the SDK provider fixes NV-9271 - #24

Merged
ChmaraX merged 3 commits into
mainfrom
nv-9271-thalamus-replace-the-custom-sigv4-fetch-with-openais-built
Nov 2, 2026
Merged

ChmaraX merged 3 commits into
mainfrom
nv-9271-thalamus-replace-the-custom-sigv4-fetch-with-openais-built

Conversation

@ChmaraX

@ChmaraX ChmaraX commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Why

Thalamus signed Amazon Bedrock requests itself, with service bedrock and a hand-built Mantle URL. The OpenAI SDK now owns that endpoint and signs Mantle requests with bedrock-mantle. Callers keep the same Bedrock config fields.

What changed

  • Bedrock API key and SigV4 clients are built with openai/providers/bedrock and openai/providers/bedrock/aws.
  • Both entrypoints load on first use. A static import breaks every Thalamus import on openai 6.17.0, which Novu still resolves.
  • The custom signer is gone. The edge observer still sends the configured API key. SigV4 on the observer stays the old placeholder bearer, because that worker does not sign.

Scope

  • Thalamus only. The public config shapes are unchanged.
  • README still names the old optional packages. The changelog and the in-repo skill name the new ones.
  • Novu is not updated. It will see a peer warning until it bumps openai.

Tradeoffs

baseURL: null is passed so an AWS_BEDROCK_BASE_URL on the host cannot replace the SDK's Mantle URL.

Blast radius

Anyone on Bedrock moves from /v1 to the SDK's /openai/v1 host. SigV4 callers install @aws-sdk/credential-provider-node, @smithy/hash-node, and @smithy/signature-v4 instead of @aws-crypto/sha256-js. The openai peer floor is >=6.45.0. Novu does not construct this provider.

Verification

  • pnpm test passed 255 tests. pnpm typecheck passed.
  • The earlier cloud run also captured a real client sending bearer traffic to https://bedrock-mantle.us-east-1.api.aws/openai/v1/responses and signing SigV4 with service bedrock-mantle.

RetriggerView in GreptileConfidence Score: 5/5

The latest extraction appears safe to merge; the existing observer test gap is non-blocking.

Fix All in CursorFindings

  1. P2 Test accepts broken observer URLs ▶
Fix with agent prompt
### Issue 1
__tests__/openai/openai.bedrock.test.ts:292-293
The new observer test passes even though it sends an invalid `streamUrl`. `MockOpenAI` has no `baseURL`, so the URL starts with `undefined/responses/`. The test only checks `Authorization`. Give the mock a valid `baseURL` and assert the full `streamUrl` so this test catches broken observer URLs.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Moves Bedrock requests to the OpenAI SDK providers while keeping the public config fields unchanged.

  • Bedrock requests use the OpenAI SDK's endpoint and signing providers.

Reviews (2) · Last reviewed commit: "refactor(openai): move Bedrock client se..." · Reviewed by Greptile

Thalamus built the Mantle URL by hand and signed with service bedrock.
The OpenAI SDK derives that host and signs with bedrock-mantle.

Both Bedrock entrypoints load on first use so importing Thalamus still
works on openai 6.17.0. The public config shapes stay the same.

fixes NV-9271
@linear-code

linear-code Bot commented Nov 2, 2026

Copy link
Copy Markdown

NV-9271

@socket-security

socket-security Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​smithy/​hash-node@​4.2.14 ⏵ 4.5.2100 +110051 -2090 +1100
Updatedopenai@​6.37.0 ⏵ 7.47.075 +7100100 +1100 +1100

View full report

Thalamus writes CHANGELOG.md in the release commit, from the commits
since the last tag. An Unreleased block is not part of that flow.
Comment on lines +292 to +293
const body = JSON.parse(String((observe?.[1] as RequestInit).body));
expect(body.headers.Authorization).toBe("Bearer bedrock-api-key-abc123");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Test accepts broken observer URLs

The new observer test passes even though it sends an invalid streamUrl. MockOpenAI has no baseURL, so the URL starts with undefined/responses/. The test only checks Authorization. Give the mock a valid baseURL and assert the full streamUrl so this test catches broken observer URLs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: __tests__/openai/openai.bedrock.test.ts
Line: 292-293

Comment:
**Test accepts broken observer URLs**

The new observer test passes even though it sends an invalid `streamUrl`. `MockOpenAI` has no `baseURL`, so the URL starts with `undefined/responses/`. The test only checks `Authorization`. Give the mock a valid `baseURL` and assert the full `streamUrl` so this test catches broken observer URLs.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Cursor

The provider file was already long. Client construction now lives in
openai-client.ts. Session mode reads auth.kind, and setup errors that
are not missing modules stay SDK errors.
@ChmaraX
ChmaraX merged commit 9fb823f into main Nov 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant