fix: restore clientHeaders passthrough in chatCore (regression from #1227) - #1335
Conversation
- restore clientHeaders passthrough lost during v3.6.5-v3.6.7 release merges - re-enables x-initiator forwarding to GitHub Copilot (regression from PR diegosouzapw#1227)
cedrugs
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the handleChatCore function in open-sse/handlers/chatCore.ts to pass client headers to the execution and retry logic. Review feedback highlights a potential type mismatch where clientRawRequest.headers might be a Headers object instead of the expected plain object, suggesting the use of Object.fromEntries to ensure type safety and correct behavior.
| log, | ||
| extendedContext, | ||
| upstreamExtraHeaders: buildUpstreamHeadersForExecute(modelToCall), | ||
| clientHeaders: clientRawRequest?.headers ?? null, |
There was a problem hiding this comment.
There's a potential type mismatch here. The ExecuteInput.clientHeaders property is typed as Record<string, string> | null, but clientRawRequest.headers can be a Headers object, which is not directly assignable. This could lead to issues in the executor if it expects a plain object.
To ensure type safety and correct behavior, it's better to explicitly convert a Headers object to a plain object before passing it.
| clientHeaders: clientRawRequest?.headers ?? null, | |
| clientHeaders: clientRawRequest?.headers instanceof Headers ? Object.fromEntries(clientRawRequest.headers) : (clientRawRequest?.headers ?? null), |
| log, | ||
| extendedContext, | ||
| upstreamExtraHeaders: buildUpstreamHeadersForExecute(retryModelId), | ||
| clientHeaders: clientRawRequest?.headers ?? null, |
There was a problem hiding this comment.
There's a potential type mismatch here. The ExecuteInput.clientHeaders property is typed as Record<string, string> | null, but clientRawRequest.headers can be a Headers object, which is not directly assignable. This could lead to issues in the executor if it expects a plain object.
To ensure type safety and correct behavior, it's better to explicitly convert a Headers object to a plain object before passing it.
| clientHeaders: clientRawRequest?.headers ?? null, | |
| clientHeaders: clientRawRequest?.headers instanceof Headers ? Object.fromEntries(clientRawRequest.headers) : (clientRawRequest?.headers ?? null), |
Integrated into release/v3.6.7
Integrated into release/v3.6.7
2 participants
Summary
clientHeadersfield inExecuteInput(base.ts), stash/read/cleanup inGithubExecutor(github.ts), and passthrough inchatCore.ts. All three parts were merged.chatCore.tslines that passclientHeaders: clientRawRequest?.headers ?? nulltoexecutor.execute()were lost. The base.ts and github.ts changes survived.clientHeadersbut never receives them - sox-initiatoralways falls back to"user", burning premium requests on every turn instead of only user-initiated ones.executor.execute()callsites.Related Issues
Validation
npm run lint- 0 errors (78 pre-existing warnings, none from this change)npm run test:unit- 3054/3061 pass, 3 pre-existing failures (chatcore-sanitization, proxy-fetch, xiaomi-mimo - all unrelated), 4 skippedTests Added Or Updated
Reviewer Notes
open-sse/handlers/chatCore.tsis modified - addingclientHeaders: clientRawRequest?.headers ?? nullat lines ~1566 and ~1767