Skip to content

feat(github-copilot): dynamic GitHub Copilot model discovery via API with github-models integration#1151

Open
Meetpatel006 wants to merge 7 commits into
Gitlawb:mainfrom
Meetpatel006:feat/github-models-dynamic-discovery
Open

feat(github-copilot): dynamic GitHub Copilot model discovery via API with github-models integration#1151
Meetpatel006 wants to merge 7 commits into
Gitlawb:mainfrom
Meetpatel006:feat/github-models-dynamic-discovery

Conversation

@Meetpatel006
Copy link
Copy Markdown
Contributor

Summary

  • Replaced hardcoded Copilot model list with dynamic model fetching from the GitHub Copilot API (api.githubcopilot.com/models)
  • Removed deprecated copilotModels.ts and all hardcoded model references
  • Added new github-models discovery kind in the integration discovery system for the GitHub Copilot API (with required IDE auth headers)
  • Fixed startup banner to show user's saved model (reads settings.model before env var)
  • Fixed model picker to pre-select the user's currently saved model instead of always showing "Default"
  • Changed default model from github:copilot → gpt-4o across all provider profiles, onboarding, and fallback paths
  • Normalized legacy github:copilot alias to gpt-4o at the settings retrieval level

Screenshot

  • Before
export-1778679132329.mp4
  • After
export-1778678732601.mp4

Impact

  • User-facing: Model picker now shows real-time models from your Copilot subscription (including GPT-5, Claude, Gemini, Grok models) instead of a stale hardcoded list. Startup banner displays your saved model correctly across restarts. No more github:copilot showing in UI.
  • Developer/maintainer: Removed 383 lines of hardcoded model data (copilotModels.ts). Discovery is now API-driven — new Copilot models appear automatically without code changes. Added github-models to ModelDiscoveryKind type for future use.

Testing

  • bun run build
  • bun run smoke
  • bun test src/integrations/index.test.ts

Notes

  • Provider/model path tested: GitHub Copilot (CLAUDE_CODE_USE_GITHUB=1)
  • Follow-up work: The github-models discovery currently strips date-stamped model versions (e.g., gpt-4o-2024-11-20 → GPT-4o). If users need access to specific dated versions, the label formatting can be adjusted. The Copilot API's Editor-Version header may need periodic updates.

Copilot AI review requested due to automatic review settings May 13, 2026 13:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Replaces the hardcoded GitHub Copilot model registry with dynamic discovery via the api.githubcopilot.com/models endpoint, wired into the existing integration discovery framework as a new github-models kind. Defaults are migrated from the legacy github:copilot alias to gpt-4o across provider profiles, onboarding, and fallback code paths, with a normalization step to translate persisted legacy values. The startup banner is updated to read the user's persisted model from settings and render its public display name.

Changes:

  • Delete copilotModels.ts and remove the static Copilot model dropdown; the model picker for GitHub now relies on the dynamic discovery catalog.
  • Add a new 'github-models' ModelDiscoveryKind with hardcoded Copilot/VS Code editor headers, a 5s timeout, in-place label formatting, and silent failure handling.
  • Replace 'github:copilot' defaults with 'gpt-4o' in launch env, provider config, profile builders, onboarding messaging, and provider summaries; normalize the legacy alias at settings read time.
  • Gate reasoning_effort emission in the OpenAI shim on modelSupportsEffort(resolvedModel).
  • Pre-select the user's saved main-loop model in the model picker via getDefaultMainLoopModelSetting() fallback.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/utils/providerProfile.ts Default model in github profile env builder switched to gpt-4o.
