Skip to content

fix(management): expose the effective web-search sidecar enabled state (reimplements #2033) - #3870

Merged
lidge-jun merged 2 commits into
devfrom
codex/rt-m6-2033
Sep 7, 2026
Merged

fix(management): expose the effective web-search sidecar enabled state (reimplements #2033)#3870
lidge-jun merged 2 commits into
devfrom
codex/rt-m6-2033

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

Reimplements #2033 by @louis-tepe on current dev (the original is 1364 commits behind and its test moved under tests/vision/). GET and PUT /api/sidecar-settings now return webSearch.enabled (unset reads as enabled) so the dashboard can tell whether the sidecar master switch is off. Chain top: this head carries the full Cross-platform CI run for the cumulative tree.

(carried/reimplemented from #2033; Co-authored-by trailer in the commit)

Verification

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.

Manual review chain (integrate bottom-up; stack: null, no native stack)

Layer Branch Base Source
1 codex/rt-m1-3532 dev #3532 (Ingwannu)
2 codex/rt-m2-3840 layer 1 #3840 (chilung-cgu)
3 codex/rt-m3-3837 layer 2 #3837 (luvs01) + test isolation fix
4 codex/rt-m4-3843 layer 3 #3843 (luvs01) + same-delta fix
5 codex/rt-m5-3845 layer 4 #3845 (luvs01)
6 codex/rt-m6-2033 layer 5 #2033 (louis-tepe) reimplemented — chain top

Verification policy (maintainer instruction, this train): local test suite / typecheck / build were NOT RUN; branches pushed with --no-verify. Lower layers carry [skip ci]; the full Cross-platform CI (lane=all, Windows shards included) runs once at the chain top head and is the exact-head evidence for the cumulative tree.

Layer 6 of 6. Review this PR's diff only.


Maintainer integration decision (MAINTAINERS.md, dev-only admin integration): @lidge-jun integrates this manual chain into dev bottom-up. Exact chain-top evidence: Cross-platform CI run 34106345180 at head 6eadb1658 (lane=all: Linux 4/4, macOS 2/2 + control, Windows 6/6, gates, storage policy, api usage, keyring ×3, npm-global ×3, docker smoke, aggregate ci = success). Tested tree 7621cac89 equals the prospective merge tree of origin/dev@ece556a6e + chain top. Independent chain review PASS; #3845 security review PASS (see #3869). Local suites NOT RUN by maintainer instruction. This is maintainer integration, not self-approval. Lower-layer PR runs are skipped/cancelled by design ([skip ci]); they are not passing evidence on their own.

…ip ci]

(cherry picked from commit 73abec9)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 7, 2026 09:24
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: b1cf95e0-4cbf-4843-bb60-50e68acc1a34

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T09:28:07.253434Z 9110472 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 911047281b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const webSearchCandidates = await webSearchCandidateRows(config);
return jsonResponse({
webSearch: {
enabled: ws.enabled !== false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the promised co-author trailer

This commit explicitly reimplements #2033 by @louis-tepe, but its parsed trailer list is empty; the prose claiming that a trailer exists does not preserve contributor attribution in GitHub’s contributor graph after squash. Add an actual Co-authored-by: trailer naming the original author.

AGENTS.md reference: AGENTS.md:L279-L283

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale finding: the reviewed commit predates the current head. Current head 6eadb16 carries the trailer; git show -s --format='%(trailers)' 6eadb16 prints Co-authored-by: louis-tepe 161487332+louis-tepe@users.noreply.github.com. Resolving.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

GET and PUT /api/sidecar-settings return webSearch.model/backend/stream flags
but omitted webSearch.enabled, so the dashboard could not tell whether the
sidecar master switch was off. Serialize the effective value (unset reads
as enabled) on both responses.

Reimplements #2033 on current dev
(the original branch is 1364 commits behind and its test moved to
tests/vision/ under the test-layout split).

Co-authored-by: louis-tepe <161487332+louis-tepe@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 58 / 80

이 PR은 기여자 #2033(louis-tepe)을 현재 릴리스 트레인 맨 위(레이어 6, codex/rt-m6-2033, base codex/rt-m5-3845)로 다시 올린 재구현입니다. 원본 브랜치는 dev보다 약 1364커밋 뒤처져 있고, 예전에 루트에 있던 테스트가 지금은 tests/vision/ 아래로 옮겨진 상태라서, 그대로 rebase 하는 대신 같은 계약을 새 HEAD에 맞춰 다시 심은 선택이 맞습니다. 지금 dev HEAD는 ece556a6e(2.47.0 + #3854 code-mode host contract)이고, 여기 GET/PUT /api/sidecar-settings 응답의 webSearch 객체에는 아직 enabled가 없습니다. 반면 vision 쪽은 이미 publicVisionSidecarSettingsenabled: vs.enabled !== false를 항상 내려줍니다. 대시보드가 vision 마스터 스위치는 읽을 수 있는데 web-search 마스터 스위치는 응답에 없어서, 꺼져 있는지 켜져 있는지(또는 unset인지) 구분할 수 없는 비대칭이 남아 있습니다.

설정 타입 OcxWebSearchSidecarConfig에는 원래부터 enabled?: boolean이 있고, 주석도 "Master switch. Default: enabled…"라고 적혀 있습니다. 런타임도 이미 이 값을 봅니다. src/web-search/index.tsplanWebSearchcfg.enabled === false이면 바로 undefined를 돌려서 사이드카를 끄고, openai replay 경로 판별도 cfg.enabled !== false를 씁니다. 즉 저장소와 실행 경로는 이미 "unset이면 켜짐, 명시적 false면 꺼짐"인데, management 응답만 그 effective 값을 빼먹고 있었던 겁니다. 이 PR은 GET과 PUT 성공 응답 양쪽에 enabled: ws.enabled !== false 한 줄을 넣어 vision과 같은 effective 의미를 맞춥니다. PUT으로 streamRoutedModelOutput만 바꿔도 응답에 현재 enabled가 따라오니, 대시보드가 부분 저장 후에도 마스터 스위치 상태를 잃지 않습니다.

테스트는 tests/vision/sidecar-settings-vision-controls.test.ts에 "GET and PUT expose the effective web-search enabled state"를 추가합니다. unset 설정은 true, webSearchSidecar: { enabled: false }false, 그리고 enabled가 false인 채로 streamRoutedModelOutput: true만 PUT해도 응답 webSearch.enabled가 여전히 false인지 확인합니다. 원본 #2033과 같은 단언이고, 테스트 레이아웃 분할 이후 경로만 tests/vision/으로 맞춰졌습니다. 추가로 tests/vision/vision-anthropic.test.ts의 두 군데 strict equality가 enabled: true를 기대하도록 고쳤습니다. GET 응답 객체에 필드가 하나 붙으면 예전 스냅샷 단언이 깨지므로, 이 갱신은 필수 위생입니다. 체인 설명대로 로컬 suite는 돌리지 않았고 Cross-platform CI는 체인 top head 6eadb1658에서 한 번 돌리는 정책이니, 이 레이어 diff만 보면 merge 후보로 충분합니다. types/config 분할에 걸려 무효화될 PR도 아닙니다.

한 가지 스코프 경계는 분명합니다. 이 변경은 읽기(응답 echo) 만 고칩니다. PUT body 타입과 적용 로직에는 여전히 webSearch.enabled 쓰기 경로가 없습니다. vision은 body.vision.enabled를 boolean으로 받아 true면 키를 지우고(default-on), false면 저장하는 대칭이 이미 있습니다. web-search는 설정 파일/다른 경로로 enabled: false를 넣은 뒤 management가 그 값을 보여 주는 단계까지만 갑니다. 대시보드 토글까지 같은 API로 끝내려면 후속 PR이 필요합니다. 이번 #2033 재구현 범위와 원본 diff가 같아서, 의도적 최소 스코프로 보는 게 맞습니다.

src/server/management/config-routes.ts GET /api/sidecar-settings - webSearch.enabled: ws.enabled !== false를 추가해 unset을 켠 상태로 노출. vision의 publicVisionSidecarSettings와 같은 effective 의미.

src/server/management/config-routes.ts PUT 성공 응답 - 같은 enabled echo. 부분 업데이트 후에도 대시보드가 마스터 스위치를 잃지 않음.

tests/vision/sidecar-settings-vision-controls.test.ts - unset→true, false→false, PUT 부분 저장 후 false 유지. 계약 회귀를 직접 잠금.

tests/vision/vision-anthropic.test.ts - 응답 equality에 enabled: true 반영. 필드 추가에 따른 스냅샷 깨짐 방지.

PUT webSearch.enabled 쓰기 - 이번 diff에 없음. 읽기 전용 노출이라 대시보드 토글은 아직 불가(후속 후보).

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

  • 체인 top(fix(management): expose the effective web-search sidecar enabled state (reimplements #2033) #3870) CI가 초록이면 레이어 1→6을 순서대로 dev에 넣을지, 이미 하층이 랜딩된 뒤 이 PR만 fast-forward로 넣을지
  • 이 PR 랜딩 직후 원본 #2033을 Landed via #3870 at <commit> + landed-via-maintainer로 바로 닫을지(원본은 여전히 OPEN)
  • web-search enabled PUT 쓰기(vision과 대칭)를 바로 이어서 열지, 대시보드가 읽기만 필요한 동안 미룰지
  • 체인 top Cross-platform CI만 믿는 정책을 이 레이어에도 그대로 둘지

너의 추천
릴리스 트레인 레이어 6(체인 top)으로 머지하세요. diff는 작고, 런타임·타입에 이미 있는 master switch를 management 응답에 맞춰 vision과 대칭을 맞추는 정확한 재구현입니다. 머지 후 원본 #2033에는 Landed via #3870 at <commit> 코멘트를 남기고 landed-via-maintainer 라벨을 붙인 뒤 completed/superseded로 닫으세요. 대시보드에서 스위치를 직접 끄고 켜야 하면 vision과 같은 PUT webSearch.enabled 쓰기를 짧은 후속으로 열면 됩니다.

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

@lidge-jun
lidge-jun changed the base branch from codex/rt-m5-3845 to dev September 7, 2026 10:11
@lidge-jun
lidge-jun merged commit d00615d into dev Sep 7, 2026
37 of 53 checks passed
@lidge-jun
lidge-jun deleted the codex/rt-m6-2033 branch September 7, 2026 10:11
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.

2 participants