diff --git a/src/main/persistence-source-control-ai-migration.test.ts b/src/main/persistence-source-control-ai-migration.test.ts index ffc7d5b6eba..f02c2286a6e 100644 --- a/src/main/persistence-source-control-ai-migration.test.ts +++ b/src/main/persistence-source-control-ai-migration.test.ts @@ -167,6 +167,38 @@ describe('Store', () => { expect(store.getSettings().commitMessageAi?.customPrompt).toBe('Use Conventional Commits.') }) + it("marks Orca's released Pi seed without reinterpreting later concrete writes", async () => { + writeDataFile({ + schemaVersion: 1, + repos: [], + worktreeMeta: {}, + settings: { + commitMessageAi: { + enabled: true, + agentId: 'pi', + selectedModelByAgent: { pi: 'github-copilot/gpt-5.4-mini' }, + selectedThinkingByModel: {}, + customPrompt: '', + customAgentCommand: '' + } + }, + ui: {}, + githubCache: { pr: {}, issue: {} }, + workspaceSession: {} + }) + + const store = await createStore() + + expect(store.getSettings().piConfiguredDefaultModelState?.defaultsByHost.local).toBe(true) + store.flush() + expect((readDataFile() as PersistedState).settings.piConfiguredDefaultModelState).toMatchObject( + { + version: 1, + defaultsByHost: { local: true } + } + ) + }) + it('migrates first-work branch auto-rename on for existing profiles once', async () => { writeDataFile({ schemaVersion: 1, diff --git a/src/main/persistence/applying-settings/settings-update.ts b/src/main/persistence/applying-settings/settings-update.ts index e6b18da0644..ad14f48f56e 100644 --- a/src/main/persistence/applying-settings/settings-update.ts +++ b/src/main/persistence/applying-settings/settings-update.ts @@ -27,6 +27,7 @@ import { normalizeSourceControlAiSettings, projectSourceControlAiToLegacyCommitMessageAi } from '../../../shared/source-control-ai' +import { applyPiConfiguredDefaultSelectionUpdate } from '../../../shared/pi-configured-default-model-state' import { PROTECTED_SECRET_SLOT, type ProtectedSecretPersistence @@ -211,6 +212,15 @@ export function updateSettings( sanitizedUpdates.sourceControlAi, operations.state.settings ) + const previousSourceControlAi = normalizeSourceControlAiSettings( + operations.state.settings.sourceControlAi, + operations.state.settings.commitMessageAi + ) + sanitizedUpdates.piConfiguredDefaultModelState = applyPiConfiguredDefaultSelectionUpdate({ + previous: previousSourceControlAi, + next: sanitizedUpdates.sourceControlAi!, + state: operations.state.settings.piConfiguredDefaultModelState + }) const normalizedSourceControlAi = normalizeSourceControlAiSettings( sanitizedUpdates.sourceControlAi, operations.state.settings.commitMessageAi diff --git a/src/main/persistence/loading-store/store.ts b/src/main/persistence/loading-store/store.ts index ad89783a5a3..dd929ba212e 100644 --- a/src/main/persistence/loading-store/store.ts +++ b/src/main/persistence/loading-store/store.ts @@ -157,6 +157,7 @@ import { projectSourceControlAiToLegacyCommitMessageAi, sourceControlAiSettingsFromLegacy } from '../../../shared/source-control-ai' +import { migratePiConfiguredDefaultModelState } from '../../../shared/pi-configured-default-model-state' import { normalizeDisabledTuiAgents } from '../../../shared/tui-agent-selection' import { hasUnsupportedTuiAgentArgs } from '../../../shared/tui-agent-launch-defaults' import { normalizeTerminalCursorStyleDefault } from '../../../shared/terminal-cursor-style-settings' @@ -934,6 +935,14 @@ export class Store { parsed.settings?.sourceControlAi, legacyCommitMessageAi ) + const piConfiguredDefaultMigration = migratePiConfiguredDefaultModelState({ + sourceControlAi: migratedSourceControlAi, + commitMessageAi: legacyCommitMessageAi, + persistedState: parsed.settings?.piConfiguredDefaultModelState + }) + if (piConfiguredDefaultMigration.changed) { + this.loadNeedsSave = true + } // Why (issue #903): old 'true' default broke non-US Option-layer chars; flip 'true'→'auto' once so the layout probe decides. const rawOptionAsAlt = parsed.settings?.terminalMacOptionAsAlt const alreadyMigrated = parsed.settings?.terminalMacOptionAsAltMigrated === true @@ -1276,6 +1285,7 @@ export class Store { }), notifications: normalizedNotifications, sourceControlAi: migratedSourceControlAi, + piConfiguredDefaultModelState: piConfiguredDefaultMigration.state, sourceControlGroupOrder: normalizedSourceControlGroupOrder, // Why: rollback builds still read commitMessageAi, so refresh the legacy projection from sourceControlAi for compat. commitMessageAi: projectSourceControlAiToLegacyCommitMessageAi( diff --git a/src/main/runtime/rpc/methods/git-params.ts b/src/main/runtime/rpc/methods/git-params.ts index 00ec4bb6913..e992160d28d 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -166,6 +166,7 @@ const SourceControlAiSettings = CommitMessageAiSettings.omit({ customPrompt: tru const ResolvedSourceControlAiGenerationParams = z.object({ agentId: z.string(), model: z.string(), + useConfiguredDefaultModel: z.boolean().optional(), thinkingLevel: z.string().optional(), customPrompt: z.string().optional(), commandInputTemplate: z.string().optional(), diff --git a/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts b/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts index a023d7143e1..dc5d85fcaab 100644 --- a/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts +++ b/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts @@ -266,12 +266,12 @@ describe('discoverCommitMessageModelsLocal', () => { await expect(pending).resolves.toMatchObject({ success: true, - defaultModelId: 'default', - models: [{ id: 'default', label: 'Pi default', isDefault: true }] + defaultModelId: 'github-copilot/gpt-5.4-mini', + models: [{ id: 'github-copilot/gpt-5.4-mini' }] }) }) - it('keeps Pi default when discovery finds only a configured non-Copilot provider', async () => { + it('publishes only concrete Pi models from host discovery', async () => { const listeners = new Map void>() const child = { pid: 123, @@ -297,11 +297,8 @@ describe('discoverCommitMessageModelsLocal', () => { await expect(pending).resolves.toMatchObject({ success: true, - defaultModelId: 'default', - models: [ - { id: 'default', label: 'Pi default', isDefault: true }, - { id: 'openai-codex/gpt-5.5' } - ] + defaultModelId: 'openai-codex/gpt-5.5', + models: [{ id: 'openai-codex/gpt-5.5' }] }) }) diff --git a/src/main/text-generation/commit-message-text-generation.ts b/src/main/text-generation/commit-message-text-generation.ts index 72a466c3ea6..989e09afd1f 100644 --- a/src/main/text-generation/commit-message-text-generation.ts +++ b/src/main/text-generation/commit-message-text-generation.ts @@ -38,6 +38,7 @@ import { getAgentModelProbeSpec, type AgentModelProbeSpec } from '../../shared/agent-model-probe-spec' +import { PI_COMPATIBILITY_MODEL, PI_DEFAULT_MODEL_ID } from '../../shared/commit-message-agent-spec' import { planAgentBinary, planCommitMessageGeneration, @@ -280,6 +281,9 @@ function finalizeModelDiscoveryOutput( models = spec.modelDiscovery?.parse(stderr) ?? [] } if (models.length === 0) { + if (spec.id === 'pi') { + return toModelDiscoveryCapability(spec, [PI_COMPATIBILITY_MODEL], PI_COMPATIBILITY_MODEL.id) + } if (spec.models.length > 0) { console.warn('[commit-message] Model discovery returned no models; using static fallback:', { label: spec.label @@ -288,10 +292,11 @@ function finalizeModelDiscoveryOutput( } return { success: false, error: `${spec.label} returned no available models.` } } + models = models.filter((model) => model.id !== PI_DEFAULT_MODEL_ID) const cliDefault = spec.models.find( (model) => model.id === spec.defaultModelId && model.isDefault ) - if (cliDefault && !models.some((model) => model.id === cliDefault.id)) { + if (spec.id !== 'pi' && cliDefault && !models.some((model) => model.id === cliDefault.id)) { models = [cliDefault, ...models] } const defaultModelId = models.some((model) => model.id === spec.defaultModelId) diff --git a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts index 318f30bdb25..94465c5d914 100644 --- a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts +++ b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts @@ -212,6 +212,25 @@ describe('buildCommitMessageGenerationParams', () => { }) }) + it('preserves Pi configured-default intent through dialog reconstruction', () => { + expect( + buildCommitMessageGenerationParams({ + agentId: 'pi', + commandTemplate: '{basePrompt}', + baseParams: { + agentId: 'pi', + model: 'github-copilot/gpt-5.4-mini', + useConfiguredDefaultModel: true + }, + settings: null + }) + ).toMatchObject({ + agentId: 'pi', + model: 'github-copilot/gpt-5.4-mini', + useConfiguredDefaultModel: true + }) + }) + it('keeps existing custom-command generation usable from the dialog', () => { expect( buildCommitMessageGenerationParams({ diff --git a/src/renderer/src/components/right-sidebar/source-control/ai/text-generation-params.ts b/src/renderer/src/components/right-sidebar/source-control/ai/text-generation-params.ts index fe82e0c3380..2cb62f5fa07 100644 --- a/src/renderer/src/components/right-sidebar/source-control/ai/text-generation-params.ts +++ b/src/renderer/src/components/right-sidebar/source-control/ai/text-generation-params.ts @@ -51,6 +51,9 @@ export function buildCommitMessageGenerationParams(args: { return { agentId: args.agentId, model: modelId, + ...(sameResolvedAgent && args.baseParams?.useConfiguredDefaultModel + ? { useConfiguredDefaultModel: true } + : {}), ...(thinkingLevel ? { thinkingLevel } : {}), commandInputTemplate: args.commandTemplate, ...(args.agentArgs !== undefined ? { agentArgs: args.agentArgs } : {}), diff --git a/src/shared/commit-message-agent-spec.ts b/src/shared/commit-message-agent-spec.ts index a02682cc11f..884fe8a6211 100644 --- a/src/shared/commit-message-agent-spec.ts +++ b/src/shared/commit-message-agent-spec.ts @@ -107,7 +107,7 @@ const CLAUDE_THINKING_LEVELS: ThinkingLevel[] = [ { id: 'max', label: 'Max' } ] -const PI_DEFAULT_MODEL_ID = 'default' +export const PI_DEFAULT_MODEL_ID = 'default' /** * The model id Orca used to persist for Pi before `Pi default` existed. Orca itself @@ -157,6 +157,12 @@ function withOpenAiThinking( : {} } +export const PI_COMPATIBILITY_MODEL: CommitMessageModel = { + id: PI_RETIRED_COPILOT_DEFAULT_MODEL_ID, + label: 'Github Copilot GPT 5.4 Mini', + ...withOpenAiThinking('gpt-5.4-mini') +} + export function parseClaudeModels(stdout: string): CommitMessageModel[] { return uniqueModels( parseClaudeModelList(stdout).map((model) => { diff --git a/src/shared/commit-message-plan-model.ts b/src/shared/commit-message-plan-model.ts new file mode 100644 index 00000000000..ad6294cc06b --- /dev/null +++ b/src/shared/commit-message-plan-model.ts @@ -0,0 +1,40 @@ +import { + getCommitMessageModel, + PI_DEFAULT_MODEL_ID, + PI_RETIRED_COPILOT_DEFAULT_MODEL_ID, + type CommitMessageModel +} from './commit-message-agent-spec' +import type { TuiAgent } from './tui-agent' + +type CommitMessagePlanModelInput = { + agentId: TuiAgent | 'custom' + model: string + thinkingLevel?: string + useConfiguredDefaultModel?: boolean +} + +type CommitMessagePlanModelResult = + | { ok: true; model: CommitMessageModel; modelId: string } + | { ok: false; error: string } + +export function resolveCommitMessagePlanModel( + input: CommitMessagePlanModelInput, + agentLabel: string +): CommitMessagePlanModelResult { + if ( + input.useConfiguredDefaultModel && + (input.agentId !== 'pi' || + input.model !== PI_RETIRED_COPILOT_DEFAULT_MODEL_ID || + input.thinkingLevel !== undefined) + ) { + return { ok: false, error: 'Configured-default model intent is invalid for this request.' } + } + const modelId = input.useConfiguredDefaultModel ? PI_DEFAULT_MODEL_ID : input.model + if (input.agentId === 'custom') { + return { ok: false, error: `Model "${input.model}" is not available for ${agentLabel}.` } + } + const model = getCommitMessageModel(input.agentId, modelId) + return model + ? { ok: true, model, modelId } + : { ok: false, error: `Model "${input.model}" is not available for ${agentLabel}.` } +} diff --git a/src/shared/commit-message-plan.test.ts b/src/shared/commit-message-plan.test.ts index 0b728307bf7..f32632d750d 100644 --- a/src/shared/commit-message-plan.test.ts +++ b/src/shared/commit-message-plan.test.ts @@ -2,6 +2,41 @@ import { describe, expect, it } from 'vitest' import { planCommitMessageGeneration, planAgentBinary } from './commit-message-plan' describe('planCommitMessageGeneration', () => { + it('omits Pi model args only for valid configured-default intent', () => { + const result = planCommitMessageGeneration( + { + agentId: 'pi', + model: 'github-copilot/gpt-5.4-mini', + useConfiguredDefaultModel: true + }, + 'PROMPT' + ) + + expect(result).toMatchObject({ ok: true }) + expect(result.ok && result.plan.args).not.toContain('--model') + }) + + it('rejects configured-default intent for a non-Pi model', () => { + expect( + planCommitMessageGeneration( + { agentId: 'codex', model: 'gpt-5.4', useConfiguredDefaultModel: true }, + 'PROMPT' + ) + ).toMatchObject({ ok: false }) + }) + + it.each([ + { model: 'openai-codex/gpt-5.5' }, + { model: 'github-copilot/gpt-5.4-mini', thinkingLevel: 'medium' } + ])('rejects invalid Pi configured-default params: $model $thinkingLevel', (params) => { + expect( + planCommitMessageGeneration( + { agentId: 'pi', useConfiguredDefaultModel: true, ...params }, + 'PROMPT' + ) + ).toMatchObject({ ok: false }) + }) + it('plans Claude non-interactive generation with the prompt on stdin only', () => { const result = planCommitMessageGeneration( { diff --git a/src/shared/commit-message-plan.ts b/src/shared/commit-message-plan.ts index 83760f00ae1..ac7b61e5468 100644 --- a/src/shared/commit-message-plan.ts +++ b/src/shared/commit-message-plan.ts @@ -1,17 +1,10 @@ import type { CommandTemplateBackslash } from './commit-message-prompt' -import { - getCommitMessageAgentSpec, - getCommitMessageModel, - isCustomAgentId -} from './commit-message-agent-spec' +import { getCommitMessageAgentSpec, isCustomAgentId } from './commit-message-agent-spec' +import { resolveCommitMessagePlanModel } from './commit-message-plan-model' import { planCustomCommand, tokenizeCustomCommandTemplate } from './commit-message-prompt' import type { TuiAgent } from './tui-agent' -// Why: planning is a pure transformation from "user request + prompt text" -// into "spawn-ready binary + argv". Keeping it in shared lets both the local -// generator (main process) and the SSH provider (which delegates to the -// relay over JSON-RPC) reuse the exact same validation and arg-building -// logic without duplicating the spec/model/thinking checks. +// Why: shared planning keeps local and remote generation validation identical. export type CommitMessagePlanInput = { agentId: TuiAgent | 'custom' @@ -20,6 +13,7 @@ export type CommitMessagePlanInput = { * to run on native Windows, where `\` is the path separator (#11375). */ backslash?: CommandTemplateBackslash model: string + useConfiguredDefaultModel?: boolean thinkingLevel?: string customAgentCommand?: string agentCommandOverride?: string @@ -276,10 +270,11 @@ export function planCommitMessageGeneration( if (!spec) { return { ok: false, error: `Agent "${input.agentId}" does not support AI commit messages.` } } - const model = getCommitMessageModel(input.agentId, input.model) - if (!model) { - return { ok: false, error: `Model "${input.model}" is not available for ${spec.label}.` } + const plannedModel = resolveCommitMessagePlanModel(input, spec.label) + if (!plannedModel.ok) { + return plannedModel } + const { model, modelId: plannedModelId } = plannedModel if (input.thinkingLevel) { if (!model.thinkingLevels && spec.modelSource !== 'dynamic') { return { @@ -298,7 +293,7 @@ export function planCommitMessageGeneration( const argvPrompt = spec.promptDelivery === 'argv' ? prompt : '' const baseArgs = spec.buildArgs({ prompt: argvPrompt, - model: input.model, + model: plannedModelId, thinkingLevel: input.thinkingLevel }) const agentArgs = planAdditionalAgentArgs(input.agentArgs, input.backslash) diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 811aada6144..e1f5d153158 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -391,6 +391,11 @@ export function getDefaultSettings(homedir: string): GlobalSettings { customAgentCommand: '' }, sourceControlAi: getDefaultSourceControlAiSettings(), + piConfiguredDefaultModelState: { + version: 1, + defaultsByHost: {}, + commitMessageSeedByHost: {} + }, voice: getDefaultVoiceSettings() } } diff --git a/src/shared/global-settings-types.ts b/src/shared/global-settings-types.ts index 6c331594e37..0345fed0f83 100644 --- a/src/shared/global-settings-types.ts +++ b/src/shared/global-settings-types.ts @@ -441,6 +441,12 @@ export type GlobalSettings = { commitMessageAi?: CommitMessageAiSettings /** Source-control AI generation settings for commit messages and hosted-review drafts. */ sourceControlAi?: SourceControlAiSettings + /** Distinguishes Orca's former automatic Pi seed from a later explicit choice. */ + piConfiguredDefaultModelState?: { + version: 1 + defaultsByHost: Record + commitMessageSeedByHost: Record + } /** GitLab project preferences (pinned + recent paths). Optional for pre-GitLab profiles; persistence merge fills the default. */ gitlabProjects?: GitLabProjectSettings /** Anonymous product-telemetry state; optional until the one-shot Store.load() migration populates it. diff --git a/src/shared/pi-configured-default-model-state.ts b/src/shared/pi-configured-default-model-state.ts new file mode 100644 index 00000000000..fe7bcd9e4ac --- /dev/null +++ b/src/shared/pi-configured-default-model-state.ts @@ -0,0 +1,160 @@ +import { + PI_DEFAULT_MODEL_ID, + PI_RETIRED_COPILOT_DEFAULT_MODEL_ID +} from './commit-message-agent-spec' +import type { CommitMessageAiSettings } from './commit-message-ai-types' +import type { GlobalSettings } from './global-settings-types' +import type { SourceControlAiSettings } from './source-control-ai-types' + +export type PiConfiguredDefaultModelState = NonNullable< + GlobalSettings['piConfiguredDefaultModelState'] +> + +function normalizeModelId(modelId: string | undefined): string | undefined { + return modelId === PI_DEFAULT_MODEL_ID ? PI_RETIRED_COPILOT_DEFAULT_MODEL_ID : modelId +} + +function normalizeChoice(choice: { + selectedModelByAgent?: Partial> + selectedModelByAgentByHost?: Partial>>> +}): boolean { + let changed = false + if (choice.selectedModelByAgent?.pi === PI_DEFAULT_MODEL_ID) { + choice.selectedModelByAgent.pi = PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + changed = true + } + for (const hostModels of Object.values(choice.selectedModelByAgentByHost ?? {})) { + if (hostModels?.pi === PI_DEFAULT_MODEL_ID) { + hostModels.pi = PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + changed = true + } + } + return changed +} + +export function migratePiConfiguredDefaultModelState(args: { + sourceControlAi: SourceControlAiSettings + commitMessageAi: CommitMessageAiSettings | null | undefined + persistedState: GlobalSettings['piConfiguredDefaultModelState'] +}): { state: PiConfiguredDefaultModelState; changed: boolean } { + const { sourceControlAi, commitMessageAi } = args + let changed = normalizeChoice(sourceControlAi) + if (commitMessageAi) { + changed = normalizeChoice(commitMessageAi) || changed + } + for (const choice of Object.values(sourceControlAi.modelOverridesByOperation ?? {})) { + if (choice) { + changed = normalizeChoice(choice) || changed + } + } + + const state: PiConfiguredDefaultModelState = + args.persistedState?.version === 1 + ? structuredClone(args.persistedState) + : { version: 1, defaultsByHost: {}, commitMessageSeedByHost: {} } + if (!args.persistedState) { + const localModel = + sourceControlAi.selectedModelByAgentByHost?.local?.pi ?? + sourceControlAi.selectedModelByAgent.pi + if (normalizeModelId(localModel) === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID) { + state.defaultsByHost.local = true + } + for (const [hostKey, models] of Object.entries( + sourceControlAi.selectedModelByAgentByHost ?? {} + )) { + if (normalizeModelId(models?.pi) === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID) { + state.defaultsByHost[hostKey] = true + } + } + const commitChoice = sourceControlAi.modelOverridesByOperation?.commitMessage + const legacyLocal = + commitMessageAi?.selectedModelByAgentByHost?.local?.pi ?? + commitMessageAi?.selectedModelByAgent.pi + const commitLocal = + commitChoice?.selectedModelByAgentByHost?.local?.pi ?? commitChoice?.selectedModelByAgent?.pi + if ( + normalizeModelId(commitLocal) === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID && + normalizeModelId(legacyLocal) === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + ) { + state.commitMessageSeedByHost.local = true + } + for (const [hostKey, models] of Object.entries( + commitChoice?.selectedModelByAgentByHost ?? {} + )) { + if ( + normalizeModelId(models?.pi) === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID && + normalizeModelId(commitMessageAi?.selectedModelByAgentByHost?.[hostKey]?.pi) === + PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + ) { + state.commitMessageSeedByHost[hostKey] = true + } + } + changed = true + } + return { state, changed } +} + +function modelForHost( + choice: { + selectedModelByAgent?: Partial> + selectedModelByAgentByHost?: Partial>>> + }, + hostKey: string +): string | undefined { + return ( + choice.selectedModelByAgentByHost?.[hostKey]?.pi ?? + (hostKey === 'local' ? choice.selectedModelByAgent?.pi : undefined) + ) +} + +export function applyPiConfiguredDefaultSelectionUpdate(args: { + previous: SourceControlAiSettings + next: SourceControlAiSettings + state: GlobalSettings['piConfiguredDefaultModelState'] +}): PiConfiguredDefaultModelState { + const state: PiConfiguredDefaultModelState = + args.state?.version === 1 + ? structuredClone(args.state) + : { version: 1, defaultsByHost: {}, commitMessageSeedByHost: {} } + const hostKeys = new Set([ + 'local', + ...Object.keys(args.previous.selectedModelByAgentByHost ?? {}), + ...Object.keys(args.next.selectedModelByAgentByHost ?? {}) + ]) + for (const hostKey of hostKeys) { + const previous = modelForHost(args.previous, hostKey) + const next = modelForHost(args.next, hostKey) + if (next === previous) { + continue + } + if (next === PI_DEFAULT_MODEL_ID) { + state.defaultsByHost[hostKey] = true + } else { + delete state.defaultsByHost[hostKey] + } + } + const previousCommit = args.previous.modelOverridesByOperation?.commitMessage ?? {} + const nextCommit = args.next.modelOverridesByOperation?.commitMessage ?? {} + const commitHostKeys = new Set([ + 'local', + ...Object.keys(previousCommit.selectedModelByAgentByHost ?? {}), + ...Object.keys(nextCommit.selectedModelByAgentByHost ?? {}) + ]) + for (const hostKey of commitHostKeys) { + const previous = modelForHost(previousCommit, hostKey) + const next = modelForHost(nextCommit, hostKey) + if (next === previous) { + continue + } + if (next === PI_DEFAULT_MODEL_ID) { + state.commitMessageSeedByHost[hostKey] = true + } else { + delete state.commitMessageSeedByHost[hostKey] + } + } + normalizeChoice(args.next) + if (args.next.modelOverridesByOperation?.commitMessage) { + normalizeChoice(args.next.modelOverridesByOperation.commitMessage) + } + return state +} diff --git a/src/shared/source-control-ai-pi-default.test.ts b/src/shared/source-control-ai-pi-default.test.ts new file mode 100644 index 00000000000..89b75612548 --- /dev/null +++ b/src/shared/source-control-ai-pi-default.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest' +import { planCommitMessageGeneration } from './commit-message-plan' +import { getDefaultSettings } from './constants' +import { resolveSourceControlAiForOperation } from './source-control-ai' +import type { SourceControlAiOperation } from './source-control-ai-types' +import type { GlobalSettings } from './global-settings-types' + +const COMPATIBILITY_MODEL_ID = 'github-copilot/gpt-5.4-mini' +const EXPLICIT_MODEL_ID = 'openai-codex/gpt-5.5' + +function piSettings(): GlobalSettings { + const base = getDefaultSettings('/tmp') + return { + ...base, + defaultTuiAgent: 'pi', + commitMessageAi: { + ...base.commitMessageAi!, + agentId: 'pi', + selectedModelByAgent: {} + }, + sourceControlAi: { + ...base.sourceControlAi!, + agentId: 'pi', + selectedModelByAgent: {} + } + } +} + +function resolvePiPlan( + operation: SourceControlAiOperation, + configuredModel?: string +): ReturnType { + const settings = piSettings() + if (configuredModel) { + settings.sourceControlAi!.discoveredModelsByAgent = { + pi: [{ id: configuredModel, label: 'Configured model' }] + } + settings.sourceControlAi!.modelOverridesByOperation = { + [operation]: { selectedModelByAgent: { pi: configuredModel } } + } + } + const resolved = resolveSourceControlAiForOperation({ + settings, + repo: null, + operation, + discoveryHostKey: 'local' + }) + expect(resolved.ok).toBe(true) + if (!resolved.ok) { + throw new Error(resolved.error) + } + return planCommitMessageGeneration(resolved.value.params, 'PROMPT') +} + +describe('Pi source-control AI model resolution', () => { + it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( + 'uses Pi configured default for %s without a model override', + (operation) => { + const result = resolvePiPlan(operation) + + expect(result).toMatchObject({ ok: true }) + expect(result.ok && result.plan.args).not.toContain('--model') + expect(result.ok && result.plan.args).not.toContain(COMPATIBILITY_MODEL_ID) + } + ) + + it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( + 'uses the migrated Pi default seed for %s', + (operation) => { + const settings = piSettings() + settings.sourceControlAi!.selectedModelByAgent = { pi: COMPATIBILITY_MODEL_ID } + settings.commitMessageAi!.selectedModelByAgent = { pi: COMPATIBILITY_MODEL_ID } + settings.piConfiguredDefaultModelState = { + version: 1, + defaultsByHost: { local: true }, + commitMessageSeedByHost: {} + } + settings.sourceControlAi!.discoveredModelsByAgent = { + pi: [{ id: COMPATIBILITY_MODEL_ID, label: 'GPT-5.4 mini' }] + } + + const resolved = resolveSourceControlAiForOperation({ + settings, + repo: null, + operation, + discoveryHostKey: 'local' + }) + expect(resolved.ok).toBe(true) + if (!resolved.ok) { + throw new Error(resolved.error) + } + const result = planCommitMessageGeneration(resolved.value.params, 'PROMPT') + + expect(result).toMatchObject({ ok: true }) + expect(result.ok && result.plan.args).not.toContain('--model') + expect(result.ok && result.plan.args).not.toContain(COMPATIBILITY_MODEL_ID) + } + ) + + it("preserves an explicit selection of Pi's former compatibility model", () => { + expect(resolvePiPlan('commitMessage', COMPATIBILITY_MODEL_ID)).toMatchObject({ + ok: true, + plan: { args: expect.arrayContaining(['--model', COMPATIBILITY_MODEL_ID]) } + }) + }) + + it('preserves a concrete Pi selection before discovery has populated', () => { + const settings = piSettings() + settings.sourceControlAi!.selectedModelByAgent = { pi: EXPLICIT_MODEL_ID } + const resolved = resolveSourceControlAiForOperation({ + settings, + repo: null, + operation: 'commitMessage', + discoveryHostKey: 'local' + }) + + expect(resolved.ok).toBe(true) + if (!resolved.ok) { + throw new Error(resolved.error) + } + expect(planCommitMessageGeneration(resolved.value.params, 'PROMPT')).toMatchObject({ + ok: true, + plan: { args: expect.arrayContaining(['--model', EXPLICIT_MODEL_ID]) } + }) + }) + + it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( + 'passes an explicit Pi model override for %s', + (operation) => { + expect(resolvePiPlan(operation, EXPLICIT_MODEL_ID)).toMatchObject({ + ok: true, + plan: { args: expect.arrayContaining(['--model', EXPLICIT_MODEL_ID]) } + }) + } + ) +}) diff --git a/src/shared/source-control-ai.test.ts b/src/shared/source-control-ai.test.ts index d506d4a510e..d839a77c769 100644 --- a/src/shared/source-control-ai.test.ts +++ b/src/shared/source-control-ai.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest' import { getDefaultSettings } from './constants' -import { planCommitMessageGeneration } from './commit-message-plan' import { clearSourceControlAiModelChoiceForHost, hasConfiguredSourceControlAiInstructions, @@ -20,8 +19,6 @@ import type { } from './source-control-ai-types' import type { GlobalSettings } from './global-settings-types' -const PI_EXPLICIT_MODEL_ID = 'openai-codex/gpt-5.5' - function settings(): GlobalSettings { const base = getDefaultSettings('/tmp') return { @@ -62,106 +59,7 @@ function resolve(operation: SourceControlAiOperation, overrides?: RepoSourceCont return result.value } -function piSettings(): GlobalSettings { - const base = getDefaultSettings('/tmp') - return { - ...base, - defaultTuiAgent: 'pi', - commitMessageAi: { - ...base.commitMessageAi!, - agentId: 'pi', - selectedModelByAgent: {} - }, - sourceControlAi: { - ...base.sourceControlAi!, - agentId: 'pi', - selectedModelByAgent: {} - } - } -} - -function resolvePiPlan( - operation: SourceControlAiOperation, - configuredModel?: string -): ReturnType { - const base = piSettings() - if (configuredModel) { - base.sourceControlAi!.discoveredModelsByAgent = { - pi: [{ id: configuredModel, label: 'Configured model' }] - } - base.sourceControlAi!.modelOverridesByOperation = { - [operation]: { selectedModelByAgent: { pi: configuredModel } } - } - } - const resolved = resolveSourceControlAiForOperation({ - settings: base, - repo: null, - operation, - discoveryHostKey: 'local' - }) - expect(resolved.ok).toBe(true) - if (!resolved.ok) { - throw new Error(resolved.error) - } - return planCommitMessageGeneration(resolved.value.params, 'PROMPT') -} - describe('source-control AI resolution', () => { - it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( - 'lets Pi use its configured default for %s when Orca has no model override', - (operation) => { - const result = resolvePiPlan(operation) - - expect(result).toMatchObject({ ok: true }) - expect(result.ok && result.plan.args).not.toContain('--model') - expect(result.ok && result.plan.args).not.toContain('github-copilot/gpt-5.4-mini') - } - ) - - // Why: Orca seeded `github-copilot/gpt-5.4-mini` into selectedModelByAgent.pi the - // first time a user picked Pi on a pre-fix build, and `pi --list-models` still - // lists it, so the resolution chain honoured it and STA-4249 kept reproducing for - // every upgrading user. The other cases here all start from an empty selection. - it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( - 'ignores the retired Copilot default that Orca persisted for Pi, for %s', - (operation) => { - const base = piSettings() - base.sourceControlAi!.selectedModelByAgent = { pi: 'github-copilot/gpt-5.4-mini' } - base.commitMessageAi!.selectedModelByAgent = { pi: 'github-copilot/gpt-5.4-mini' } - base.sourceControlAi!.discoveredModelsByAgent = { - pi: [{ id: 'github-copilot/gpt-5.4-mini', label: 'GPT-5.4 mini' }] - } - - const resolved = resolveSourceControlAiForOperation({ - settings: base, - repo: null, - operation, - discoveryHostKey: 'local' - }) - expect(resolved.ok).toBe(true) - if (!resolved.ok) { - throw new Error(resolved.error) - } - const result = planCommitMessageGeneration(resolved.value.params, 'PROMPT') - - expect(result).toMatchObject({ ok: true }) - expect(result.ok && result.plan.args).not.toContain('--model') - expect(result.ok && result.plan.args).not.toContain('github-copilot/gpt-5.4-mini') - } - ) - - it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( - 'passes an explicit Pi model override for %s', - (operation) => { - const result = resolvePiPlan(operation, PI_EXPLICIT_MODEL_ID) - - expect(result).toMatchObject({ - ok: true, - plan: { args: expect.arrayContaining(['--model', PI_EXPLICIT_MODEL_ID]) } - }) - } - ) - it('uses the global default model for every operation', () => { expect(resolve('commitMessage').params.model).toBe('gpt-5.5') expect(resolve('pullRequest').params.model).toBe('gpt-5.5') diff --git a/src/shared/source-control-ai.ts b/src/shared/source-control-ai.ts index b7072ac67f8..439264eee38 100644 --- a/src/shared/source-control-ai.ts +++ b/src/shared/source-control-ai.ts @@ -9,6 +9,7 @@ import { type CustomAgentId, isCustomAgentId, resolveCommitMessageAgentChoice, + PI_DEFAULT_MODEL_ID, PI_RETIRED_COPILOT_DEFAULT_MODEL_ID } from './commit-message-agent-spec' import { LOCAL_COMMIT_MESSAGE_HOST_KEY } from './commit-message-host-key' @@ -49,6 +50,7 @@ export const DEFAULT_SOURCE_CONTROL_AI_PR_CREATION_DEFAULTS: Required & Partial> repo?: Pick | null @@ -116,6 +122,28 @@ function copyRecord(value: T | undefined): T | undefined { return value === undefined ? undefined : structuredClone(value) } +function prunePiConfiguredDefaultFromDiscovery( + value: SourceControlAiSettings['discoveredModelsByAgent'] | undefined +): SourceControlAiSettings['discoveredModelsByAgent'] | undefined { + const copied = copyRecord(value) + if (copied?.pi) { + copied.pi = copied.pi.filter((model) => model.id !== PI_DEFAULT_MODEL_ID) + } + return copied +} + +function prunePiConfiguredDefaultFromHostDiscovery( + value: SourceControlAiSettings['discoveredModelsByAgentByHost'] | undefined +): SourceControlAiSettings['discoveredModelsByAgentByHost'] | undefined { + const copied = copyRecord(value) + for (const hostModels of Object.values(copied ?? {})) { + if (hostModels?.pi) { + hostModels.pi = hostModels.pi.filter((model) => model.id !== PI_DEFAULT_MODEL_ID) + } + } + return copied +} + function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value) } @@ -777,9 +805,11 @@ export function normalizeSourceControlAiSettings( selectedModelByAgentByHost: copyRecord(base.selectedModelByAgentByHost) ?? defaults.selectedModelByAgentByHost, discoveredModelsByAgent: - copyRecord(base.discoveredModelsByAgent) ?? defaults.discoveredModelsByAgent, + prunePiConfiguredDefaultFromDiscovery(base.discoveredModelsByAgent) ?? + defaults.discoveredModelsByAgent, discoveredModelsByAgentByHost: - copyRecord(base.discoveredModelsByAgentByHost) ?? defaults.discoveredModelsByAgentByHost, + prunePiConfiguredDefaultFromHostDiscovery(base.discoveredModelsByAgentByHost) ?? + defaults.discoveredModelsByAgentByHost, selectedThinkingByModel: { ...defaults.selectedThinkingByModel, ...base.selectedThinkingByModel @@ -894,8 +924,11 @@ export function projectSourceControlAiToLegacyCommitMessageAi( sourceControlAi.selectedModelByAgentByHost, commitMessageChoice?.selectedModelByAgentByHost ), - discoveredModelsByAgent: copyRecord(sourceControlAi.discoveredModelsByAgent) ?? {}, - discoveredModelsByAgentByHost: copyRecord(sourceControlAi.discoveredModelsByAgentByHost) ?? {}, + discoveredModelsByAgent: + prunePiConfiguredDefaultFromDiscovery(sourceControlAi.discoveredModelsByAgent) ?? {}, + discoveredModelsByAgentByHost: + prunePiConfiguredDefaultFromHostDiscovery(sourceControlAi.discoveredModelsByAgentByHost) ?? + {}, selectedThinkingByModel: { ...sourceControlAi.selectedThinkingByModel, ...commitMessageChoice?.selectedThinkingByModel @@ -940,7 +973,7 @@ function getDiscoveredModels( ) } -function selectPersistedModelId(args: { +function selectPersistedModel(args: { source: SourceControlAiSettings legacy: CommitMessageAiSettings | null | undefined repoOverrides: RepoSourceControlAiOverrides | null | undefined @@ -948,33 +981,48 @@ function selectPersistedModelId(args: { hostKey: string agentId: TuiAgent defaultModelId: string -}): string { - const { source, legacy, repoOverrides, operation, hostKey, agentId, defaultModelId } = args - const persisted = - readSourceControlAiModelChoiceForHost( - repoOverrides?.modelOverridesByOperation?.[operation], - hostKey, - agentId - ) ?? - readSourceControlAiModelChoiceForHost( - source.modelOverridesByOperation?.[operation], - hostKey, - agentId - ) ?? + configuredDefaultState: GlobalSettings['piConfiguredDefaultModelState'] +}): { modelId: string; useConfiguredDefaultModel: boolean } { + const { + source, + legacy, + repoOverrides, + operation, + hostKey, + agentId, + defaultModelId, + configuredDefaultState + } = args + const repoModel = readSourceControlAiModelChoiceForHost( + repoOverrides?.modelOverridesByOperation?.[operation], + hostKey, + agentId + ) + const operationModel = readSourceControlAiModelChoiceForHost( + source.modelOverridesByOperation?.[operation], + hostKey, + agentId + ) + const defaultModel = readDefaultSelectedModelId(source, hostKey, agentId) ?? legacy?.selectedModelByAgentByHost?.[hostKey]?.[agentId] ?? (hostKey === LOCAL_COMMIT_MESSAGE_HOST_KEY ? legacy?.selectedModelByAgent?.[agentId] - : undefined) ?? - defaultModelId - // Why: Orca seeded `github-copilot/gpt-5.4-mini` into Pi's persisted selection - // before `Pi default` existed, so a pre-upgrade user is pinned to a Copilot model - // they never chose and STA-4249 would still reproduce for them. Discovery still - // lists that id, so the resolution chain below would otherwise honour it. - if (agentId === 'pi' && persisted === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID) { - return defaultModelId - } - return persisted + : undefined) + const persisted = repoModel ?? operationModel ?? defaultModel ?? defaultModelId + const markedOperationSeed = + operation === 'commitMessage' && + configuredDefaultState?.commitMessageSeedByHost[hostKey] === true && + repoModel === undefined && + operationModel === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + const markedDefault = + configuredDefaultState?.defaultsByHost[hostKey] === true && + repoModel === undefined && + operationModel === undefined && + persisted === PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + const useConfiguredDefaultModel = + agentId === 'pi' && (persisted === PI_DEFAULT_MODEL_ID || markedOperationSeed || markedDefault) + return { modelId: persisted, useConfiguredDefaultModel } } function resolveThinkingLevel(args: { @@ -1291,31 +1339,38 @@ export function resolveSourceControlAiForOperation( } const hostKey = input.discoveryHostKey ?? LOCAL_COMMIT_MESSAGE_HOST_KEY - const persistedModelId = selectPersistedModelId({ + const persistedModel = selectPersistedModel({ source, legacy, repoOverrides, operation: input.operation, hostKey, agentId: resolvedActionAgentId, - defaultModelId: spec.defaultModelId + defaultModelId: spec.defaultModelId, + configuredDefaultState: input.settings.piConfiguredDefaultModelState }) + const persistedModelId = persistedModel.modelId const discoveredModels = getDiscoveredModels(source, legacy, hostKey, resolvedActionAgentId) const model = spec.models.find((candidate) => candidate.id === persistedModelId) ?? discoveredModels.find((candidate) => candidate.id === persistedModelId) ?? + (spec.modelSource === 'dynamic' && persistedModelId + ? { id: persistedModelId, label: persistedModelId } + : undefined) ?? getCommitMessageModel(resolvedActionAgentId, spec.defaultModelId) if (!model) { return { ok: false, error: `No model is available for ${spec.label}.` } } - const thinkingLevel = resolveThinkingLevel({ - model, - source, - legacy, - repoOverrides, - operation: input.operation - }) + const thinkingLevel = persistedModel.useConfiguredDefaultModel + ? undefined + : resolveThinkingLevel({ + model, + source, + legacy, + repoOverrides, + operation: input.operation + }) const agentCommandOverride = input.settings.agentCmdOverrides?.[resolvedActionAgentId]?.trim() return { ok: true, @@ -1323,7 +1378,10 @@ export function resolveSourceControlAiForOperation( enabled: true, params: { agentId: resolvedActionAgentId, - model: model.id, + model: persistedModel.useConfiguredDefaultModel + ? PI_RETIRED_COPILOT_DEFAULT_MODEL_ID + : model.id, + ...(persistedModel.useConfiguredDefaultModel ? { useConfiguredDefaultModel: true } : {}), thinkingLevel, customPrompt: resolveInstructionsFromNormalized( source,