Repository navigation
Reject unsupported reasoning efforts before proxying - #9520
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Model fallback retries can rewrite the model after validation and proxy an unsupported effort.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds reasoning-effort validation based on runtime model capabilities before proxying requests.
Changes:
- Validates Chat Completions, Responses, and Messages effort fields.
- Returns actionable 400 errors for unsupported values.
- Adds unit/integration coverage and packages the validator.
| File | Description |
|---|---|
containers/api-proxy/reasoning-effort-validation.js |
Implements capability validation. |
containers/api-proxy/reasoning-effort-validation.test.js |
Tests validation behavior. |
containers/api-proxy/body-handler.js |
Runs validation after body transforms. |
containers/api-proxy/server.token-steering.test.js |
Covers routed, steered requests. |
containers/api-proxy/Dockerfile |
Includes the validator in the image. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| } | ||
|
|
||
| validateReasoningEffort(body, provider, req.url); |
| it('rejects an effort not advertised for the resolved model', () => { | ||
| replaceRuntimeModels('copilot', [{ | ||
| id: 'gpt-5-mini', | ||
| supportedReasoningEfforts: ['low', 'medium', 'high'], | ||
| }]); | ||
|
|
||
| expect(() => validateReasoningEffort( | ||
| Buffer.from(JSON.stringify({ model: 'gpt-5-mini', reasoning_effort: 'short' })), | ||
| 'copilot', | ||
| '/v1/chat/completions', | ||
| )).toThrow(expect.objectContaining({ | ||
| statusCode: 400, | ||
| code: 'unsupported_reasoning_effort', | ||
| message: 'reasoning_effort "short" is not supported by model gpt-5-mini; supported values: [low medium high]', | ||
| })); | ||
| }); |
|
@copilot address review feedback |
Both retry paths now validate the rewritten body at upstream dispatch, and unsupported-effort coverage now includes all three endpoint shapes. Full API-proxy tests pass (113 suites, 2,208 tests). Commit: |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅
|
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓
|
|
💎 Smoke Gemini is crystallizing results on this pull request... |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
✅ Build Test Suite completed successfully!
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
🔮 The ancient spirits stir... Smoke Codex awakens to divine this pull request... |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
🔑 Smoke Copilot BYOK AOAI (api-key) is testing Azure OpenAI BYOK (api-key) mode on this pull request... |
|
🪪 Smoke Copilot BYOK AOAI (Entra) is testing Azure OpenAI BYOK (Entra / GitHub OIDC) mode on this pull request... |
|
✅ Security Guard completed successfully! Security review complete: PR #9520 adds reasoning effort validation to the API proxy. No security concerns found. Changes implement input validation that prevents unsupported reasoning efforts from being proxied upstream, with proper error handling (400 responses). No weakened firewall rules, expanded egress, capability additions, or security regressions detected. PR passes security review.
|
|
Smoke Cloud Hypervisor completed. Cloud Hypervisor + Copilot passed.
|
|
✅ Smoke Claude passed
|
Cloud Hypervisor + Copilot Smoke Test
All checks passed.
|
|
Smoke services: ✅ Redis PONG · ✅ pg_isready accepting connections · ✅ SELECT 1 = 1. PASS
|
Smoke Claude Results
Overall result: PASS
|
Copilot BYOK (Direct) Smoke Test: PASS ✅Test Results:
Running in direct BYOK mode via api-proxy sidecar.
|
|
OTEL smoke test (re-run locally)
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
EGRESS_RESULT allow=pass deny=pass
|
|
Smoke Copilot: PASS ✅
|
🏗️ Build Test Suite Results
Overall: 6/8 ecosystems passed — FAIL Failures
All 8 repos cloned successfully. The
|
Chroot Version Comparison
Result: Not all tests passed. Node.js differs between host and chroot, so the
|
Requests to
gpt-5-minican includereasoning_effort: "short", which the model rejects. Available logs don’t establish whether the CLI or another component supplied the value.short→low.{ "error": { "type": "invalid_request_error", "code": "unsupported_reasoning_effort", "message": "reasoning_effort \"short\" is not supported by model gpt-5-mini; supported values: [low medium high]" } }