src/utils/model/modelOptions.ts Removed Copilot static option list; GitHub provider now exposes only the Default option.
src/utils/model/model.ts Default fallbacks switched to gpt-4o; legacy alias github:copilot normalized in getUserSpecifiedModelSetting.
src/utils/model/copilotModels.ts File deleted (was a 383-line static registry).
src/services/api/providerConfig.ts GitHub fallback for resolved model changed to gpt-4o.
src/services/api/openaiShim.ts reasoning_effort only emitted when the resolved model supports it.
src/integrations/gateways/github.ts Catalog switched from static to dynamic using the new github-models kind.
src/integrations/discoveryService.ts Implements the github-models discovery branch (auth header, version stamping, label formatting, dedup/filter, silent fail).
src/integrations/descriptors.ts Adds 'github-models' to the ModelDiscoveryKind union.
src/components/StartupScreen.ts Reads getInitialSettings() for the GitHub branch and returns a display name instead of the raw model ID.
src/components/ProviderManager.tsx Updates GitHub provider default model constant to gpt-4o.
src/commands/provider/provider.tsx Updates provider summary fallback model to gpt-4o.
src/commands/onboard-github/onboard-github.tsx Updates default and user-facing instruction string to gpt-4o.
src/commands/model/model.tsx Falls back to getDefaultMainLoopModelSetting() when mainLoopModel is unset so the picker pre-selects the saved model.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +96 to +101
const settings = getInitialSettings() || {}
const model = modelOverride || settings.model || process.env.OPENAI_MODEL || 'gpt-4o'
const displayName = model === 'github:copilot' ? 'GPT-4o' : (getPublicModelDisplayName(model) || model)
const baseUrl =
process.env.OPENAI_BASE_URL || 'https://api.githubcopilot.com'
return { name: 'GitHub Copilot', model, baseUrl, isLocal: false }
return { name: 'GitHub Copilot', model: displayName, baseUrl, isLocal: false }
Comment on lines +14 to +15
import { getInitialSettings } from '../utils/settings/settings.js'
import { getPublicModelDisplayName, parseUserSpecifiedModel } from '../utils/model/model.js'
Comment on lines +248 to +255
const apiKey = getRouteDiscoveryApiKey(routeId, options)
const headers: Record<string, string> = {
Authorization: `Bearer ${apiKey}`,
'Editor-Version': 'vscode/1.96.0',
'Editor-Plugin-Version': 'copilot/1.250.0',
'User-Agent': 'GitHubCopilot/1.250.0',
Accept: 'application/json',
}
Comment on lines +263 to +291
if (base.startsWith('gpt-')) {
const rest = base.slice(4)
const named = rest
.replace(/^4o-mini/, '4o Mini')
.replace(/^4o/, '4o')
.replace(/^4-o/, '4o')
.replace(/^4\.1/, '4.1')
.replace(/^4/, '4')
.replace(/^3\.5-turbo/, '3.5 Turbo')
.replace(/^5\.5-mini/, '5.5 Mini')
.replace(/^5\.5/, '5.5')
.replace(/^5\.4-mini/, '5.4 Mini')
.replace(/^5\.4/, '5.4')
.replace(/^5\.3-codex/, '5.3 Codex')
.replace(/^5\.2-codex/, '5.2 Codex')
.replace(/^5\.2/, '5.2')
.replace(/^5\.1-codex/, '5.1 Codex')
.replace(/^5-mini/, '5 Mini')
.replace(/^5/, '5')
return `GPT-${named}`
}
if (base.startsWith('gemini-')) return base.replace('gemini-', 'Gemini ')
if (base.startsWith('grok-')) {
const v = base.replace('grok-', '')
if (v === 'code-fast-1') return 'Grok Code Fast 1'
return `Grok ${v}`
}
return rawId
}
Comment on lines +249 to +318
const headers: Record<string, string> = {
Authorization: `Bearer ${apiKey}`,
'Editor-Version': 'vscode/1.96.0',
'Editor-Plugin-Version': 'copilot/1.250.0',
'User-Agent': 'GitHubCopilot/1.250.0',
Accept: 'application/json',
}

