Skip to content

feat(registry): remote-refresh Codex client model catalog#4276

Merged
sususu98 merged 1 commit into
router-for-me:devfrom
sususu98:codex/upstream-dev-20260713
Jul 13, 2026
Merged

feat(registry): remote-refresh Codex client model catalog#4276
sususu98 merged 1 commit into
router-for-me:devfrom
sususu98:codex/upstream-dev-20260713

Conversation

@sususu98

Copy link
Copy Markdown
Collaborator

Summary

  • refresh codex_client_models.json from the shared models repository alongside models.json
  • validate remote catalogs and atomically keep the current embedded or last-known-good snapshot when refreshes fail
  • reload Codex client templates by catalog revision, including Home mode where model IDs still come from Home
  • bake the validated Codex catalog into CI, Docker, and release builds

Validation

  • go test ./...
  • go test -race ./internal/registry ./sdk/api/handlers/openai
  • go vet ./cmd/server ./cmd/fetch_codex_models ./cmd/validate_codex_models ./internal/registry ./internal/api ./sdk/api/handlers/openai
  • go build -o test-output ./cmd/server && rm test-output
  • bash -n .github/scripts/refresh-model-catalogs.sh
  • actionlint on the changed workflows
  • validated the current router-for-me/models Codex client catalog

Load codex_client_models.json like models.json with validation, revisioned
handler reloads, CI bake, and Home-aware updater gates so list IDs still
come from Home while templates can refresh on the edge.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces dynamic fetching, validation, and reloading of the Codex client model catalog (codex_client_models.json). It adds a background updater, a dedicated validation command, and updates the server and API handlers to reload templates when the catalog revision changes. The review feedback suggests two key improvements: first, avoid discarding successfully read catalog data if resp.Body.Close() returns an error; second, optimize template loading concurrency by replacing the exclusive sync.Mutex with a sync.RWMutex and implementing a double-checked locking pattern to prevent lock contention on /v1/models requests.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +99 to +102
if errClose != nil {
log.Debugf("Codex client models response close failed for %s: %v", sourceURL, errClose)
continue
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Discarding the successfully fetched and read catalog data solely because resp.Body.Close() returned an error is overly strict. Transport-level errors during connection teardown can occur even when the payload has been fully and correctly read. It is safer to log the close error and proceed with the validated data.

		if errClose != nil {
			log.Debugf("Codex client models response close failed for %s: %v", sourceURL, errClose)
		}

codexClientModelTemplates map[string]map[string]any
codexClientDefaultTemplate map[string]any
codexClientModelTemplatesErr error
codexClientModelTemplatesMu sync.Mutex

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a sync.Mutex forces an exclusive lock on every call to /v1/models (via loadCodexClientModelTemplatesSnapshot), even when the templates are already loaded and the revision has not changed. Under high concurrency, this can lead to lock contention. Consider using a sync.RWMutex to allow concurrent read access for the common case.

Suggested change
codexClientModelTemplatesMu sync.Mutex
codexClientModelTemplatesMu sync.RWMutex

Comment on lines +142 to +147
func loadCodexClientModelTemplatesSnapshot(raw []byte, revision uint64) (map[string]map[string]any, map[string]any, error) {
codexClientModelTemplatesMu.Lock()
defer codexClientModelTemplatesMu.Unlock()
if codexClientModelTemplatesLoaded && codexClientModelTemplatesRevision == revision {
return codexClientModelTemplates, codexClientDefaultTemplate, codexClientModelTemplatesErr
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

With codexClientModelTemplatesMu upgraded to a sync.RWMutex, we can implement a double-checked locking pattern. This allows concurrent requests to read the cached templates using a read lock (RLock), completely avoiding exclusive lock contention when the revision has not changed.

func loadCodexClientModelTemplatesSnapshot(raw []byte, revision uint64) (map[string]map[string]any, map[string]any, error) {
	codexClientModelTemplatesMu.RLock()
	if codexClientModelTemplatesLoaded && codexClientModelTemplatesRevision == revision {
		templates, defaultTemplate, err := codexClientModelTemplates, codexClientDefaultTemplate, codexClientModelTemplatesErr
		codexClientModelTemplatesMu.RUnlock()
		return templates, defaultTemplate, err
	}
	codexClientModelTemplatesMu.RUnlock()

	codexClientModelTemplatesMu.Lock()
	defer codexClientModelTemplatesMu.Unlock()
	if codexClientModelTemplatesLoaded && codexClientModelTemplatesRevision == revision {
		return codexClientModelTemplates, codexClientDefaultTemplate, codexClientModelTemplatesErr
	}

@sususu98 sususu98 merged commit 4fe2c60 into router-for-me:dev Jul 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant