feat: recursive alias resolution with cycle protection in validateRequestedModel#6054
Conversation
…uestedModel - Add _resolveModelForValidation helper that uses resolveModel() for proper recursive alias resolution with cycle detection - Remove early direct model-ID check — resolveModel handles direct matches via _resolveDirectMatch, so the pre-check was redundant - Log each resolution step at debug level (model_validation_step) - Add tests for recursive alias chains and cycle detection
- Use alias-key presence check for accurate resolved_via classification - Use descriptive fixture names (top-alias/mid-alias, cycle-a/cycle-b) - Assert number of debug log steps for multi-hop alias chain - Assert no success log is emitted when cycle breaks resolution
|
@copilot add test suite for model alias resolution with complex trees of model ids. |
Added a
Commit: 8a88e18 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR aligns startup-time AWF_REQUESTED_MODEL validation with request-time model resolution by routing validation through resolveModel (including recursive alias chains and cycle detection), and adds tests to cover deeper alias graphs and cycles.
Changes:
- Refactors startup validation to use a single resolution path (
resolveModel) across providers (with middle-power fallback disabled) and adds per-step debug logging (model_validation_step). - Improves
resolved_viaclassification by detecting alias-key usage (instead of relying only on string equality). - Expands unit test coverage for multi-hop alias chains, cyclic aliases, and larger alias trees.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/key-validation.js | Adds _resolveModelForValidation, removes the early direct-match bypass, emits stepwise debug logs, and refines resolved_via logic. |
| containers/api-proxy/server.startup-model-validation.test.js | Adds startup validation tests for recursive alias chains, cycles, and debug-step logging payloads. |
| containers/api-proxy/model-resolver.test.js | Adds a comprehensive test suite for deep/wide alias trees, branching, dedupe, and multiple cycle shapes. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Low
| // Confirm model_validation_step debug entries cover both alias hops | ||
| const debugCalls = isolatedLog.mock.calls.filter(c => c[1] === 'model_validation_step'); | ||
| expect(debugCalls.length).toBeGreaterThanOrEqual(2); |
|
⏳ Copilot review left inline comments. @copilot To proceed:
|
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Claude passed |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Contribution Check completed successfully! PR #6054 follows the applicable CONTRIBUTING.md guidelines based on the prefetched metadata, diff, and contribution guidance; no review comment needed. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed. Connectivity and GitHub MCP tool were unreachable/missing. File writing and bash tools passed. Overall status: FAIL. |
|
🔌 Smoke Services — All services reachable! ✅ |
|
🚀 Security Guard has started processing this pull request |
|
✅ Build Test Suite completed successfully! |
|
❌ Smoke Copilot BYOK reports failed. BYOK mode investigation needed... |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
🔑 Smoke Copilot BYOK AOAI (api-key) is testing Azure OpenAI BYOK (api-key) mode on this pull request... |
Smoke Test\n-
|
Smoke Test: Copilot PAT Auth
Overall: PASS (core connectivity verified) Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Claude Engine Validation
Overall Result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Services Connectivity
Overall: FAIL — Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🔥 Smoke Test Results
Overall: PASS Thanks Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed. Python minor version differs (3.12.13 vs 3.12.3) and Node.js major version differs (v24 vs v22). The Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass. 🎉 Warning Firewall blocked 4 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "127.0.0.1"
- "api.example.com"
- "api.openai.com"
- "awmgmcpg"See Network Configuration for more information.
|
|
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall Status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
validateRequestedModelused an early direct string match (allModels.some(...)) before trying alias resolution — bypassingresolveModel's recursive chain and cycle detection entirely. This meant multi-hop aliases weren't followed and cyclic aliases weren't guarded at startup validation time.Changes
containers/api-proxy/key-validation.js_resolveModelForValidationhelper: delegates toresolveModelacross all providers (fallback disabled), making it the single resolution path for both direct and alias casesallModels.some(m => m.toLowerCase() === normalizedRequested)dropped —_resolveDirectMatchinsideresolveModelcovers thisresolved_viaclassification: checks alias-key presence in the map rather than comparing strings, correctly labels 'alias' even when alias and model names coincidemodel_validation_stepevents, consistent with request-time resolution logging inmakeModelBodyTransformtestHelpersexport for unit testing via@internalconventioncontainers/api-proxy/server.startup-model-validation.test.jstop-alias → mid-alias → copilot/*claude*— verifies multi-hop resolution and correct debug step countcycle-a ↔ cycle-b— verifies loop breaks surface asmodel_unavailable_at_startupwith no false-positive success eventmodel_validation_stepevents carry provider context