test(model): cover createAzureOpenAILanguageModel auth headers - #436
Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
Conversation
The Azure factory sets both an Authorization Bearer header, needed for managed identity, and an api-key header. Nothing asserted either, so a regression to the pre-microsoft#139 single-header shape would have gone unnoticed.
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
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.
createAzureOpenAILanguageModelsets two headers, and the comments inmodel.ts:214-219say why:Authorization: Bearer <key>for managed identity,api-keyfor a regular key. PR #139 added the first one after 401s.typescript/tests/model.test.mjshaddescribeblocks forcreateOpenAILanguageModel,createLanguageModeland the timeout/size-limit behaviour, and nothing for Azure, so a regression to the single-header shape would not have been caught.This adds a
createAzureOpenAILanguageModelblock asserting both headers, following the existingsetupFetch/capturedRequestspattern in the file.Verified it actually catches the regression by deleting the
Authorizationline from the built Azure factory only, leaving the OpenAI one intact, and re-running:Could not verify:
npm cifrom a cleannode_modulesruns the workspacepreparelifecycle, which does not complete on this machine, so I installed with--ignore-scriptsand built withnpx tsc -p srcbefore runningnode --test. Same compiled output, different entry point.