fix(adapters): preserve Responses URL authority - #1718
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Responses URL builder now uses ChangesResponses URL handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change preserves the intended URL authority while keeping existing routing behavior; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
3/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested='23d10de33ce8eb25bfd12c0e69e3f9447a2f0e26'
echo '== HEAD =='
git rev-parse HEAD
echo '== requested commit =='
git rev-parse "$requested"
echo '== commit metadata =='
git show -s --format='%H%nParents: %P%nSubject: %s' "$requested"
echo '== changed files =='
git diff-tree --no-commit-id --name-status -r "$requested"
echo '== exact diff =='
git diff "${requested}^" "$requested" --check
git diff --find-renames --find-copies "${requested}^" "$requested" -- .
echo '== outlines of changed source files =='
git diff-tree --no-commit-id --name-only -r "$requested" |
while IFS= read -r f; do
case "$f" in
*.ts|*.tsx|*.js|*.jsx)
echo "--- $f"
ast-grep outline "$f" --items all --view expanded || true
;;
esac
done
echo '== URL helper candidates and usages =='
rg -n -i -C 4 \
'(responsesPath|/v1/responses|new URL\s*\(|key-auth|keyAuth|baseURL|baseUrl)' \
-g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' .Length of output: 201513 🏁 Script executed: #!/bin/bash
set -euo pipefail
requested='23d10de33ce8eb25bfd12c0e69e3f9447a2f0e26'
printf '%s\n' '== commit identity =='
printf 'HEAD: '; git rev-parse HEAD
printf 'requested: '; git rev-parse "$requested"
git show -s --format='parent: %P%nsubject: %s' "$requested"
printf '%s\n' '== changed paths =='
git diff-tree --no-commit-id --name-status -r "$requested"
printf '%s\n' '== patch =='
git diff --no-ext-diff --unified=80 "${requested}^" "$requested" -- \
$(git diff-tree --no-commit-id --name-only -r "$requested")
printf '%s\n' '== changed-file outlines =='
while IFS= read -r file; do
case "$file" in
*.ts|*.tsx|*.js|*.jsx)
printf '\n--- %s ---\n' "$file"
ast-grep outline "$file" --items all --view expanded
;;
esac
done < <(git diff-tree --no-commit-id --name-only -r "$requested")
printf '%s\n' '== focused helper symbols and test labels =='
git grep -n -E \
'(responsesPath|responses.*URL|response.*URL|/v1/responses|authMode.*key|authMode.*forward)' \
"$requested" -- \
'*.ts' '*.tsx' '*.js' '*.jsx' |
head -n 500Length of output: 10751 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== URL helper with line numbers =='
nl -ba src/adapters/openai-responses-url.ts
echo '== adapter references to the helper and route selection =='
rg -n -C 12 '\bopenaiResponsesUrl\s*\(|\bresponsesPath\b|\bauthMode\b' \
src/adapters tests/openai-responses-passthrough.test.ts
echo '== baseUrl validation declarations and rules =='
rg -n -C 12 \
'(baseUrl.*(URL|url|credentials|query|fragment)|new URL\(.*baseUrl|URL\(.*baseUrl|must not include embedded credentials)' \
src tests \
-g '*.ts' -g '*.tsx'
echo '== focused changed test section with line numbers =='
nl -ba tests/openai-responses-passthrough.test.ts | sed -n '100,210p'
echo '== all direct helper usages at requested commit =='
git grep -n -E '\bopenaiResponsesUrl\s*\(' HEAD -- '*.ts' '*.tsx'Length of output: 1184 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== URL helper with line numbers =='
awk '{ printf "%6d %s\n", NR, $0 }' src/adapters/openai-responses-url.ts
echo '== adapter references to the helper and route selection =='
rg -n -C 12 '\bopenaiResponsesUrl\s*\(|\bresponsesPath\b|\bauthMode\b' \
src/adapters tests/openai-responses-passthrough.test.ts
echo '== baseUrl validation declarations and rules =='
rg -n -C 12 \
'(baseUrl.*(URL|url|credentials|query|fragment)|new URL\(.*baseUrl|URL\(.*baseUrl|must not include embedded credentials)' \
src tests \
-g '*.ts' -g '*.tsx' | head -n 800
echo '== focused changed test section with line numbers =='
awk 'NR >= 100 && NR <= 210 { printf "%6d %s\n", NR, $0 }' \
tests/openai-responses-passthrough.test.ts
echo '== all direct helper usages at requested commit =='
git grep -n -E '\bopenaiResponsesUrl\s*\(' HEAD -- '*.ts' '*.tsx'Length of output: 133208 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== provider baseUrl validation locations =='
rg -n -C 8 \
'(baseUrl must not|baseUrl.*(query|fragment|credentials)|embedded credentials|search.*=.*""|hash.*=.*""|protocol.*https?:)' \
src \
-g '*.ts' | head -n 500
echo '== configuration module candidates =='
fd -t f -e ts . src | rg '(^|/)(config|schema|validation|provider).*[.]ts$' | head -n 200
echo '== exact config validation function context =='
rg -n -C 30 \
'function (providerConfigError|validateProvider|providerManagementConfigError)|export function (providerConfigError|validateProvider|providerManagementConfigError)' \
src \
-g '*.ts' | head -n 700
echo '== exact diff hygiene =='
git diff --check 81ada7cd092d4be3b25f3013c996cd3262a2f99b 23d10de33ce8eb25bfd12c0e69e3f9447a2f0e26
git status --shortLength of output: 40269
No findings. The change in The helper is used only for non-forward requests with no
The added tests cover the affected hostname cases, a non-default port, and an IPv6 literal. ✅ Action performedReview finished.
|
23d10de to
eca01d7
Compare
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested='eca01d7780c7cc70646d2e8d31caa5b207904149'
echo '== Exact commit identity =='
echo -n 'HEAD: '; git rev-parse HEAD
echo -n 'Requested: '; git rev-parse "$requested"
git show -s --format='Parents: %P%nSubject: %s%n%n%B' "$requested"
echo '== Changed paths =='
git diff-tree --no-commit-id --name-status -r "$requested"
echo '== Diff check =='
git diff --check "${requested}^" "$requested"
echo '== Incremental patch =='
git diff --no-ext-diff --find-renames --find-copies --unified=80 \
"${requested}^" "$requested" -- \
$(git diff-tree --no-commit-id --name-only -r "$requested")
echo '== Changed TypeScript file outlines =='
git diff-tree --no-commit-id --name-only -r "$requested" |
while IFS= read -r file; do
case "$file" in
*.ts|*.tsx|*.js|*.jsx)
echo "--- $file"
ast-grep outline "$file" --items all --view expanded || true
;;
esac
done
echo '== Focused references to changed URL helper and routing controls =='
rg -n -C 10 \
'\bopenaiResponsesUrl\s*\(|\bresponsesPath\b|\bauthMode\b|/v1/responses|new URL\s*\(' \
src tests -g '*.ts' -g '*.tsx' | head -n 1200
echo '== Working tree status =='
git status --shortLength of output: 99170 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Helper and all call sites =='
rg -n -C 20 '\bopenaiResponsesUrl\s*\(' \
src tests -g '*.ts' -g '*.tsx'
echo '== Responses adapter URL selection =='
ast-grep outline src/adapters/openai-responses.ts --items all --view expanded | head -n 500
rg -n -C 35 \
'(responsesPath|openaiResponsesUrl|buildRequest|baseUrl)' \
src/adapters/openai-responses.ts
echo '== URL helper and test file relevant sections =='
nl -ba src/adapters/openai-responses-url.ts
nl -ba tests/openai-responses-passthrough.test.ts | sed -n '1,210p'
echo '== Validation call graph and URL validation tests =='
rg -n -C 12 \
'\bproviderBaseUrlConfigError\s*\(|baseUrl must not include query strings or fragments|baseUrl must not include embedded credentials' \
src tests -g '*.ts' -g '*.tsx'
echo '== Direct helper tests and URL-shape expectations =='
rg -n -C 10 \
'(openaiResponsesUrl|responses URL|Responses key-auth URL construction|baseUrl.*[?#])' \
tests src/adapters -g '*.ts' -g '*.tsx'
echo '== HTTPS policy for provider POST requests =='
rg -n -C 15 \
'provider POST URL must use HTTPS|providerOutboundRequest|providerPost' \
src/lib/provider-outbound.ts src/adapters/openai-responses.tsLength of output: 21505 🏁 Script executed: #!/bin/bash
set -euo pipefail
show_lines() {
local file="$1"
local start="$2"
local end="$3"
awk -v start="$start" -v end="$end" \
'NR >= start && NR <= end { printf "%6d %s\n", NR, $0 }' "$file"
}
echo '== URL helper =='
show_lines src/adapters/openai-responses-url.ts 1 80
echo '== Key-auth URL tests =='
show_lines tests/openai-responses-passthrough.test.ts 120 190
show_lines tests/openai-responses-passthrough.test.ts 2015 2040
echo '== Provider URL validation and configuration entry point =='
show_lines src/config.ts 745 785
rg -n -C 20 \
'(providerBaseUrlConfigError\(|superRefine|safeParse\(|parse\()' \
src/config.ts | head -n 800
echo '== Helper import and URL test coverage across the repository =='
rg -n -C 6 \
'from ["'\''].*openai-responses-url|openaiResponsesUrl\s*\(' \
src tests -g '*.ts' -g '*.tsx'
echo '== Query or fragment provider URL tests =='
rg -n -C 8 \
'(query strings or fragments|embedded credentials|baseUrl.*[?]x=|baseUrl.*#)' \
tests -g '*.ts' -g '*.tsx' | head -n 700
echo '== Exact diff against the requested parent =='
git diff --check eca01d7780c7cc70646d2e8d31caa5b207904149^ eca01d7780c7cc70646d2e8d31caa5b207904149Length of output: 58591
No findings.
I also applied the existing repository context for provider URL validation.
🧠 Learnings used
|
|
Thanks for the fix — parsing with new URL() and editing only pathname is the right call, and the table tests cover the authority-mangling cases well. Keeping this as draft for process reasons only: as an external-contributor draft it has not had full CI (fork limitation), so it needs the readiness checklist completed and a maintainer-triggered full CI run before merge. Flagging for maintainer CI. |
lidge-jun
added
provider
eca01d7 to
4a48836
Compare
2 participants
Summary
responsesorv1, non-default ports, and IPv6 literalsresponsesPathrouting unchangedThe previous string-wide suffix replacement could treat a hostname as a path segment (for example
https://responses) and construct a different authority. Because the adapter attaches API credentials to the resulting request, this is both a routing-correctness and credential-destination fix.Scope and compatibility
This changes only the default key-auth
/v1/responsesURL helper. Provider configuration already requires an HTTP(S) URL and rejects embedded credentials, queries, and fragments. URL serialization may canonicalize equivalent syntax such as host case or a default port; it does not change the destination.No UI is changed.
Verification
Published range:
b81314cd29b78fecb447df882dc4fc1a987434b9→4a48836d06dc314d311651ff86129eaec482dfa9.The validated and published exact head is
4a48836d06dc314d311651ff86129eaec482dfa9; its parent is currentdevatb81314cd29b78fecb447df882dc4fc1a987434b9. The latest-dev rebase preserved stable patch IDff189d1726cd47d768ce4566aa53b86fcf7cfd6b.git diff --checkpassed87bad3b6-a829-4953-be42-c9b01e63de00: no P0–P2 findings; snapshotcodex-security-snapshot/v1:sha256:78b507ea3064e622652c4f6de2a2bea99c2ffe31d78e373e375015315696a669; the scanned patch and exact head share stable patch IDff189d1726cd47d768ce4566aa53b86fcf7cfd6bThe full repository suite is not claimed green. A Bun 1.3.14 attempt reproducibly exited with an internal assertion in the unrelated
api-storage-policy-run.test.ts, including when isolated. A Bun 1.4 canary run completed with 11,479 passing, 17 skipped, 368 failing, and 16 errors, clustered in existing Windows effective-account/ACL/symlink/process-timeout and unrelated catalog areas. No plausible call path from this two-file patch to those failures was found. Maintained CI remains authoritative.Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
/v1bases, custom ports, and IPv6 addresses.