fix(gateway): harden Responses terminal lifecycle - #1632
Merged
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
BYK
force-pushed
the
fix/stream-eof-before-post-response
branch
from
August 15, 2026 19:05
5c943d6 to
b6df2dc
Compare
BYK
changed the title
Comment on lines
129
to
+133
| // on parse-failure to reject it. appendCustomHeader also sanitizes, but we | ||
| // normalize at the source so every consumer sees a clean value. | ||
| // oxlint-disable-next-line no-control-regex -- intentional control-character sanitization | ||
| const trimmed = raw.replace(/[\x00-\x1f\x7f]/g, "").trim(); | ||
| if (!trimmed) continue; | ||
| if (!trimmed) invalid(); |
There was a problem hiding this comment.
Bug: The captureUserEnvCredential function now silently discards a valid auth token if an associated upstream URL is invalid, instead of ignoring the invalid URL.
Severity: MEDIUM
Suggested Fix
Instead of immediately returning null when an invalid upstream URL is found, the function should either throw an error to explicitly inform the user about the invalid URL (similar to captureUserUpstream), or revert to the previous behavior of continuing the loop to search for other valid upstream URLs.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/gateway/src/cli/agents.ts#L129-L133
Potential issue: In the `captureUserEnvCredential` function, if a valid authentication
token is found but a configured upstream URL is determined to be invalid by
`cleanBaseUrl`, the entire function now returns `null`. Previously, it would skip the
invalid URL and could still return the credential with a null `upstreamUrl`. This change
causes a valid credential to be silently discarded if the user has also configured an
invalid upstream URL, leading to credential discovery failure without a clear error
message explaining why.
Owner
Author
There was a problem hiding this comment.
Addressed in #1640 by surfacing an explicit invalid-upstream error while preserving token-only provider-default credentials.
Contributor
Codecov Results 📊✅ Patch coverage is 90.83%. Project has 8177 uncovered lines. Files with missing lines (13)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 79.42% 80.33% +0.91%
==========================================
Files 217 218 +1
Lines 40073 41573 +1500
Branches 31579 32929 +1350
==========================================
+ Hits 31828 33396 +1568
- Misses 8245 8177 -68
- Partials 3546 3643 +97Generated by Codecov Action |
Contributor
|
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
1 participant
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
Follow-up to #1625 that makes Responses completion transactional: session-owned mutation, temporal persistence, recall anchors, and transfer accounting occur only after a validated terminal response reaches downstream EOF and post-response work succeeds. The branch is integrated with current
mainwhile preserving tenant isolation, safe compact routing, and setup/uninstall changes.Changes
Verification
Reviewed commit:
4e8b4759b4679ae688c55dd54ae696becf8ce6d6Approved PR parent retained in history:
b6df2dcd48b8e4326566047f733246336a2d6e42Integrated main parent:
8a5bc6119ce6b10144756fa81ebcb3739a3c0644pnpm test: 433 files, 9,330 tests passed; 15 files and 227 tests skippedpnpm run typecheckpnpm run format:checkpnpm run lint(exit 0, baseline warnings only)packages/gateway/test/sync.property.test.ts: 50/50 passed across ten serial runs