function formatModelLabel(rawId: string): string {
const base = rawId.replace(/-\d{4}-\d{2}-\d{2}$/, '').replace(/-\d{4}$/, '')
if (base.startsWith('claude-')) {
const parts = base.replace('claude-', '').split('-')
if (parts.length >= 2) return `Claude ${parts[0].charAt(0).toUpperCase() + parts[0].slice(1)} ${parts.slice(1).join('.')}`
}
if (base.startsWith('gpt-')) {
const rest = base.slice(4)
const named = rest
.replace(/^4o-mini/, '4o Mini')
.replace(/^4o/, '4o')
.replace(/^4-o/, '4o')
.replace(/^4\.1/, '4.1')
.replace(/^4/, '4')
.replace(/^3\.5-turbo/, '3.5 Turbo')
.replace(/^5\.5-mini/, '5.5 Mini')
.replace(/^5\.5/, '5.5')
.replace(/^5\.4-mini/, '5.4 Mini')
.replace(/^5\.4/, '5.4')
.replace(/^5\.3-codex/, '5.3 Codex')
.replace(/^5\.2-codex/, '5.2 Codex')
.replace(/^5\.2/, '5.2')
.replace(/^5\.1-codex/, '5.1 Codex')
.replace(/^5-mini/, '5 Mini')
.replace(/^5/, '5')
return `GPT-${named}`
}
if (base.startsWith('gemini-')) return base.replace('gemini-', 'Gemini ')
if (base.startsWith('grok-')) {
const v = base.replace('grok-', '')
if (v === 'code-fast-1') return 'Grok Code Fast 1'
return `Grok ${v}`
}
return rawId
}

try {
const response = await fetch('https://api.githubcopilot.com/models', {
headers,
signal: AbortSignal.timeout(5000),
})
if (!response.ok) return null

const body = (await response.json()) as { data?: Array<{ id?: string }> }
if (!body.data || !Array.isArray(body.data)) return null

const seen = new Set<string>()
const models = body.data
.map(item => {
const id = item.id?.trim()
if (!id) return null
if (id.startsWith('accounts/') || id.startsWith('oswe-') || id.includes('text-embedding')) return null
if (seen.has(id)) return null
seen.add(id)
return { id, apiName: id, label: formatModelLabel(id) } as ModelCatalogEntry
})
.filter((m): m is ModelCatalogEntry => m !== null)

return models.length > 0 ? models : null
} catch {
return null
}
Comment on lines +293 to +318
try {
const response = await fetch('https://api.githubcopilot.com/models', {
headers,
signal: AbortSignal.timeout(5000),
})
if (!response.ok) return null

const body = (await response.json()) as { data?: Array<{ id?: string }> }
if (!body.data || !Array.isArray(body.data)) return null

const seen = new Set<string>()
const models = body.data
.map(item => {
const id = item.id?.trim()
if (!id) return null
if (id.startsWith('accounts/') || id.startsWith('oswe-') || id.includes('text-embedding')) return null
if (seen.has(id)) return null
seen.add(id)
return { id, apiName: id, label: formatModelLabel(id) } as ModelCatalogEntry
})
.filter((m): m is ModelCatalogEntry => m !== null)

return models.length > 0 ? models : null
} catch {
return null
}
Comment on lines +46 to +50
source: 'dynamic',
discovery: { kind: 'github-models', requiresAuth: true },
discoveryCacheTtl: '1h',
discoveryRefreshMode: 'background-if-stale',
allowManualRefresh: true,
Comment thread src/utils/model/model.ts
Comment on lines +139 to +142
// Normalize legacy GitHub Copilot alias to concrete model ID
if (specifiedModel === 'github:copilot') {
specifiedModel = 'gpt-4o'
}
Comment on lines 391 to 394
function getModelOptionsBase(fastMode = false): ModelOption[] {
if (getAPIProvider() === 'github') {
return [getDefaultOptionForUser(fastMode), ...getCopilotModelOptions()]
return [getDefaultOptionForUser(fastMode)]
}
Accept: 'application/json',
}

