fix(models): enforce pending selection write guards - #3645
Conversation
Close the valid late review findings from #3636. Preserve authentication and validation order, test pending rejection plus ready/legacy updates, and prove the existing Claude Desktop pending filter without adding a redundant production path. Clarify that opening Models is a user action.
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. |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 73 / 80설명 이 PR은 방금 변경은 작습니다. 프리셋 PUT은 provider·mode 검증 뒤에, 선택 목록 PUT은 provider 존재 확인 뒤에 지금 라인 844-846 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c4ff93973
ℹ️ 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".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds a write fence for pending initial model discovery. Model preset and selected-model updates return HTTP 409 until discovery completes. Tests cover pending, ready, legacy, and validation states. Management API documentation describes the new endpoint and error behavior. ChangesInitial model selection write fence
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to Model-selection writes now return the documented conflict response until initial discovery is ready, without changing saved state; normal ready and legacy editing behavior remains covered. Sequence Diagram(s)sequenceDiagram
participant Client
participant ManagementAPI
participant ModelRoutes
participant Config
Client->>ManagementAPI: PUT model preset or selected model
ManagementAPI->>ModelRoutes: Dispatch management request
ModelRoutes->>Config: Validate input and check discovery status
Config-->>ModelRoutes: Pending, ready, or legacy
alt Pending
ModelRoutes-->>Client: 409 initial_model_selection_pending
else Ready or legacy
ModelRoutes->>Config: Apply model selection update
ModelRoutes-->>Client: 200 updated configuration
end
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (8 skipped: 8 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Keep permanent request errors before the pending-selection conflict, retain ready and legacy editing, and document the conflict/recovery contract in every directly translated API reference. Add unsupported-state and validation-priority regressions without changing authentication or model-value normalization.
|
Owner-authorized admin approval bypass after fresh verification, not a CI waiver or self-approval. Exact head bb05473 passed Cross-platform CI33961600984; actual merge6028fabb67a677f8041ff9a95bb31d53893f2f65/tree f4f770511db04b01f2b9376833a4f4f5012ae1a7 matches. All current checks pass, independent reviews passed, and no valid unresolved findings or direct open child PRs remain. Logs prove25 write cases and the real Desktop GET filter regression; eight-language documentation build passed. Current dev remains cfe95ee. Landing with exact head match; no release or deployment. |
1 participant
Summary
Follow up the valid late review findings on feat(onboarding): guide registrations to model selection #3636: reject preset and selected-model edits while initial discovery is pending, matching the existing visibility-write 409 response.
Preserve existing authentication and input-validation order, and preserve ready/legacy editing behavior.
Clarify that the confirmation dialog lets the user open Models. Keep the existing Claude Desktop production filter; add a regression proving that pending candidates are already excluded.
Preserve permanent JSON/provider/mode/preset-support validation before the pending-state conflict. Add explicit unsupported-provider and validation-precedence regressions.
Document the PUT 409 code and discovery-retry contract in English and all seven directly translated management API references.
Verification
git diff --checkpassed.Checklist
Summary by CodeRabbit
Behavior Changes
Documentation