Skip to content

[WRONG BRANCH] update - #4703

Closed
Loinver wants to merge 2 commits into
lidge-jun:mainfrom
Loinver:main
Closed

Loinver wants to merge 2 commits into
lidge-jun:mainfrom
Loinver:main

Conversation

@Loinver

@Loinver Loinver commented Sep 15, 2026

Copy link
Copy Markdown

Summary

  • Explain the user-visible or maintainer-facing change.

Verification

  • List the commands or checks you ran.
  • If this PR changes the GUI, include a screenshot of the UI change in the description.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

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

  • Bug Fixes

    • Improved compatibility with Muse Spark web search across all destinations when using supported models.
    • Preserved web-search settings for preview tools and unrelated models.
    • Added support for nested tool configurations and relay-provider scenarios.
  • Documentation

    • Clarified when web-search fields are adjusted for supported Muse Spark models.

Loinver and others added 2 commits September 14, 2026 19:59
The strict-mode guard matched exact gateway URLs, so a Console Go reseller
host serving the same Muse Spark models kept search_content_types and 400ed
every Codex web_search turn. Key the sanitizer on the model id alone: the
strict ids name one gateway family wherever it is reached from, and the
canonical OpenAI path never carries them. Relay hosts now get the same
sanitization, while web_search_preview and unrelated models are untouched.
@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 15, 2026
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4b81cc8b-f213-451d-8562-eb29a4dbdf86

📥 Commits

Reviewing files that changed from the base of the PR and between e4a8539 and 01247a6.

📒 Files selected for processing (3)
  • src/adapters/openai-responses.ts
  • structure/transports/responses.md
  • tests/providers/muse-spark-web-search-compat.test.ts

📝 Walkthrough

Walkthrough

Changes

Muse Spark web-search sanitization

Layer / File(s) Summary
Model-based sanitization
src/adapters/openai-responses.ts
The sanitizer no longer checks destination URLs. It removes search_content_types and indexed_web_access when the trimmed, case-insensitive model id is in MUSE_SPARK_WEB_SEARCH_STRICT_MODELS. The call site no longer passes responseUrl (1996–2031, 2333).
Relay coverage and documentation
tests/providers/muse-spark-web-search-compat.test.ts, structure/transports/responses.md
Documentation now states that exact model ids control sanitization. Tests cover relay destinations, nested web_search tools, preserved web_search_preview fields, and unrelated models (44–49, 223, 267–308; 341).

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

⚠️ This pull request shows signs of AI-generated slop (description_diff_mismatch, ai_padded_prose). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot changed the title update [WRONG BRANCH] update Sep 15, 2026
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • wrong target branch (main); retarget to dev.

What to do

  • Retarget this PR to dev — all contributions go to dev.
  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ 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.

0/4 boxes ticked.

Its title has been prefixed with [WRONG BRANCH].
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@Loinver Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@github-actions
github-actions Bot marked this pull request as draft September 15, 2026 09:35
@Loinver Loinver closed this Sep 15, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 48 / 80

이 PR은 Muse Spark(오픈코드 Zen/Go·직접 Meta) 쪽으로 가는 web_search 요청에서 게이트웨이가 거부하는 필드(search_content_types, indexed_web_access)를 빼 주는 호환 로직을 고친다. 지금 코드는 모델 아이디가 맞고, 요청 URL이 정해진 세 주소(opencode.ai zen / zen/go, api.meta.ai)일 때만 필드를 지운다. 그래서 같은 Muse Spark 모델을 Console Go 리셀러·중계 호스트처럼 다른 origin으로 쓰면 URL 허용 목록에 안 걸려 필드가 그대로 남고, 게이트웨이가 매 턴 400을 낸다. 커밋 메시지가 말한 2026-09-14 재현과 맞다.

고치는 방향 자체는 현재 dev에도 그대로 필요하다. 오늘 dev HEAD는 51d577c3f(#4700, 패키지 2.57.0)이고, 허용 URL 집합과 함수는 src/adapters/openai-responses/web-search.ts에 있으며, 호출부는 src/adapters/openai-responses/passthrough.ts 326행 근처에서 stripMuseSparkUnsupportedWebSearchFields(outBody, parsed.modelId, url)로 URL을 넘긴다. “모델 아이디만으로 판정하고 URL 인자를 빼라”는 의도라면 그 두 리프를 고쳐야 한다.

그런데 이 PR 패치는 예전 모놀리스 src/adapters/openai-responses.ts 한 파일(수천 줄짜리)을 고친다. devmain 둘 다 그 경로는 이미 짧은 파사드이고, 본문은 openai-responses/ 아래 리프로 옮겨진 상태다(#4671 등 godfile round5). GitHub도 mergeable=CONFLICTING / DIRTY다. 베이스가 main이라 enforce-target도 실패했다. 제품 작업은 dev 대상이 규칙이다.

테스트 추가(중계 호스트에서도 동일 sanitize, preview·무관 모델은 필드 유지)와 문서 한 줄 보강 의도는 좋다. 다만 그 테스트도 옛 모놀리스 트리 가정 위라, 지금 dev에 그대로 붙이면 경로·임포트부터 다시 맞춰야 한다. 제목 update는 무엇이 바뀌는지 전혀 안 보여 준다. types.ts/config.ts 스플릿으로 무효화되는 종류는 아니지만, openai-responses 파사드 스플릿 이후 옛 모놀리스를 고치는 PR과 같은 취급이다. 리베이스로 살리기보다 닫고, dev 기준으로 리프에 다시 올리는 편이 싸다.

우선순위 48/80: 버그 진단과 수정 방향은 현재 dev에도 유효하고 재현 이야기도 구체적이다. 하지만 패치가 랜딩 가능한 트리 모양이 아니고, 타깃 브랜치·충돌·제목까지 막혀 있어서 지금 형태로는 머지할 수 없다.

PR 경로 src/adapters/openai-responses.ts - 현재 dev/main에서는 파사드만 남고 본문은 openai-responses/web-search.ts · passthrough.ts로 갈라져 있어, 이 diff는 충돌만 나고 의도한 가드에 닿지 않는다.
web-search.ts 79–83행 MUSE_SPARK_WEB_SEARCH_STRICT_RESPONSE_URLS - 현재 dev에 아직 있는 정확한 URL 허용 목록이 이번 버그의 원인이다. 고칠 자리인데 PR이 여기를 건드리지 않는다.
web-search.ts 99–115행 stripMuseSparkUnsupportedWebSearchFields(..., responseUrl) - URL 파싱·허용 목록 검사가 그대로다. 모델 아이디만으로 판정하려면 시그니처에서 responseUrl을 빼고 이 분기를 제거해야 한다.
passthrough.ts 326행 - 호출이 아직 stripMuseSparkUnsupportedWebSearchFields(outBody, parsed.modelId, url)이다. 리프 포팅 시 인자 url도 함께 빼야 한다.
베이스 main + enforce-target 실패 - 제품 PR은 dev로 열어야 한다. 지금 상태는 머지 게이트에서 막힌다.
제목 update - 무엇을 고치는지 안 보인다. fix(providers): key Muse Spark web_search sanitization on model id 같은 커밋 헤드라인이 PR 제목이어야 한다.
mergeable=CONFLICTING - main에도 이미 파사드가 있어 이 브랜치로는 clean merge가 안 된다.

메인테이너의 판단이 필요한 지점

  • 모델 아이디만으로 sanitize 하는 쪽이 맞는지, 아니면 “알려진 게이트웨이 패밀리 + 모델”처럼 완화된 호스트 힌트를 남길지. (PR 논리는 “이 id들은 그 게이트웨이 패밀리에만 존재한다”.)
  • 기여자에게 닫고 dev 리프 재제출을 요청할지, 메인테이너가 web-search.ts/passthrough.ts로 직접 포팅할지.
  • 중계 호스트가 Muse Spark id를 쓰면서 사실 다른 계약(필드 허용)인 경우가 실제로 있는지. 있으면 모델-only 키잉이 과할 수 있다.

너의 추천
이 PR은 닫으세요(close-don’t-rebase). 의도는 살릴 가치가 있으니, dev 기준으로 src/adapters/openai-responses/web-search.ts에서 URL 허용 목록·responseUrl 인자를 제거하고 passthrough.ts 호출부를 맞춘 뒤, tests/providers/muse-spark-web-search-compat.test.ts에 중계 호스트 케이스를 포팅한 새 PR로 다시 열어 주세요. 제목은 커밋 메시지처럼 구체화하고, hosted CI가 그 헤드 SHA에서 초록일 때만 머지하세요. 지금 diff를 main이나 옛 모놀리스 경로에 리베이스해 살리려 하지 마세요.

이 댓글은 grok-bot이 작성했습니다

@Ingwannu

Copy link
Copy Markdown
Owner

This was correctly closed by the target-branch gate and is not mergeable as submitted. It is based on main, modifies the pre-split monolithic Responses adapter, has an empty description/checklist, and conflicts with current dev.

There is also a product blocker in the proposed rule: a model id is not proof of an upstream gateway in a user-configurable proxy. Operators can reuse muse-spark-* ids on arbitrary compatible endpoints, so removing the destination boundary would silently strip supported web-search fields from unrelated custom routes. A relay-specific fix needs verifiable provider capability/identity (or the existing explicit supportsOpenAiWebSearchToolFields: false contract), not a global model-name heuristic.

If there is a real Console Go relay reproduction, open a fresh PR against current dev, link the issue and upstream evidence, implement it in the current split adapter leaf, and include exact-head focused plus full CI evidence. Please do not reopen this main-based PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants