fix(shadow-call): follow up on #2849 self-target gaps #2852
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
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
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 |
|---|---|---|
|
|
@@ -65,6 +65,7 @@ import { isPlainRecord, parseDebugLogQuery, tokPerSecondResult, unavailableCostR | |
| import type { MetricUnavailableReason, TokPerSecondResult, CostEstimateReason, CostResult, MetricSource } from "./shared"; | ||
| import type { ManagementContext } from "./context"; | ||
| import { readManagementJsonBody, rethrowManagementBodyTooLarge } from "./body"; | ||
| import { shadowCallTargetError } from "./shadow-call-validation"; | ||
|
|
||
|
|
||
| /** Management wire shape: omit default imageInput "auto" (persist/response sparse). */ | ||
|
|
@@ -169,7 +170,6 @@ export async function handleComboRoutes(ctx: ManagementContext): Promise<Respons | |
| const nextCombos = { ...(config.combos ?? {}) }; | ||
| if (renameFrom) delete nextCombos[renameFrom]; | ||
| nextCombos[id] = stored; | ||
| let shouldSyncClaudeAgentDefs = false; | ||
| const migratedModels = new Map<string, string>(); | ||
| if (oldPublicModel && oldPublicModel !== newPublicModel && previous?.nativeAlias !== true) { | ||
|
|
@@ -183,6 +183,15 @@ export async function handleComboRoutes(ctx: ManagementContext): Promise<Respons | |
| previous?.nativeAlias === true ? comboModelId(id) : newPublicModel, | ||
| ); | ||
| } | ||
| const currentShadowTarget = config.shadowCallIntercept?.model; | ||
| const migratedShadowTarget = currentShadowTarget | ||
| ? migratedModels.get(currentShadowTarget) | ||
| : undefined; | ||
| if (migratedShadowTarget) { | ||
| const targetError = shadowCallTargetError({ ...config, combos: nextCombos }, migratedShadowTarget); | ||
| if (targetError) return jsonResponse({ error: targetError }, 400); | ||
|
Comment on lines
+190
to
+192
There was a problem hiding this comment.
When AGENTS.md reference: src/AGENTS.md:L24-L26 Useful? React with 👍 / 👎. |
||
| } | ||
| config.combos = nextCombos; | ||
| if (migratedModels.size > 0) { | ||
| const migrateReference = (model: string): string => migratedModels.get(model) ?? model; | ||
| const migrateAgentReference = (model: string): string => { | ||
|
|
||
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,29 @@ | ||
| import { shadowCallTargetsIntersect, shadowSourceModels } from "../../lib/shadow-call"; | ||
| import { OPENAI_CODEX_PROVIDER_ID } from "../../providers/openai-tiers"; | ||
| import { routeConcreteModel, routeModel } from "../../router"; | ||
| import type { OcxConfig } from "../../types"; | ||
|
|
||
| /** Validate a prospective persisted shadow-call target against its resolved source identities. */ | ||
| export function shadowCallTargetError(config: OcxConfig, targetModel: string | undefined): string | null { | ||
| if (!targetModel) return null; | ||
|
|
||
| let target; | ||
| try { | ||
| target = routeModel(config, targetModel); | ||
| } catch { | ||
| return "model must resolve to a configured provider"; | ||
| } | ||
|
|
||
| const intersectsSource = shadowSourceModels(config.shadowCallIntercept?.sourceModels).some(sourceModel => { | ||
| let source = { providerName: OPENAI_CODEX_PROVIDER_ID, modelId: sourceModel }; | ||
| try { | ||
| const resolved = routeConcreteModel(config, sourceModel); | ||
| source = { providerName: resolved.providerName, modelId: sourceModel }; | ||
|
Comment on lines
+20
to
+21
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- applicable repository knowledge ---'
find /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- changed hunk ---'
git diff -- src/server/management/shadow-call-validation.ts
printf '%s\n' '--- target file outline ---'
ast-grep outline src/server/management/shadow-call-validation.ts
printf '%s\n' '--- target file relevant source ---'
cat -n src/server/management/shadow-call-validation.ts | sed -n '1,180p'
printf '%s\n' '--- routeConcreteModel definitions and callers ---'
rg -n -C 8 'routeConcreteModel|routeModel' srcRepository: lidge-jun/opencodex Length of output: 50375 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- source conventions ---'
cat /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732/conventions/src.md
printf '%s\n' '--- shadow-call helpers ---'
ast-grep outline src/lib/shadow-call.ts
cat -n src/lib/shadow-call.ts | sed -n '1,220p'
printf '%s\n' '--- router types and concrete routing ---'
cat -n src/router.ts | sed -n '1,90p'
cat -n src/router.ts | sed -n '565,780p'
printf '%s\n' '--- route result construction and provider/model parsing ---'
rg -n -C 12 'function routeResult|const routeResult|providerName:|modelId:' src/router.ts | head -220Repository: lidge-jun/opencodex Length of output: 31320 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- provider model catalog resolution ---'
rg -n -C 12 'function knownModelIdsForProvider|knownModelIdsForProvider\(' src/router.ts src/providers src
printf '%s\n' '--- xai provider definitions and model fields ---'
rg -n -C 8 'xai|custom-helper' src/providers src/types.ts src/server/management tests 2>/dev/null | head -260
printf '%s\n' '--- management validation callers and status mapping ---'
rg -n -C 14 'shadowCallTargetError|shadow-call target must not intersect|sourceModels' src/server/management tests 2>/dev/null | head -320Repository: lidge-jun/opencodex Length of output: 50375 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- complete known-model implementation ---'
cat -n src/router.ts | sed -n '92,145p'
printf '%s\n' '--- shadow-call management update path ---'
cat -n src/server/management/config-routes.ts | sed -n '847,930p'
printf '%s\n' '--- direct callers of shadowCallTargetError ---'
rg -n -C 10 'shadowCallTargetError' src/server/management/config-routes.ts src/server/management/*.tsRepository: lidge-jun/opencodex Length of output: 23127 Use At Assign 🤖 Prompt for AI AgentsSource: Path instructions |
||
| } catch { /* Unconfigured native Codex source models remain OpenAI-owned. */ } | ||
| return shadowCallTargetsIntersect(source, target); | ||
| }); | ||
|
|
||
| return intersectsSource | ||
| ? "shadow-call target must not intersect a source model" | ||
| : null; | ||
| } | ||
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
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize provider-qualified prefixes before matching model IDs.
At Line 389,
modelIdkeeps the full selector. Forxai/gpt-5.6, the resolved identity becomes{ provider: "xai", modelId: "xai/gpt-5.6" }. At Lines 391-392, nomodel.idcan match that value because model IDs do not include the provider prefix. Same-provider variants can therefore remain selectable.Parse the canonical
provider/modelselector first. Keep the provider inprovider, and compare the unqualified, correctly decoded model ID againstmodel.id. Add a regression case for a provider-qualified prefix.As per path instructions, preserve provider-qualified identities and distinguish provider/model prefixes from bare native Codex models.
🤖 Prompt for AI Agents
Source: Path instructions