Skip to content

fix(management): validate modelSupportsVerbosity record shape - #5273

Draft
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/provider-verbosity-validation
Draft

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/provider-verbosity-validation

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • providerManagementConfigError shape-checked sibling capability records (modelSupportsReasoningSummaries, modelSuppressSyntheticMax) but not modelSupportsVerbosity, so an invalid value (array, non-boolean values, empty keys) could persist and reach consumers unchecked.
  • Adds the same booleanRecordConfigError validation for modelSupportsVerbosity, plus route-level coverage for accepted/rejected shapes.

Verification

  • bun test tests/server/management-provider-validation.test.ts — 137 pass, 0 fail
  • bun x tsc --noEmit — clean

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.

Summary by CodeRabbit

  • Bug Fixes

    • Improved provider configuration validation for verbosity support settings.
    • Invalid verbosity configurations now return clear errors instead of being accepted.
  • Tests

    • Added coverage for valid verbosity settings and invalid values, arrays, and empty keys.

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.

The provider management validator shape-checked sibling capability records but not modelSupportsVerbosity, so an invalid value (array, non-boolean values, empty keys) could persist and reach consumers unchecked.
@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c44c6dec-bff6-4630-97ab-87005f41d546

📥 Commits

Reviewing files that changed from the base of the PR and between 447ac22 and 3f5c4f8.

📒 Files selected for processing (2)
  • src/server/auth-cors.ts
  • tests/server/management-provider-validation.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The provider management validation now checks modelSupportsVerbosity and reports invalid configurations. Tests cover valid persistence and rejection of invalid arrays, values, and keys.

Changes

Provider verbosity validation

Layer / File(s) Summary
Validate model verbosity configuration
src/server/auth-cors.ts, tests/server/management-provider-validation.test.ts
providerManagementConfigError validates modelSupportsVerbosity with booleanRecordConfigError and prefixes failures with the provider name. Management API tests verify valid persistence and HTTP 400 responses for invalid shapes, non-boolean values, and empty keys.

Priority: ⬇️ Low

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

Change: Bug fix

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: validating the modelSupportsVerbosity record shape in the management API.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

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 added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

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

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 59 / 80

이 PR은 관리 화면에서 제공자 설정을 저장할 때, modelSupportsVerbosity 값의 모양을 검사하지 않던 구멍을 막습니다. 같은 종류의 지도인 modelSupportsReasoningSummariesmodelSuppressSyntheticMax는 이미 booleanRecordConfigError로 걸렀는데, verbosity만 빠져 있었습니다. 그래서 배열이거나, 값이 불리언이 아니거나, 키 문자열이 비어 있어도 디스크에 남을 수 있었습니다. 이번 변경은 providerManagementConfigError 안에 형제와 같은 검사 두 줄을 넣고, /api/providers POST로 좋은 값·나쁜 값을 받는 테스트를 형제와 같은 꼴로 붙였습니다. 베이스는 dev가 맞습니다. types.ts/config.ts 분할과 무관하고, 같은 주제로 닫을 다른 열린 PR도 없습니다. 본문에 적힌 로컬 테스트·타입체크는 이 원격 머리에서 다시 돌리지 않았습니다. 호스티드 CI는 글을 쓰는 지금 hygiene 실패·label 통과·resolve-pr 대기 상태입니다.

라인 - tests/server/management-provider-validation.test.ts: 이 브랜치 머지 베이스는 5498줄이고, 이 PR이 30줄을 더해 5528줄이 됩니다. 저장소 캡은 tests/fixtures/file-size-baseline.json의 5506입니다. 그대로면 file-size ratchet이 빨개집니다. 예전에도 같은 파일은 형제 파일로 케이스를 옮기는 처방(#5011, #5018 계열)을 썼습니다.

라인 - hygiene / 라벨 intake: hygiene-blocked: 경로가 src/server/auth-cors.tsunsponsored_surface로 막혔습니다. 실제 변경은 검증 두 줄뿐이고 CORS·인증 로직을 바꾸지 않지만, 레인은 메인테이너가 보고 maintainer-sponsored를 붙이기 전에는 진행이 안 됩니다.

라인 - PR 본문 체크리스트: 세 칸 모두 비어 있습니다. 범위는 이미 좁고, 문서·릴리즈 노트는 이 구멍 막기에 필수는 아니어 보입니다. 다만 Ready 전에 체크는 채우는 편이 좋습니다.

라인 - supportsVerbosity(제공자 전체 불리언): 에디터 필드로 열려 있지만, 이번 검사와 같은 함수 안에서 typeof === "boolean" 가드가 없습니다. 이번 PR 범위 밖인 형제 구멍입니다. 이번 머지를 막을 이유는 아닙니다.

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

auth-cors 경로의 검증 한 줄 추가를 보안 표면으로 보고 sponsored를 붙일지, 아니면 검증만의 변경이라 짧은 확인 뒤 바로 붙일지. 테스트 30줄을 같은 파일에 두고 캡을 넘기게 둘지(불가에 가깝다), 형제 파일로 바로 쪼갤지.

너의 추천

코드 방향은 맞고 머지 후보다다. 먼저 verbosity 테스트를 형제 파일로 옮겨 file-size 캡 아래로 맞추고, 메인테이너가 검증-only임을 확인한 뒤 maintainer-sponsored를 붙이세요. 그다음 exact-head에서 해당 server 테스트·래칫이 초록이면 머지하면 됩니다. supportsVerbosity 불리언 가드는 후속 한 줄로 열어도 됩니다. types/config 분할로 닫을 PR이 아니고, 중복 닫기 대상도 없습니다. preview deploy 이야기는 하지 않았습니다.

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

@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

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.

4/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@github-actions
github-actions Bot marked this pull request as draft September 20, 2026 06:23
@luvs01
luvs01 marked this pull request as ready for review September 20, 2026 06:50
@github-actions
github-actions Bot marked this pull request as draft September 20, 2026 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants