From e6f062d114b0977ff2e5e707d470c309ae3a481a Mon Sep 17 00:00:00 2001 From: hgaol Date: Thu, 3 Sep 2026 14:52:47 +0000 Subject: [PATCH 1/2] fix(copilot): expose only account-available models GitHub's model policy has three historical states: enabled, disabled, and unconfigured. Maka cannot accept model policy terms, so exclude every present policy that is not enabled while continuing to admit policy-free current responses. Treat the filtered Copilot /models response as an authoritative account catalog during persistence. The first fetch replaces bootstrap fallback IDs; later refreshes remove withdrawn IDs without automatically opting the user into newly introduced models. Cover the mixed provider payload, authoritative reconciliation, and real Runtime Policy model-fetch commit. Generated-by: gpt-5.6-sol --- .../src/__tests__/llm-connections.test.ts | 32 +++++++++++++++++ packages/core/src/llm-connections.ts | 21 +++++++++++ .../__tests__/provider-contract-overrides.ts | 10 +++++- packages/runtime/src/model-fetcher.ts | 15 ++++++-- .../__tests__/runtime-policy-stores.test.ts | 36 +++++++++++++++++++ .../connection-catalog-document.ts | 4 +++ 6 files changed, 115 insertions(+), 3 deletions(-) diff --git a/packages/core/src/__tests__/llm-connections.test.ts b/packages/core/src/__tests__/llm-connections.test.ts index 97cce94cb9..d18dc3cb0d 100644 --- a/packages/core/src/__tests__/llm-connections.test.ts +++ b/packages/core/src/__tests__/llm-connections.test.ts @@ -126,6 +126,38 @@ test('a fetch never deletes a choice the user made', () => { ); }); +test('an authoritative account catalog removes unavailable bootstrap and stale models', () => { + assert.deepEqual( + reconcileConnectionAfterModelFetch( + { + defaultModel: 'fallback-unavailable', + enabledModelIds: ['fallback-unavailable', 'account-available'], + hasModelInventory: false, + }, + [{ id: 'account-available' }, { id: 'newly-available' }], + { authoritative: true }, + ), + { + defaultModel: 'account-available', + enabledModelIds: ['account-available', 'newly-available'], + }, + ); + // Once an account inventory exists, a refresh removes withdrawn selections + // without automatically opting the user into newly introduced models. + assert.deepEqual( + reconcileConnectionAfterModelFetch( + { + defaultModel: 'account-available', + enabledModelIds: ['account-available', 'withdrawn'], + hasModelInventory: true, + }, + [{ id: 'account-available' }, { id: 'newly-available' }], + { authoritative: true }, + ), + { defaultModel: 'account-available', enabledModelIds: ['account-available'] }, + ); +}); + test('model reconciliation never invents a default the user cleared', () => { // Unchecking the default leaves a legitimate {no default, some enabled} // state. Repair had nothing to repair here, so it reached for "the first diff --git a/packages/core/src/llm-connections.ts b/packages/core/src/llm-connections.ts index b17ffff8ed..33fc7896d4 100644 --- a/packages/core/src/llm-connections.ts +++ b/packages/core/src/llm-connections.ts @@ -458,6 +458,11 @@ export function reconcileConnectionAfterModelFetch( * caller that knows the provider's naming supplies the table. */ readonly aliases?: Readonly>; + /** + * The provider guarantees this is the account's complete usable catalog. + * Missing ids are therefore unavailable, unlike ordinary partial snapshots. + */ + readonly authoritative?: boolean; }, ): { defaultModel: string; @@ -490,6 +495,22 @@ export function reconcileConnectionAfterModelFetch( ), ), ]; + if (options?.authoritative) { + // The first account-scoped fetch replaces the provider fallback guess: no + // user chose those bootstrap ids, and every usable model should be offered. + // Later refreshes preserve explicit user choices only while they remain in + // the account catalog; newly introduced models stay opt-in. + const enabledModelIds = connection.hasModelInventory + ? previousEnabled.filter((id) => live.has(id)) + : liveIds; + if (enabledModelIds.length === 0 && previousEnabled.length > 0 && liveIds.length > 0) { + enabledModelIds.push(liveIds[0]!); + } + const defaultModel = enabledModelIds.includes(previousDefault) + ? previousDefault + : (enabledModelIds[0] ?? ''); + return { defaultModel, enabledModelIds }; + } // Seed a first choice only for a connection that has never had a list to // pick from: four providers ship no `fallbackModels`, so for them discovery // is the only place a first default can come from. diff --git a/packages/runtime/src/__tests__/provider-contract-overrides.ts b/packages/runtime/src/__tests__/provider-contract-overrides.ts index ca7c049a2f..a3b1af6310 100644 --- a/packages/runtime/src/__tests__/provider-contract-overrides.ts +++ b/packages/runtime/src/__tests__/provider-contract-overrides.ts @@ -348,13 +348,21 @@ async function runGitHubCopilotDiscovery(): Promise { assert.equal(request.headers['x-github-api-version'], '2026-06-01'); respondJson(response, 200, { data: [ - copilotModel('gpt-5.4', ['/responses']), + { + ...copilotModel('gpt-5.4', ['/responses']), + // Current GitHub clients also accept models with no policy gate. + policy: undefined, + }, copilotModel('claude-sonnet-4.6', ['/v1/messages']), copilotModel('gemini-3.1-pro-preview', ['/chat/completions']), { ...copilotModel('disabled-by-policy', ['/chat/completions']), policy: { state: 'disabled' }, }, + { + ...copilotModel('policy-not-accepted', ['/chat/completions']), + policy: { state: 'unconfigured' }, + }, { ...copilotModel('hidden-from-picker', ['/chat/completions']), model_picker_enabled: false, diff --git a/packages/runtime/src/model-fetcher.ts b/packages/runtime/src/model-fetcher.ts index e886114c3e..f6db59b150 100644 --- a/packages/runtime/src/model-fetcher.ts +++ b/packages/runtime/src/model-fetcher.ts @@ -114,7 +114,7 @@ type RawGitHubCopilotModel = { name?: string; model_picker_enabled?: boolean; supported_endpoints?: string[]; - policy?: { state?: string }; + policy?: unknown; capabilities?: { limits?: { max_context_window_tokens?: number; @@ -523,7 +523,12 @@ function toGitHubCopilotModelInfo(model: RawGitHubCopilotModel): ModelInfo[] { typeof model.id !== 'string' || !model.id || model.model_picker_enabled !== true || - model.policy?.state === 'disabled' || + // GitHub historically returned enabled/disabled/unconfigured policy gates. + // A policy-free model needs no acknowledgement; when the gate is present, + // Maka can use the model only after another client has enabled it. Maka has + // no policy-acceptance flow, so fail closed over unconfigured and unknown + // states instead of advertising a model that inference will reject. + !isGitHubCopilotModelPolicyEnabled(model.policy) || model.capabilities?.supports?.tool_calls !== true ) return []; @@ -572,6 +577,12 @@ function toGitHubCopilotModelInfo(model: RawGitHubCopilotModel): ModelInfo[] { ]; } +function isGitHubCopilotModelPolicyEnabled(policy: unknown): boolean { + if (policy === undefined) return true; + if (!policy || typeof policy !== 'object' || Array.isArray(policy)) return false; + return (policy as Record).state === 'enabled'; +} + async function fetchCohereModels( baseUrl: string, apiKey: string, diff --git a/packages/storage/src/__tests__/runtime-policy-stores.test.ts b/packages/storage/src/__tests__/runtime-policy-stores.test.ts index dbc20f4ac0..2f5ef7450b 100644 --- a/packages/storage/src/__tests__/runtime-policy-stores.test.ts +++ b/packages/storage/src/__tests__/runtime-policy-stores.test.ts @@ -1664,6 +1664,42 @@ describe('runtime policy stores', () => { }); }); + test('replaces Copilot bootstrap ids with the account-authorized model catalog', async () => { + await withInteractiveOwner(async ({ stores }) => { + const connection = await createConnection( + stores, + 0, + connectionDraft('copilot-models', 'github-copilot', 'Copilot models'), + ); + const configured = await stores.credentialVault.set({ + locator: connectionCredential(connection, 'oauth_token'), + expected: null, + secret: JSON.stringify({ + access_token: 'github-access', + refresh_token: 'github-refresh', + expires_at: Number.MAX_SAFE_INTEGER, + }), + }); + assert.equal(configured.kind, 'committed'); + + const prepared = await stores.operations.beginModelFetch(connection.connectionId); + assert.equal(prepared.kind, 'ready'); + if (prepared.kind !== 'ready') return; + const completed = await stores.operations.completeModelFetch(prepared.ticket, { + models: [{ id: 'account-available' }, { id: 'account-preview' }], + source: 'fetched', + fetchedAt: 43, + }); + assert.equal(completed.kind, 'committed'); + if (completed.kind !== 'committed') return; + + const updated = completed.snapshot.connections[0]; + assert.deepEqual(updated?.models, [{ id: 'account-available' }, { id: 'account-preview' }]); + assert.deepEqual(updated?.enabledModelIds, ['account-available', 'account-preview']); + assert.equal(updated?.enabledModelIds.includes('gpt-5'), false); + }); + }); + test('keeps the canonical default target when discovery stops listing its model', async () => { await withInteractiveOwner(async ({ stores }) => { const connection = await createConnection( diff --git a/packages/storage/src/runtime-policy/connection-catalog-document.ts b/packages/storage/src/runtime-policy/connection-catalog-document.ts index 2ff3568290..7377c3b285 100644 --- a/packages/storage/src/runtime-policy/connection-catalog-document.ts +++ b/packages/storage/src/runtime-policy/connection-catalog-document.ts @@ -469,6 +469,10 @@ export class ConnectionCatalogDocumentOwner { result.models, { aliases: modelIdAliasesForProvider(previous.providerType), + // GitHub Copilot's filtered /models response is the account's complete + // usable catalog. Unlike generic provider snapshots, omission here is + // an entitlement answer and must remove bootstrap/stale ids. + authoritative: previous.providerType === 'github-copilot', }, ); // Discovery MOVES a target: a provider's model rename carries the default From 650eb136bc2e59018aa2f580403a4679e6c456ec Mon Sep 17 00:00:00 2001 From: hgaol Date: Mon, 7 Sep 2026 12:49:44 +0000 Subject: [PATCH 2/2] fix(copilot): preserve account catalog authorization semantics Keep complete-catalog authority in model-metadata instead of hard-coding the provider in Storage. When a refresh withdraws every enabled model, preserve an empty selection rather than silently enabling the first catalogue entry; a model that later returns remains opt-in. Classify a non-empty Copilot catalogue whose otherwise usable entries are all policy-blocked as auth, including interactive entitlement verification, rather than collapsing it into an invalid empty response. Generated-by: gpt-5.6-sol --- .../src/__tests__/llm-connections.test.ts | 14 +++++++ .../core/src/__tests__/model-metadata.test.ts | 9 +++++ packages/core/src/llm-connections.ts | 3 -- packages/core/src/model-metadata.ts | 5 +++ .../github-copilot-oauth-enrollment.test.ts | 12 ++++++ .../src/__tests__/model-fetcher.test.ts | 28 ++++++++++++++ .../src/github-copilot-oauth-enrollment.ts | 6 +-- packages/runtime/src/model-fetcher.ts | 37 ++++++++++++++++++- .../connection-catalog-document.ts | 10 ++--- 9 files changed, 111 insertions(+), 13 deletions(-) diff --git a/packages/core/src/__tests__/llm-connections.test.ts b/packages/core/src/__tests__/llm-connections.test.ts index 8e27f7cc4c..a27caa90e1 100644 --- a/packages/core/src/__tests__/llm-connections.test.ts +++ b/packages/core/src/__tests__/llm-connections.test.ts @@ -168,6 +168,20 @@ test('an authoritative account catalog removes unavailable bootstrap and stale m ), { defaultModel: 'account-available', enabledModelIds: ['account-available'] }, ); + // Losing every selected model does not silently opt the user into the first + // catalogue entry. A model that later returns remains available but opt-in. + assert.deepEqual( + reconcileConnectionAfterModelFetch( + { + defaultModel: 'withdrawn', + enabledModelIds: ['withdrawn'], + hasModelInventory: true, + }, + [{ id: 'replacement' }], + { authoritative: true }, + ), + { defaultModel: '', enabledModelIds: [] }, + ); }); test('model reconciliation never invents a default the user cleared', () => { diff --git a/packages/core/src/__tests__/model-metadata.test.ts b/packages/core/src/__tests__/model-metadata.test.ts index b0b932d809..8609efcfb8 100644 --- a/packages/core/src/__tests__/model-metadata.test.ts +++ b/packages/core/src/__tests__/model-metadata.test.ts @@ -22,11 +22,20 @@ import { describe, it } from 'node:test'; import { lookupModelMetadata, openAiAdapterApiProtocol, + providerReportsCompleteModelCatalog, resolveModelVisionSupport, } from '../model-metadata.js'; import { PROVIDER_REGISTRY, providerFallbackModelIds } from '../provider-registry.js'; import type { ModelInfo, ProviderType } from '../llm-connections.js'; +describe('provider model-catalog completeness', () => { + it('treats only GitHub Copilot discovery as a complete account catalog', () => { + assert.equal(providerReportsCompleteModelCatalog('github-copilot'), true); + assert.equal(providerReportsCompleteModelCatalog('openai-codex'), false); + assert.equal(providerReportsCompleteModelCatalog('openai'), false); + }); +}); + describe('model-metadata vision capability', () => { it('treats a Claude newer than the generated snapshot as able to read images', () => { assert.deepEqual(lookupModelMetadata('anthropic', 'claude-opus-6'), {}); diff --git a/packages/core/src/llm-connections.ts b/packages/core/src/llm-connections.ts index 9035821a28..11e9205bea 100644 --- a/packages/core/src/llm-connections.ts +++ b/packages/core/src/llm-connections.ts @@ -503,9 +503,6 @@ export function reconcileConnectionAfterModelFetch( const enabledModelIds = connection.hasModelInventory ? previousEnabled.filter((id) => live.has(id)) : liveIds; - if (enabledModelIds.length === 0 && previousEnabled.length > 0 && liveIds.length > 0) { - enabledModelIds.push(liveIds[0]!); - } const defaultModel = enabledModelIds.includes(previousDefault) ? previousDefault : (enabledModelIds[0] ?? ''); diff --git a/packages/core/src/model-metadata.ts b/packages/core/src/model-metadata.ts index fb711f7b3e..4669523e59 100644 --- a/packages/core/src/model-metadata.ts +++ b/packages/core/src/model-metadata.ts @@ -87,6 +87,11 @@ function generatedMetadataProviderType(providerType: ProviderType): ProviderType return GENERATED_METADATA_PROVIDER_ALIASES[providerType] ?? providerType; } +/** Whether discovery is the complete usable model catalog for this account. */ +export function providerReportsCompleteModelCatalog(providerType: ProviderType): boolean { + return providerType === 'github-copilot'; +} + /** * Whether the active metadata describes this model at all. `lookupModelMetadata` * answers "no" with an empty object, and callers were reading that sentinel by diff --git a/packages/runtime/src/__tests__/github-copilot-oauth-enrollment.test.ts b/packages/runtime/src/__tests__/github-copilot-oauth-enrollment.test.ts index 4bbdea4e7a..4cfa598e3b 100644 --- a/packages/runtime/src/__tests__/github-copilot-oauth-enrollment.test.ts +++ b/packages/runtime/src/__tests__/github-copilot-oauth-enrollment.test.ts @@ -278,6 +278,18 @@ test('entitlement refuses an account the provider proved ineligible', async () = // provider refusing this account rather than failing to answer. const proofs: ReadonlyArray<() => Response> = [ () => copilotModelsResponse([]), + () => + Response.json({ + data: [ + { + id: 'policy-blocked', + model_picker_enabled: true, + supported_endpoints: ['/responses'], + policy: { state: 'unconfigured' }, + capabilities: { supports: { tool_calls: true } }, + }, + ], + }), () => new Response(null, { status: 401 }), () => new Response(null, { status: 403 }), ]; diff --git a/packages/runtime/src/__tests__/model-fetcher.test.ts b/packages/runtime/src/__tests__/model-fetcher.test.ts index 5f067d6245..6e615175a6 100644 --- a/packages/runtime/src/__tests__/model-fetcher.test.ts +++ b/packages/runtime/src/__tests__/model-fetcher.test.ts @@ -363,6 +363,34 @@ describe('fetchProviderModels', () => { ); }); + test('connection discovery classifies a wholly policy-blocked Copilot catalog as auth', async () => { + const server = await startJsonServer((_request, response) => { + respondJson(response, 200, { + data: [ + { + id: 'policy-blocked', + model_picker_enabled: true, + supported_endpoints: ['/responses'], + policy: { state: 'unconfigured' }, + capabilities: { supports: { tool_calls: true } }, + }, + ], + }); + }); + + const outcome = await runConnectionModelDiscoveryEffect( + { + providerType: 'github-copilot', + baseUrl: server.url, + defaultModel: 'policy-blocked', + }, + 'github-account-token', + { fetch: globalThis.fetch }, + ); + + assert.deepEqual(outcome, { ok: false, error: { kind: 'auth' } }); + }); + test('connection discovery classifies structurally invalid JSON from a real HTTP response', async () => { const secret = 'raw-provider-secret'; for (const body of [ diff --git a/packages/runtime/src/github-copilot-oauth-enrollment.ts b/packages/runtime/src/github-copilot-oauth-enrollment.ts index a0c9a89dbf..5114287420 100644 --- a/packages/runtime/src/github-copilot-oauth-enrollment.ts +++ b/packages/runtime/src/github-copilot-oauth-enrollment.ts @@ -23,7 +23,7 @@ import { isSupportedGitHubCopilotAccountToken, type OAuthSubscriptionTokens, } from './subscription-credentials.js'; -import { fetchGitHubCopilotModels } from './model-fetcher.js'; +import { fetchGitHubCopilotModels, GitHubCopilotModelPolicyError } from './model-fetcher.js'; import { ConnectionEffectHttpError } from './connection-effect-outcome.js'; import { OAUTH_LOGIN_MAX_TOKEN_CHARS, @@ -250,8 +250,8 @@ export async function verifyGitHubCopilotModelEntitlement(input: { // ineligibility would send a paying user back through a device login that // was never the problem. if ( - error instanceof ConnectionEffectHttpError && - (error.status === 401 || error.status === 403) + error instanceof GitHubCopilotModelPolicyError || + (error instanceof ConnectionEffectHttpError && (error.status === 401 || error.status === 403)) ) { throw new GitHubCopilotEntitlementError({ cause: error }); } diff --git a/packages/runtime/src/model-fetcher.ts b/packages/runtime/src/model-fetcher.ts index b389f14be9..03213ca944 100644 --- a/packages/runtime/src/model-fetcher.ts +++ b/packages/runtime/src/model-fetcher.ts @@ -407,6 +407,13 @@ function normalizeConnectionEffectModels(models: ModelInfo[]): readonly ModelInf } } +export class GitHubCopilotModelPolicyError extends Error { + constructor() { + super('GitHub Copilot model policy is not enabled'); + this.name = 'GitHubCopilotModelPolicyError'; + } +} + export async function fetchGitHubCopilotModels( baseUrl: string, accessToken: string, @@ -426,11 +433,16 @@ export async function fetchGitHubCopilotModels( throw new ConnectionEffectHttpError(response.status); } const payload = await readProviderJson<{ data?: unknown }>(response); - return providerObjectArray( + const rawModels = providerObjectArray( payload.data, 'GitHub Copilot models', true, - ).flatMap(toGitHubCopilotModelInfo); + ); + const models = rawModels.flatMap(toGitHubCopilotModelInfo); + if (models.length === 0 && rawModels.some(isGitHubCopilotModelBlockedByPolicy)) { + throw new GitHubCopilotModelPolicyError(); + } + return models; } type RawOpenAiCodexModel = { @@ -590,6 +602,26 @@ function isGitHubCopilotModelPolicyEnabled(policy: unknown): boolean { return (policy as Record).state === 'enabled'; } +function isGitHubCopilotModelBlockedByPolicy(model: RawGitHubCopilotModel): boolean { + if ( + typeof model.id !== 'string' || + !model.id || + model.model_picker_enabled !== true || + model.capabilities?.supports?.tool_calls !== true || + !Array.isArray(model.supported_endpoints) || + !model.supported_endpoints.some((endpoint) => + ['/v1/messages', '/responses', '/chat/completions'].includes(endpoint), + ) + ) { + return false; + } + if (!model.policy || typeof model.policy !== 'object' || Array.isArray(model.policy)) { + return false; + } + const state = (model.policy as Record).state; + return state === 'disabled' || state === 'unconfigured'; +} + async function fetchCohereModels( baseUrl: string, apiKey: string, @@ -930,6 +962,7 @@ function nextProviderPageToken(value: unknown): string | undefined { } function classifyDiscoveryError(error: unknown): ConnectionEffectError { + if (error instanceof GitHubCopilotModelPolicyError) return { kind: 'auth' }; if (error instanceof ConnectionEffectFetchError) return { kind: error.kind }; if (error instanceof ConnectionEffectHttpError) { return classifyConnectionEffectStatus(error.status); diff --git a/packages/storage/src/runtime-policy/connection-catalog-document.ts b/packages/storage/src/runtime-policy/connection-catalog-document.ts index 7377c3b285..14f0caa3b5 100644 --- a/packages/storage/src/runtime-policy/connection-catalog-document.ts +++ b/packages/storage/src/runtime-policy/connection-catalog-document.ts @@ -49,7 +49,10 @@ import { type UpdateCatalogConnectionInput, } from '@maka/core/runtime-policy'; import { PROVIDER_REGISTRY, reconcileConnectionAfterModelFetch } from '@maka/core/llm-connections'; -import { modelIdAliasesForProvider } from '@maka/core/model-metadata'; +import { + modelIdAliasesForProvider, + providerReportsCompleteModelCatalog, +} from '@maka/core/model-metadata'; import { isRetiredProvider } from '@maka/core/provider-registry'; import { pruneRelayModelProfiles } from '@maka/core/model-thinking'; import { deepFreeze, nextRevision, record, revision, unique } from './codec.js'; @@ -469,10 +472,7 @@ export class ConnectionCatalogDocumentOwner { result.models, { aliases: modelIdAliasesForProvider(previous.providerType), - // GitHub Copilot's filtered /models response is the account's complete - // usable catalog. Unlike generic provider snapshots, omission here is - // an entitlement answer and must remove bootstrap/stale ids. - authoritative: previous.providerType === 'github-copilot', + authoritative: providerReportsCompleteModelCatalog(previous.providerType), }, ); // Discovery MOVES a target: a provider's model rename carries the default