Fix AI provider header redaction#6650
Conversation
🦋 Changeset detectedLatest commit: b1aac18 The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughProvider clients now scope sensitive header names during request execution so mapped HTTP errors redact API keys and OpenAI organization/project headers. Tests add mock layers and verify redaction, request behavior, streaming, embeddings, and error mappings across Anthropic, OpenAI, OpenAI-compatible, and OpenRouter clients. ChangesProvider header redaction
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ai/openai-compat/test/OpenAiClient.test.ts (1)
257-263: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the error shape instead of falling back to
Headers.empty.If the reason isn't
InvalidRequestError, the three assertions fail with"undefined" !== "<redacted>", which hides the real cause. The Anthropic and OpenRouter variants narrow explicitly first — worth matching for a clearer failure message.♻️ Proposed refactor
- const headers = result.reason._tag === "InvalidRequestError" - ? result.reason.http?.request.headers ?? Headers.empty - : Headers.empty - + assert.strictEqual(result.reason._tag, "InvalidRequestError") + if (result.reason._tag !== "InvalidRequestError" || result.reason.http === undefined) { + return yield* Effect.die(new Error("Expected InvalidRequestError with HTTP context")) + } + const headers = result.reason.http.request.headers🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/openai-compat/test/OpenAiClient.test.ts` around lines 257 - 263, Update the test around the result.reason header assertions to explicitly narrow or assert that result.reason is an InvalidRequestError before accessing http.request.headers. Remove the Headers.empty fallback, then assert the redacted authorization, openai-organization, and openai-project headers from the narrowed error so unexpected error shapes fail clearly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai/openai/test/OpenAiClient.test.ts`:
- Around line 551-558: Type inference for the HttpClient.makeWith preprocess is
too broad in both test harnesses. In
packages/ai/openai/test/OpenAiClient.test.ts lines 551-558, replace the
preprocess lambda with the typed Effect.succeed as
HttpClient.HttpClient.Preprocess<HttpClientError.HttpClientError, never>; make
the same change at packages/ai/openrouter/test/OpenRouterClient.test.ts lines
63-70 and add the HttpClientError type import from effect/unstable/http.
- Around line 523-535: Update the MockResponse type’s body and events payload
declarations to accept the Generated.Response.Encoded values returned by
makeResponseBody, rather than restricting them to Schema.Json. Preserve the
existing Json and SSE mock structure and status/header fields.
---
Nitpick comments:
In `@packages/ai/openai-compat/test/OpenAiClient.test.ts`:
- Around line 257-263: Update the test around the result.reason header
assertions to explicitly narrow or assert that result.reason is an
InvalidRequestError before accessing http.request.headers. Remove the
Headers.empty fallback, then assert the redacted authorization,
openai-organization, and openai-project headers from the narrowed error so
unexpected error shapes fail clearly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31e9dbf2-fef4-4877-87a1-c44c7805d180
📒 Files selected for processing (10)
.changeset/fix-anthropic-header-redaction.md.changeset/fix-openai-header-redaction.mdpackages/ai/anthropic/src/AnthropicClient.tspackages/ai/anthropic/test/AnthropicClient.test.tspackages/ai/openai-compat/src/OpenAiClient.tspackages/ai/openai-compat/test/OpenAiClient.test.tspackages/ai/openai/src/OpenAiClient.tspackages/ai/openai/src/OpenAiClientGenerated.tspackages/ai/openai/test/OpenAiClient.test.tspackages/ai/openrouter/test/OpenRouterClient.test.ts
| const httpClient = HttpClient.makeWith( | ||
| Effect.fnUntraced(function*(requestEffect) { | ||
| const request = yield* requestEffect | ||
| capturedRequests.push(request) | ||
| return makeResponse(request, mock.response) | ||
| }), | ||
| (request) => Effect.succeed(request) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Untyped HttpClient.makeWith preprocess breaks the Types check in two harnesses. Passing (request) => Effect.succeed(request) leaves E/R uninferred, so makeWith yields HttpClient.With<unknown, unknown>, which is rejected by Context.make(HttpClient.HttpClient, ...). The Anthropic and OpenAI-compat harnesses avoid this by annotating the preprocess.
packages/ai/openai/test/OpenAiClient.test.ts#L551-L558: replace the lambda withEffect.succeed as HttpClient.HttpClient.Preprocess<HttpClientError.HttpClientError, never>(fixes the CI error at Line 564).packages/ai/openrouter/test/OpenRouterClient.test.ts#L63-L70: apply the same annotation and add thetype HttpClientErrorimport fromeffect/unstable/http(fixes the CI error at Line 76).
📍 Affects 2 files
packages/ai/openai/test/OpenAiClient.test.ts#L551-L558(this comment)packages/ai/openrouter/test/OpenRouterClient.test.ts#L63-L70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/openai/test/OpenAiClient.test.ts` around lines 551 - 558, Type
inference for the HttpClient.makeWith preprocess is too broad in both test
harnesses. In packages/ai/openai/test/OpenAiClient.test.ts lines 551-558,
replace the preprocess lambda with the typed Effect.succeed as
HttpClient.HttpClient.Preprocess<HttpClientError.HttpClientError, never>; make
the same change at packages/ai/openrouter/test/OpenRouterClient.test.ts lines
63-70 and add the HttpClientError type import from effect/unstable/http.
Source: Pipeline failures
Bundle Size Analysis
|
Summary
Testing
pnpm test packages/ai/openai-compat/test/OpenAiClient.test.tspnpm test packages/ai/openai/test/OpenAiClient.test.tspnpm test packages/ai/anthropic/test/AnthropicClient.test.ts packages/ai/anthropic/test/AnthropicLanguageModel.test.tspnpm test packages/ai/openrouter/test/OpenRouterClient.test.ts packages/ai/openrouter/test/OpenRouterLanguageModel.test.tsSummary by CodeRabbit