function formatModelLabel(rawId: string): string {
Copilot AI review requested due to automatic review settings May 13, 2026 14:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (6)

src/integrations/discoveryService.ts:283

  • formatModelLabel contains many redundant/no-op replacements that obscure intent. For example, .replace(/^4o/, '4o'), .replace(/^4/, '4'), and .replace(/^4o/, '4o') after .replace(/^4o-mini/, '4o Mini') (the latter already replaced the prefix so the ^4o rule cannot meaningfully change anything in the dash-mini case; for plain 4o it’s a no-op). Several other rules (/^5\.5/, /^5\.4/, etc.) similarly map a string to itself. This makes the formatting table look exhaustive but the only effective transformations are the ones that change spacing (e.g. 4o-mini4o Mini, 5.5-mini5.5 Mini, 5.3-codex5.3 Codex, …). Consider removing the no-op replacements and documenting that the goal is purely to insert spaces and capitalize sub-tokens, otherwise future maintainers will think these lines do something they don’t.
        if (base.startsWith('gpt-')) {
          const rest = base.slice(4)
          const named = rest
            .replace(/^4o-mini/, '4o Mini')
            .replace(/^4o/, '4o')
            .replace(/^4-o/, '4o')
            .replace(/^4\.1/, '4.1')
            .replace(/^4/, '4')
            .replace(/^3\.5-turbo/, '3.5 Turbo')
            .replace(/^5\.5-mini/, '5.5 Mini')
            .replace(/^5\.5/, '5.5')
            .replace(/^5\.4-mini/, '5.4 Mini')
            .replace(/^5\.4/, '5.4')
            .replace(/^5\.3-codex/, '5.3 Codex')
            .replace(/^5\.2-codex/, '5.2 Codex')
            .replace(/^5\.2/, '5.2')
            .replace(/^5\.1-codex/, '5.1 Codex')
            .replace(/^5-mini/, '5 Mini')
            .replace(/^5/, '5')
          return `GPT-${named}`
        }

src/integrations/discoveryService.ts:262

  • Label formatting for Claude models is fragile. Stripping a trailing -YYYY-MM-DD and then splitting on - works for claude-opus-4-6 (→ Claude Opus 4.6) but produces awkward results for ids like claude-3-5-sonnet-20241022, which becomes Claude 3 5.sonnet, or claude-3-7-sonnet-thinkingClaude 3 7.sonnet.thinking. Since the GitHub Copilot catalog historically includes such IDs, the picker will show confusing labels. Consider a more structured parser (recognize the family token explicitly) or fall back to a title-cased raw id when the pattern doesn’t match the expected claude-<family>-<major>-<minor> shape.
        if (base.startsWith('claude-')) {
          const parts = base.replace('claude-', '').split('-')
          if (parts.length >= 2) return `Claude ${parts[0].charAt(0).toUpperCase() + parts[0].slice(1)} ${parts.slice(1).join('.')}`
        }

src/integrations/discoveryService.ts:315

  • The model id/apiName is preserved verbatim, but formatModelLabel strips dated suffixes (e.g. gpt-4o-2024-08-06 and gpt-4o-2024-11-20 both label as GPT-4o). Deduplication is performed on id, not on label, so the picker can end up with multiple visually identical entries that resolve to different concrete models. Users will not be able to distinguish them. Either include the date suffix in the label when there is a collision, or dedupe by label and pick a canonical id.
        const seen = new Set<string>()
        const models = body.data
          .map(item => {
            const id = item.id?.trim()
            if (!id) return null
            if (id.startsWith('accounts/') || id.startsWith('oswe-') || id.includes('text-embedding')) return null
            if (seen.has(id)) return null
            seen.add(id)
            return { id, apiName: id, label: formatModelLabel(id) } as ModelCatalogEntry
          })
          .filter((m): m is ModelCatalogEntry => m !== null)

src/integrations/discoveryService.ts:322

  • The Editor-Version, Editor-Plugin-Version, and User-Agent headers are hardcoded to specific VS Code/Copilot versions. The PR description acknowledges this header "may need periodic updates", but if GitHub's API tightens validation, every Claude Code user will silently lose Copilot model discovery (the function returns null on !response.ok, which then masks the issue). At minimum, log a warning when the response is non-OK (status, body) so this is diagnosable in production, and consider centralizing this version string so future bumps touch one place. Also, no telemetry/logging on the catch handler means any network/parse error is also silently swallowed.
      const headers: Record<string, string> = {
        Authorization: `Bearer ${apiKey}`,
        'Editor-Version': 'vscode/1.96.0',
        'Editor-Plugin-Version': 'copilot/1.250.0',
        'User-Agent': 'GitHubCopilot/1.250.0',
        Accept: 'application/json',
      }

      function formatModelLabel(rawId: string): string {
        const base = rawId.replace(/-\d{4}-\d{2}-\d{2}$/, '').replace(/-\d{4}$/, '')
        if (base.startsWith('claude-')) {
          const parts = base.replace('claude-', '').split('-')
          if (parts.length >= 2) return `Claude ${parts[0].charAt(0).toUpperCase() + parts[0].slice(1)} ${parts.slice(1).join('.')}`
        }
        if (base.startsWith('gpt-')) {
          const rest = base.slice(4)
          const named = rest
            .replace(/^4o-mini/, '4o Mini')
            .replace(/^4o/, '4o')
            .replace(/^4-o/, '4o')
            .replace(/^4\.1/, '4.1')
            .replace(/^4/, '4')
            .replace(/^3\.5-turbo/, '3.5 Turbo')
            .replace(/^5\.5-mini/, '5.5 Mini')
            .replace(/^5\.5/, '5.5')
            .replace(/^5\.4-mini/, '5.4 Mini')
            .replace(/^5\.4/, '5.4')
            .replace(/^5\.3-codex/, '5.3 Codex')
            .replace(/^5\.2-codex/, '5.2 Codex')
            .replace(/^5\.2/, '5.2')
            .replace(/^5\.1-codex/, '5.1 Codex')
            .replace(/^5-mini/, '5 Mini')
            .replace(/^5/, '5')
          return `GPT-${named}`
        }
        if (base.startsWith('gemini-')) return base.replace('gemini-', 'Gemini ')
        if (base.startsWith('grok-')) {
          const v = base.replace('grok-', '')
          if (v === 'code-fast-1') return 'Grok Code Fast 1'
          return `Grok ${v}`
        }
        return rawId
      }

      const controller = new AbortController()
      const timeoutId = setTimeout(() => controller.abort(), 5000)
      try {
        const response = await fetch('https://api.githubcopilot.com/models', {
          headers,
          signal: controller.signal,
        })
        if (!response.ok) return null

        const body = (await response.json()) as { data?: Array<{ id?: string }> }
        if (!body.data || !Array.isArray(body.data)) return null

        const seen = new Set<string>()
        const models = body.data
          .map(item => {
            const id = item.id?.trim()
            if (!id) return null
            if (id.startsWith('accounts/') || id.startsWith('oswe-') || id.includes('text-embedding')) return null
            if (seen.has(id)) return null
            seen.add(id)
            return { id, apiName: id, label: formatModelLabel(id) } as ModelCatalogEntry
          })
          .filter((m): m is ModelCatalogEntry => m !== null)

        return models.length > 0 ? models : null
      } catch {
        return null
      } finally {
        clearTimeout(timeoutId)
      }

src/integrations/discoveryService.ts:291

  • formatModelLabel is declared inside the switch case block using a function declaration. Function declarations inside blocks have implementation-defined hoisting behavior in strict mode; while it works in modern V8/Node, prefer either lifting this helper to module scope (it's pure) or using const formatModelLabel = (rawId: string) => { ... }. Hoisting it also lets it be unit-tested directly, which is desirable given its complexity.
      function formatModelLabel(rawId: string): string {
        const base = rawId.replace(/-\d{4}-\d{2}-\d{2}$/, '').replace(/-\d{4}$/, '')
        if (base.startsWith('claude-')) {
          const parts = base.replace('claude-', '').split('-')
          if (parts.length >= 2) return `Claude ${parts[0].charAt(0).toUpperCase() + parts[0].slice(1)} ${parts.slice(1).join('.')}`
        }
        if (base.startsWith('gpt-')) {
          const rest = base.slice(4)
          const named = rest
            .replace(/^4o-mini/, '4o Mini')
            .replace(/^4o/, '4o')
            .replace(/^4-o/, '4o')
            .replace(/^4\.1/, '4.1')
            .replace(/^4/, '4')
            .replace(/^3\.5-turbo/, '3.5 Turbo')
            .replace(/^5\.5-mini/, '5.5 Mini')
            .replace(/^5\.5/, '5.5')
            .replace(/^5\.4-mini/, '5.4 Mini')
            .replace(/^5\.4/, '5.4')
            .replace(/^5\.3-codex/, '5.3 Codex')
            .replace(/^5\.2-codex/, '5.2 Codex')
            .replace(/^5\.2/, '5.2')
            .replace(/^5\.1-codex/, '5.1 Codex')
            .replace(/^5-mini/, '5 Mini')
            .replace(/^5/, '5')
          return `GPT-${named}`
        }
        if (base.startsWith('gemini-')) return base.replace('gemini-', 'Gemini ')
        if (base.startsWith('grok-')) {
          const v = base.replace('grok-', '')
          if (v === 'code-fast-1') return 'Grok Code Fast 1'
          return `Grok ${v}`
        }
        return rawId
      }

src/integrations/discoveryService.ts:310

  • The filter id.startsWith('accounts/') || id.startsWith('oswe-') || id.includes('text-embedding') is an ad-hoc denylist for non-chat models. Other irrelevant model classes (e.g. *-search, image generation, moderation models) may be added to the Copilot catalog and would pass through here, appearing in the model picker for chat use. A more robust approach is to inspect the capabilities/type/object fields returned by the API (the Copilot /models response includes per-model capability metadata) and accept only chat-completion-capable models.
            if (id.startsWith('accounts/') || id.startsWith('oswe-') || id.includes('text-embedding')) return null

Comment on lines +248 to +255
const apiKey = getRouteDiscoveryApiKey(routeId, options)
const headers: Record<string, string> = {
Authorization: `Bearer ${apiKey}`,
'Editor-Version': 'vscode/1.96.0',
'Editor-Plugin-Version': 'copilot/1.250.0',
'User-Agent': 'GitHubCopilot/1.250.0',
Accept: 'application/json',
}
Comment thread src/utils/model/model.ts
Comment on lines +139 to +142
// Normalize legacy GitHub Copilot alias to concrete model ID
if (specifiedModel === 'github:copilot') {
specifiedModel = 'gpt-4o'
}
function getModelOptionsBase(fastMode = false): ModelOption[] {
if (getAPIProvider() === 'github') {
return [getDefaultOptionForUser(fastMode), ...getCopilotModelOptions()]
return [getDefaultOptionForUser(fastMode)]
onDone: (result?: string, options?: { display?: CommandResultDisplay }) => void
}) {
const mainLoopModel = useAppState((s: AppState) => s.mainLoopModel)
const mainLoopModel = useAppState((s: AppState) => s.mainLoopModel) ?? getDefaultMainLoopModelSetting()
Comment on lines +96 to +101
const settings = getInitialSettings() || {}
const model = modelOverride || settings.model || process.env.OPENAI_MODEL || 'gpt-4o'
const displayName = model === 'github:copilot' ? 'GPT-4o' : (getPublicModelDisplayName(model) || model)
const baseUrl =
process.env.OPENAI_BASE_URL || 'https://api.githubcopilot.com'
return { name: 'GitHub Copilot', model, baseUrl, isLocal: false }
return { name: 'GitHub Copilot', model: displayName, baseUrl, isLocal: false }
Copy link
Copy Markdown
Collaborator

@jatmn jatmn left a comment

Choose a reason for hiding this comment

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

Findings

  • [P2] Keep curated GitHub models as a hybrid fallback
    src/integrations/gateways/github.ts:45
    The integration docs already support the catalog behavior this PR is trying to build out: docs/integrations/how-to/add-gateway.md documents hybrid catalogs as curated entries plus discovered models, and docs/integrations/reference-samples.md calls the hosted hybrid gateway shape the recommended pattern for discovery-heavy gateways. The implementation already follows that contract too: discoverModelsForRoute() merges static entries with cached/network discovery results and falls back to static entries when discovery cannot run. I also could not find official GitHub documentation for https://api.githubcopilot.com/models or for requiring VS Code/Copilot Editor-Version headers as a supported integration contract. The documented GitHub Models catalog endpoint is https://models.github.ai/catalog/models with normal GitHub REST headers, while the Copilot docs publish supported model lists and plan/client availability rather than a Copilot model-discovery API. So the new github-models discovery kind is not justified by the public docs as written, and this PR goes well beyond the existing architecture by changing GitHub from a static catalog to a purely dynamic one with no models fallback. If nonessential traffic is disabled, the user is offline, auth is unavailable, or the first refresh fails before a cache exists, discoverModelsForRoute() returns only staticEntries, which is now an empty array. That means /model regresses from exposing the known Copilot models to effectively only the current/default model until network discovery succeeds. Please narrow the PR to a documented/proven gap: keep the existing curated GitHub/Copilot models as a hybrid catalog, remove the undocumented dynamic-only replacement, and only add provider-specific discovery code if there is a documented API contract or a demonstrated reason the existing descriptor/generic discovery hooks cannot cover it.

  • [P2] Fix the startup banner regression and failing CI
    src/components/StartupScreen.ts:98
    The PR says the startup banner now shows the saved/current GitHub model display name, but CI is failing on detectProvider — modelOverride from --model flag > modelOverride works for GitHub provider: detectProvider('gpt-4o') still returns "gpt-4o" instead of "GPT-4o". This is in the exact path changed by the PR, so the test update is currently proving the implementation does not do what it claims. Please fix the display-name resolution here and keep the test passing before merge.

Recommendation

Request changes. The shared integration infrastructure already supports dynamic discovery, caching, refresh, and hybrid fallback. The documented hybrid approach appears to provide the expected GitHub model-picker behavior with much less churn, and the new Copilot-specific discovery endpoint/headers are not backed by the official docs I could find. Please remove the redundant overreach around replacing curated model support with a dynamic-only catalog and keep the existing model metadata as the fallback. The currently failing startup test also needs to be fixed.

@Vasanthdev2004
Copy link
Copy Markdown
Collaborator

Blockers

  1. Should use hybrid catalog instead of dynamic-only — The PR replaces the curated GitHub/Copilot models with a purely dynamic catalog. If discovery fails, users see only the current/default model. The existing architecture supports hybrid catalogs (curated entries + discovered models) which is the recommended pattern.

  2. Startup banner test failing — CI is failing on detectProvider — modelOverride from --model flag > modelOverride works for GitHub provider. The display-name resolution doesn't work as claimed.

Non-Blocking

  • Ad-hoc denylist for non-chat models — should inspect capabilities/type fields instead.
  • Undocumented Copilot API endpoint — api.githubcopilot.com/models is not in official GitHub docs.

Looks Good

  • Removes 383 lines of hardcoded model data
  • Dynamic discovery would show real-time models from Copilot subscription
  • Fixes startup banner to show saved model
  • Fixes model picker to pre-select saved model

Verdict: Changes Requested — use hybrid catalog, fix failing test.

@gnanam1990
Copy link
Copy Markdown
Collaborator

Confirmed @jatmn's findings, and flagging this against our provider/fork guidelines since it sits right on them. Three concerns, all verified in the diff:

  1. Undocumented endpoint + spoofed client identity. The new github-models discovery kind issues fetch('https://api.githubcopilot.com/models') with a hardcoded Editor-Version: vscode/1.96.0 header. As @jatmn noted, there's no public GitHub documentation for that Copilot model-discovery endpoint or for an Editor-Version requirement — the documented catalog is https://models.github.ai/catalog/models with normal GitHub REST headers. Presenting a spoofed VS Code editor identity to an undocumented endpoint on an unproven "Copilot requires it" basis is exactly the kind of impersonation we're moving the fork away from. This needs either authoritative documentation that this contract is supported, or a switch to the documented GitHub Models catalog endpoint.
  2. Removal of the static fallback. Flipping GitHub from a curated/hybrid catalog to source: 'dynamic' with no models fallback means that when non-essential traffic is disabled, the user is offline, or auth is unavailable, the GitHub provider has no models at all. The existing hybrid pattern (curated entries + discovered) is the documented and safer shape here.
  3. Net effect is a large architectural change (static → purely dynamic) beyond the stated scope.

Routing through the documented endpoint with a hybrid catalog that keeps the curated entries as fallback would address all three. Appreciate the effort here — happy to re-review once it's realigned.

@Meetpatel006
Copy link
Copy Markdown
Contributor Author

Findings

  • [P2] Keep curated GitHub models as a hybrid fallback
    src/integrations/gateways/github.ts:45
    The integration docs already support the catalog behavior this PR is trying to build out: docs/integrations/how-to/add-gateway.md documents hybrid catalogs as curated entries plus discovered models, and docs/integrations/reference-samples.md calls the hosted hybrid gateway shape the recommended pattern for discovery-heavy gateways. The implementation already follows that contract too: discoverModelsForRoute() merges static entries with cached/network discovery results and falls back to static entries when discovery cannot run. I also could not find official GitHub documentation for https://api.githubcopilot.com/models or for requiring VS Code/Copilot Editor-Version headers as a supported integration contract. The documented GitHub Models catalog endpoint is https://models.github.ai/catalog/models with normal GitHub REST headers, while the Copilot docs publish supported model lists and plan/client availability rather than a Copilot model-discovery API. So the new github-models discovery kind is not justified by the public docs as written, and this PR goes well beyond the existing architecture by changing GitHub from a static catalog to a purely dynamic one with no models fallback. If nonessential traffic is disabled, the user is offline, auth is unavailable, or the first refresh fails before a cache exists, discoverModelsForRoute() returns only staticEntries, which is now an empty array. That means /model regresses from exposing the known Copilot models to effectively only the current/default model until network discovery succeeds. Please narrow the PR to a documented/proven gap: keep the existing curated GitHub/Copilot models as a hybrid catalog, remove the undocumented dynamic-only replacement, and only add provider-specific discovery code if there is a documented API contract or a demonstrated reason the existing descriptor/generic discovery hooks cannot cover it.

  • [P2] Fix the startup banner regression and failing CI
    src/components/StartupScreen.ts:98
    The PR says the startup banner now shows the saved/current GitHub model display name, but CI is failing on detectProvider — modelOverride from --model flag > modelOverride works for GitHub provider: detectProvider('gpt-4o') still returns "gpt-4o" instead of "GPT-4o". This is in the exact path changed by the PR, so the test update is currently proving the implementation does not do what it claims. Please fix the display-name resolution here and keep the test passing before merge.

Recommendation

Request changes. The shared integration infrastructure already supports dynamic discovery, caching, refresh, and hybrid fallback. The documented hybrid approach appears to provide the expected GitHub model-picker behavior with much less churn, and the new Copilot-specific discovery endpoint/headers are not backed by the official docs I could find. Please remove the redundant overreach around replacing curated model support with a dynamic-only catalog and keep the existing model metadata as the fallback. The currently failing startup test also needs to be fixed.

On it 

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.

5 participants