Repository navigation
feat(codex): CPR-style model_mappings for codex-auto-review (#526) #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| /** | ||
| * CPR-style inbound model id rewrite (client model → upstream GPT id). | ||
| * Applied before detectInboundPlatform; literal detect on raw ids is unchanged. | ||
| */ | ||
| import { isGptSeriesId } from './gpt-ids.mjs' | ||
| import { isGptModelEnabled } from './gpt-model-policy.mjs' | ||
|
|
||
| /** Recommended secondary default when an operator clears the primary auto-review target. */ | ||
| export const DEFAULT_CODEX_MODEL_MAPPING_FALLBACK = 'gpt-6.1-sol' | ||
|
|
||
| export const DEFAULT_CODEX_MODEL_MAPPINGS = Object.freeze({ | ||
| 'codex-auto-review': 'gpt-6-luna', | ||
| }) | ||
|
|
||
| function bareModelId(model) { | ||
| const raw = String(model || '').trim() | ||
| if (!raw) return '' | ||
| return raw.split('/').filter(Boolean).pop() || raw | ||
| } | ||
|
|
||
| function mappingLookup(map, model) { | ||
| const bare = bareModelId(model) | ||
| if (!bare) return null | ||
| const lower = bare.toLowerCase() | ||
| if (Object.prototype.hasOwnProperty.call(map, bare)) return map[bare] | ||
| if (Object.prototype.hasOwnProperty.call(map, lower)) return map[lower] | ||
| for (const [key, target] of Object.entries(map)) { | ||
| if (String(key).toLowerCase() === lower) return target | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| /** | ||
| * Merge routing overrides onto product defaults. Empty/null target for codex-auto-review | ||
| * falls back to {@link DEFAULT_CODEX_MODEL_MAPPING_FALLBACK}; other keys with empty target drop the override. | ||
| */ | ||
| export function normalizeCodexModelMappings(raw) { | ||
| const merged = { ...DEFAULT_CODEX_MODEL_MAPPINGS } | ||
| const src = raw && typeof raw === 'object' && !Array.isArray(raw) ? raw : null | ||
| if (!src) return merged | ||
| for (const [key, value] of Object.entries(src)) { | ||
| const alias = String(key || '').trim() | ||
| if (!alias) continue | ||
| if (value === null || value === '') { | ||
| if (alias.toLowerCase() === 'codex-auto-review') { | ||
| merged[alias] = DEFAULT_CODEX_MODEL_MAPPING_FALLBACK | ||
| } else { | ||
| delete merged[alias] | ||
| if (alias.toLowerCase() !== alias) delete merged[alias.toLowerCase()] | ||
| } | ||
| continue | ||
| } | ||
| merged[alias] = String(value).trim() | ||
| } | ||
| return merged | ||
| } | ||
|
|
||
| export function validateCodexModelMappingTarget(target) { | ||
| const id = String(target || '').trim() | ||
| if (!id || !isGptSeriesId(id)) { | ||
| return { | ||
| ok: false, | ||
| code: 'codex_model_mapping_invalid', | ||
| message: `model mapping target '${id || ''}' must be an enabled GPT chat model id`, | ||
| } | ||
| } | ||
| if (!isGptModelEnabled(id)) { | ||
| return { | ||
| ok: false, | ||
| code: 'model_disabled', | ||
| message: `model mapping target '${id}' is disabled by gateway GPT model policy`, | ||
| } | ||
| } | ||
| return { ok: true, target: id } | ||
| } | ||
|
|
||
| export function validateCodexModelMappingsPatch(raw, path = 'codex.model_mappings') { | ||
| if (raw == null) return | ||
| if (typeof raw !== 'object' || Array.isArray(raw)) { | ||
| throw typedError(`${path} 必须是对象`, 'invalid_codex_model_mappings') | ||
| } | ||
| for (const [key, value] of Object.entries(raw)) { | ||
| const alias = String(key || '').trim() | ||
| if (!alias) throw typedError(`${path} 的键必须是非空字符串`, 'invalid_codex_model_mappings') | ||
| if (value === null || value === '') continue | ||
| if (typeof value !== 'string' || !value.trim()) { | ||
| throw typedError(`${path}.${alias} 必须是字符串、null 或空字符串`, 'invalid_codex_model_mappings') | ||
| } | ||
| const check = validateCodexModelMappingTarget(value.trim()) | ||
| if (!check.ok) throw typedError(`${path}.${alias}: ${check.message}`, check.code) | ||
| } | ||
| } | ||
|
|
||
| function typedError(message, code) { | ||
| const error = new TypeError(message) | ||
| error.code = code | ||
| return error | ||
| } | ||
|
|
||
| function rewriteModelField(model, upstreamId) { | ||
| const raw = String(model || '').trim() | ||
| const parts = raw.split('/').filter(Boolean) | ||
| if (parts.length <= 1) return upstreamId | ||
| parts[parts.length - 1] = upstreamId | ||
| return parts.join('/') | ||
| } | ||
|
|
||
| /** | ||
| * @returns {{ ok: true, body: object, mapped?: boolean, requestedModel?: string, upstreamModel?: string | null } | { ok: false, code: string, message: string }} | ||
| */ | ||
| export function applyCodexInboundModelRewrite(body, routing = {}) { | ||
| if (!body || typeof body !== 'object') return { ok: true, body: body || {} } | ||
| const model = body.model | ||
| if (typeof model !== 'string' || !model.trim()) return { ok: true, body } | ||
| const codex = routing.codex && typeof routing.codex === 'object' ? routing.codex : routing | ||
| const mappings = normalizeCodexModelMappings(codex?.model_mappings) | ||
| const target = mappingLookup(mappings, model) | ||
| if (target == null) return { ok: true, body, requestedModel: model.trim(), upstreamModel: null, mapped: false } | ||
| const validated = validateCodexModelMappingTarget(target) | ||
| if (!validated.ok) return validated | ||
| const upstreamModel = validated.target | ||
| return { | ||
| ok: true, | ||
| body: { ...body, model: rewriteModelField(model, upstreamModel) }, | ||
| mapped: true, | ||
| requestedModel: model.trim(), | ||
| upstreamModel, | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import test from 'node:test' | ||
| import assert from 'node:assert/strict' | ||
| import { detectInboundPlatform } from '../../src/lib/protocol/platform-detect.mjs' | ||
| import { | ||
| applyCodexInboundModelRewrite, | ||
| DEFAULT_CODEX_MODEL_MAPPING_FALLBACK, | ||
| DEFAULT_CODEX_MODEL_MAPPINGS, | ||
| normalizeCodexModelMappings, | ||
| validateCodexModelMappingTarget, | ||
| } from '../../src/lib/protocol/codex-model-mapping.mjs' | ||
| import { normalizeCodexRouting } from '../../src/lib/protocol/codex-route.mjs' | ||
| import { resetGptModelPolicy, saveGptModelPolicy } from '../../src/lib/protocol/gpt-model-policy.mjs' | ||
| import fs from 'node:fs' | ||
| import path from 'node:path' | ||
|
|
||
| test('default map rewrites codex-auto-review before platform detect', () => { | ||
| resetGptModelPolicy() | ||
| const rewrite = applyCodexInboundModelRewrite({ model: 'codex-auto-review' }, {}) | ||
| assert.equal(rewrite.ok, true) | ||
| assert.equal(rewrite.mapped, true) | ||
| assert.equal(rewrite.requestedModel, 'codex-auto-review') | ||
| assert.equal(rewrite.upstreamModel, DEFAULT_CODEX_MODEL_MAPPINGS['codex-auto-review']) | ||
| const platform = detectInboundPlatform(rewrite.body.model) | ||
| assert.equal(platform.ok, true) | ||
| assert.equal(platform.platform, 'openai') | ||
| assert.equal(platform.model, 'gpt-6-luna') | ||
| }) | ||
|
|
||
| test('literal detectInboundPlatform still fail-closes codex-auto-review', () => { | ||
| assert.equal(detectInboundPlatform('codex-auto-review').ok, false) | ||
| }) | ||
|
|
||
| test('custom routing.model_mappings overrides default target', () => { | ||
| resetGptModelPolicy() | ||
| const routing = { codex: { model_mappings: { 'codex-auto-review': 'gpt-6-sol' } } } | ||
| const rewrite = applyCodexInboundModelRewrite({ model: 'codex-auto-review' }, routing) | ||
| assert.equal(rewrite.upstreamModel, 'gpt-6-sol') | ||
| assert.equal(detectInboundPlatform(rewrite.body.model).platform, 'openai') | ||
| }) | ||
|
|
||
| test('empty configured target for codex-auto-review uses fallback sol model', () => { | ||
| resetGptModelPolicy() | ||
| const mappings = normalizeCodexModelMappings({ 'codex-auto-review': '' }) | ||
| assert.equal(mappings['codex-auto-review'], DEFAULT_CODEX_MODEL_MAPPING_FALLBACK) | ||
| const rewrite = applyCodexInboundModelRewrite( | ||
| { model: 'codex-auto-review' }, | ||
| { codex: { model_mappings: { 'codex-auto-review': '' } } }, | ||
| ) | ||
| assert.equal(rewrite.upstreamModel, DEFAULT_CODEX_MODEL_MAPPING_FALLBACK) | ||
| }) | ||
|
|
||
| test('invalid non-GPT mapping target returns codex_model_mapping_invalid', () => { | ||
| resetGptModelPolicy() | ||
| const rewrite = applyCodexInboundModelRewrite( | ||
| { model: 'codex-auto-review' }, | ||
| { codex: { model_mappings: { 'codex-auto-review': 'claude-sonnet-4-6' } } }, | ||
| ) | ||
| assert.equal(rewrite.ok, false) | ||
| assert.equal(rewrite.code, 'codex_model_mapping_invalid') | ||
| }) | ||
|
|
||
| test('disabled GPT mapping target returns model_disabled', () => { | ||
| resetGptModelPolicy() | ||
| saveGptModelPolicy({ | ||
| models: { | ||
| 'gpt-6-luna': { id: 'gpt-6-luna', enabled: false }, | ||
| }, | ||
| }) | ||
| const check = validateCodexModelMappingTarget('gpt-6-luna') | ||
| assert.equal(check.ok, false) | ||
| assert.equal(check.code, 'model_disabled') | ||
| const rewrite = applyCodexInboundModelRewrite({ model: 'codex-auto-review' }, {}) | ||
| assert.equal(rewrite.ok, false) | ||
| assert.equal(rewrite.code, 'model_disabled') | ||
| }) | ||
|
|
||
| test('normalizeCodexRouting hot-loads model_mappings defaults', () => { | ||
| const codex = normalizeCodexRouting({}) | ||
| assert.equal(codex.model_mappings['codex-auto-review'], 'gpt-6-luna') | ||
| }) | ||
|
|
||
| test('handle-protocol applies codex model rewrite before detectInboundPlatform', () => { | ||
| const src = fs.readFileSync(path.join(process.cwd(), 'src/lib/protocol/handle-protocol.mjs'), 'utf8') | ||
| const rewriteAt = src.indexOf('applyCodexInboundModelRewrite') | ||
| const detectAt = src.indexOf('detectInboundPlatform(ctx.body?.model)') | ||
| assert.ok(rewriteAt > 0 && detectAt > rewriteAt) | ||
| }) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback mapping target not catalog-safe
Medium Severity
Clearing
codex-auto-reviewis documented to fall back togpt-6.1-sol, but that id is not inseedGptPolicy, patch validation skips empty values, and persist then stores the resolved fallback. After a Codex catalog sync, unseen GPT ids start disabled (or are unknown), so auto-review requests 400 withmodel_disabled, and a later PUT of the GET payload can fail even though the empty save succeeded.Additional Locations (1)
src/lib/protocol/codex-model-mapping.mjs#L76-L91Reviewed by Cursor Bugbot for commit 847c2cb. Configure here.