Repository navigation
fix(models): apply new-model policy before discovery, sync and export publication #6331
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
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 |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { isDeepStrictEqual } from "node:util"; | ||
| import { mutatePersistedConfig, readConfigDiagnostics, validateConfigCandidate } from "../config"; | ||
| import { fileBackedConfigPath } from "../config/rebase-provenance"; | ||
| import { getConfigPath } from "../config/paths"; | ||
| import { observeModelCacheRevision } from "../codex/model-cache"; | ||
| import { adoptPersistedModelDiscovery } from "../config/live-reconcile"; | ||
| import type { OcxConfig } from "../types"; | ||
| import { reconcileSuccessfulModelDiscoveries } from "./new-model-policy"; | ||
|
|
||
| function inventory(config: OcxConfig): unknown { | ||
| const validated = validateConfigCandidate(config); | ||
| if (!validated.ok) return null; | ||
| const providers = Object.fromEntries(Object.entries(validated.config.providers).map(([name, provider]) => [name, { | ||
| ...provider, | ||
| // Initial selection may finish during this discovery, but a re-registration is distinct. | ||
| initialModelSelection: provider.initialModelSelection?.registrationId, | ||
| }])); | ||
| // Compare normalized persisted data, not injected fetch functions. Never log this identity. | ||
| return JSON.parse(JSON.stringify({ providers, combos: validated.config.combos, | ||
| policy: validated.config.modelDiscovery?.newModelPolicy })); | ||
| } | ||
|
|
||
| export function captureModelDiscoveryBaseline(config: OcxConfig) { | ||
| const identity = inventory(config); | ||
| const disk = readConfigDiagnostics(); | ||
| return { identity, persisted: identity !== null && disk.source === "file" | ||
| && isDeepStrictEqual(identity, inventory(disk.config)) }; | ||
| } | ||
|
|
||
| /** Publish new-arrival choices before returning discovery rows to any visibility consumer. */ | ||
| export function finalizeModelDiscovery( | ||
| config: OcxConfig, | ||
| baseline: ReturnType<typeof captureModelDiscoveryBaseline>, | ||
| models: Iterable<{ provider: string; id: string; custom?: boolean }>, | ||
| authoritativeProviders: string[], | ||
| revisions: Map<string, string>, | ||
| ): boolean { | ||
| const filePath = fileBackedConfigPath(config); | ||
| const rows = [...models]; | ||
| const now = new Date().toISOString(); | ||
| const current = () => (filePath === undefined || filePath === getConfigPath()) | ||
| && isDeepStrictEqual(baseline.identity, inventory(config)) | ||
| && [...revisions].every(([provider, revision]) => observeModelCacheRevision(provider) === revision); | ||
| if (!current()) return false; | ||
| const reconcile = (target: OcxConfig) => reconcileSuccessfulModelDiscoveries({ | ||
| config: target, models: rows, authoritativeProviders, now, mode: "discovery", | ||
| }); | ||
| if (!baseline.persisted && filePath === undefined) { | ||
| // Standalone/synthetic callers may project their own config, never overwrite another home. | ||
| reconcile(config); | ||
| return true; | ||
| } | ||
| // A file-loaded instance remains file-backed after inventory drift. The locked | ||
| // freshness gate below refuses stale rows without creating unsaved live disables. | ||
| try { | ||
| const result = mutatePersistedConfig(fresh => { | ||
| if (!current() || !isDeepStrictEqual(baseline.identity, inventory(fresh))) { | ||
| return { changed: false, value: null }; | ||
| } | ||
| // Re-evaluate against the latest baseline and manual choices under the mutation lock. | ||
| // A second caller then adopts a first caller's result rather than disabling an ID twice. | ||
| const changed = reconcile(fresh); | ||
| return { changed, value: { modelDiscovery: fresh.modelDiscovery, disabledModels: fresh.disabledModels } }; | ||
| }); | ||
| if (result.status === "unavailable" || !result.value) return false; | ||
| adoptPersistedModelDiscovery(config, result.value); | ||
| return true; | ||
| } catch { | ||
| // Do not publish an unrecorded arrival when persistence failed. The caller retries discovery. | ||
| return false; | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
Treating every unmarked object as synthetic reintroduces the stale-config overwrite in the actual server path:
startServerpasses the result ofloadConfig()throughrunOpenAiTierStartupMigration, whose projector alwaysstructuredClones the config even on a no-op, so the long-lived config loses the new WeakMap provenance. If the file changes before a later discovery begins,baseline.persistedis false and this branch mutates the stale live object instead of refusing it; an operator's persisted enable can therefore become disabled in memory and be written back by an unrelated save. Preserve/transfer provenance through startup transformations, or require explicit synthetic provenance rather than inferring it fromfilePath === undefined.Useful? React with 👍 / 👎.