fix(management): expose the effective web-search sidecar enabled state (reimplements #2033) - #3870
Conversation
…ip ci] (cherry picked from commit 73abec9) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 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, |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
|
✅ 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>
9110472 to
6eadb16
Compare
리뷰 · 우선순위 58 / 80이 PR은 기여자 #2033(louis-tepe)을 현재 릴리스 트레인 맨 위(레이어 6, 설정 타입 테스트는 한 가지 스코프 경계는 분명합니다. 이 변경은 읽기(응답 echo) 만 고칩니다. PUT body 타입과 적용 로직에는 여전히
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
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-settingsnow returnwebSearch.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
tests/vision/sidecar-settings-vision-controls.test.ts"GET and PUT expose the effective web-search enabled state".Checklist
Manual review chain (integrate bottom-up; stack: null, no native stack)
codex/rt-m1-3532devcodex/rt-m2-3840codex/rt-m3-3837codex/rt-m4-3843codex/rt-m5-3845codex/rt-m6-2033Verification 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
devbottom-up. Exact chain-top evidence: Cross-platform CI run 34106345180 at head6eadb1658(lane=all: Linux 4/4, macOS 2/2 + control, Windows 6/6, gates, storage policy, api usage, keyring ×3, npm-global ×3, docker smoke, aggregateci= success). Tested tree7621cac89equals the prospective merge tree oforigin/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.