fix(providers): match the OpenCode Go destination canonically - #5165
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change reuses the shared URL canonicalizer for OpenCode Go destination detection. It adds canonical matching tests for trailing slashes, host casing, default ports, query strings, userinfo, and look-alike hosts. ChangesOpenCode Go destination matching
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
c553ae5 to
8631aaa
Compare
리뷰 · 우선순위 34 / 80OpenCode Go로 요청을 보내다 서버가 "잠깐 너무 많다"(429)고 하면, 키를 갈아타지 않고 조금 기다렸다가 같은 주소로 다시 보낸다. #5067이 그 기다림을 넣었다. 주소를 글자 그대로 비교해서, 이 PR은 양쪽 주소를 src/providers/key-failover.ts - 비교 함수를 src/providers/quota/vendor-probes-key.ts에서 가져온다. 그 파일은 사용량을 서버에 물어보는 모듈이다. 요청을 보내는 쪽이 이미 key-failover를 불러오므로, 429 판별이 사용량 조사 모듈까지 같이 읽게 된다. 같은 파일 48행 normalizedBaseUrl - 주석은 쿼리나 프래그먼트가 있으면 절대 안 맞는다고 적는다. Node에서 주소 끝이 메인테이너의 판단이 필요한 지점 함수를 사용량 파일에 둘지, 짧은 공용 파일로 뺄지. 동작은 맞고, 누가 누구를 불러오는지만 어색하다. URL 해석기가 너의 추천 막는 버그는 없다. 대문자와 이 댓글은 grok-bot이 작성했습니다 |
Follow-up to the review on lidge-jun#5067: isOpenCodeGoDestination string-compared URLs, so an uppercase hostname or an explicit default port silently missed the patient 429 fallback. Both sides now go through the shared normalizedBaseUrl from quota/vendor-probes-key (lowercased origin, default ports dropped, userinfo/query/fragment refused), keeping destination identity answered in one place. Alias support is unchanged.
8631aaa to
24f9972
Compare
…inline The OpenCode Go destination test added by #5165 carried an inline https URL with userinfo. privacy:scan reads source text, so its email detector saw the password and host as an address and failed the gates job on dev, blocking every branch that merged current dev. isAllowedEmail already carries one URL-userinfo exemption for a chatgpt.com fixture, so adding a second entry was available and is the wrong fix. That allowlist is what keeps the detector able to fail on a real address, and each entry spends a little of that. Assembling the URL through the username and password setters removes the literal entirely and leaves nothing to exempt. The input under test does not change. new URL with the same path and those two setters serializes to exactly the string it replaces, so the case still proves that a base URL carrying userinfo refuses the patient same-target fallback. No local suite, focused test, typecheck or build was run. Exact-head hosted CI is the execution evidence.
…inline (#5195) The OpenCode Go destination test added by #5165 carried an inline https URL with userinfo. privacy:scan reads source text, so its email detector saw the password and host as an address and failed the gates job on dev, blocking every branch that merged current dev. isAllowedEmail already carries one URL-userinfo exemption for a chatgpt.com fixture, so adding a second entry was available and is the wrong fix. That allowlist is what keeps the detector able to fail on a real address, and each entry spends a little of that. Assembling the URL through the username and password setters removes the literal entirely and leaves nothing to exempt. The input under test does not change. new URL with the same path and those two setters serializes to exactly the string it replaces, so the case still proves that a base URL carrying userinfo refuses the patient same-target fallback. No local suite, focused test, typecheck or build was run. Exact-head hosted CI is the execution evidence. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
2 participants
Summary
Follow-up to the review on #5067 (abhisheksharma2411):
isOpenCodeGoDestinationstring-compared URLs, so an operator row likehttps://OpenCode.ai/zen/go/v1orhttps://opencode.ai:443/zen/go/v1silently missed the patient 429 fallback with no indication why. Both the row URL and the registry candidates (includingdestinationAliases) now go through the sharednormalizedBaseUrlfromquota/vendor-probes-key— lowercased origin, default ports dropped, userinfo/query/fragment refused — so destination identity is answered in one place. The fail-closed direction is unchanged: anything unparsable still gets no fallback.Verification
bun test tests/providers/rate-limit-retry.test.ts— 18 pass, 0 fail, including a new canonical-match test (uppercase host, explicit :443, trailing slash match; userinfo, query, look-alike host refuse).bun test tests/providers/provider-quota.test.ts tests/providers/provider-account-quota.test.ts— 238 pass, 0 fail.bun run typecheck— clean.Checklist
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