feat(gemini): support mid-session model change#379
Conversation
There was a problem hiding this comment.
Findings
- [Major] Selecting
Defaultdoes not actually clear an explicit Gemini session model. Inset-session-config, thenullpath is converted back toruntimeConfig.model, and that value is returned inapplied.model, so the hub persists the previous concrete model and resume keeps treating it as explicit. Evidencecli/src/gemini/runGemini.ts:151, contexthub/src/sync/sessionCache.ts:279.
Suggested fix:let sessionModel: string | null = persistedModel ?? null let resolvedModel = sessionModel ?? runtimeConfig.model if (config.model !== undefined) { sessionModel = resolveModel(config.model) resolvedModel = sessionModel ?? runtimeConfig.model applied.model = sessionModel } sessionInstance.setModel(sessionModel)
- [Major] Custom Gemini models disappear from the composer selector. The new Gemini branch only mirrors
MODEL_OPTIONS.gemini, so a session started from a custom env/local model has no selected option, andCmd/Ctrl+Mimmediately jumps toDefault, overwriting the custom model on first toggle. Evidenceweb/src/components/AssistantChat/modelOptions.ts:7, contextweb/src/components/AssistantChat/HappyComposer.tsx:544.
Suggested fix:function getGeminiModelOptions(currentModel?: string | null): ModelOption[] { const options = MODEL_OPTIONS.gemini.map((m) => ({ value: m.value === 'auto' ? null : m.value, label: m.label })) const normalized = currentModel?.trim() || null if (normalized && !options.some((option) => option.value === normalized)) { options.splice(1, 0, { value: normalized, label: normalized }) } return options }
Summary
Review mode: initial. Two regressions in the Gemini model-change flow: Default cannot clear a persisted explicit model, and custom Gemini models are no longer preserved in the composer UI.
Testing
Not run (automation): bun is unavailable in this runner (/bin/bash: bun: command not found). Existing added tests do not cover clearing a persisted Gemini model or preserving a custom Gemini model in the composer selector.
HAPI Bot
cli/src/gemini/runGemini.ts
Outdated
|
|
||
| if (config.model !== undefined) { | ||
| const newModel = resolveModel(config.model); | ||
| resolvedModel = newModel ?? runtimeConfig.model; |
There was a problem hiding this comment.
[MAJOR] null is turned back into runtimeConfig.model here, so the hub never sees a cleared preference. After a session has ever been started with an explicit Gemini model, choosing Default keeps persisting that concrete value and resume continues launching with it.
Suggested fix:
let sessionModel: string | null = persistedModel ?? null
let resolvedModel = sessionModel ?? runtimeConfig.model
if (config.model !== undefined) {
sessionModel = resolveModel(config.model)
resolvedModel = sessionModel ?? runtimeConfig.model
applied.model = sessionModel
}
sessionInstance.setModel(sessionModel)|
|
||
| export type ModelOption = ClaudeComposerModelOption | ||
|
|
||
| function getGeminiModelOptions(): ModelOption[] { |
There was a problem hiding this comment.
[MAJOR] This helper ignores currentModel for Gemini, so custom models coming from env/local config are missing from the option list. In the composer that means no selected radio for the active model, and Cmd/Ctrl+M jumps straight to Default, overwriting the custom value on the first toggle.
Suggested fix:
function getGeminiModelOptions(currentModel?: string | null): ModelOption[] {
const options = MODEL_OPTIONS.gemini.map((m) => ({
value: m.value === 'auto' ? null : m.value,
label: m.label
}))
const normalized = currentModel?.trim() || null
if (normalized && !options.some((option) => option.value === normalized)) {
options.splice(1, 0, { value: normalized, label: normalized })
}
return options
}d1edd2b to
4cfe9d6
Compare
- Add GEMINI_MODEL_LABELS (single source of truth), GEMINI_MODEL_PRESETS (derived via Object.keys), and DEFAULT_GEMINI_MODEL to shared/modes.ts - Import DEFAULT_GEMINI_MODEL from @hapi/protocol in cli/config.ts - Generate web model options from shared presets and labels - Use GEMINI_MODEL_PRESETS.join() for ACP error message
- Add model field handling to Gemini set-session-config RPC handler, including null (Default) support, following the runClaude.ts pattern - Pass null through to hub for DB clearing on Default selection - Add setModel() to GeminiSession for keepalive/DB persistence - Read current model from session in loop launchers on local/remote switch - Allow /sessions/:id/model endpoint for Gemini sessions (was Claude-only) - Introduce supportsModelChange() flavor capability function - Add flavor-aware model options for HappyComposer UI, including custom model display for env/config-provided non-preset models
4cfe9d6 to
94d2119
Compare
There was a problem hiding this comment.
Findings
- [Major] Gemini sessions now persist the resolved runtime model as an explicit session preference, so sessions started on
Defaultstop behaving likeDefaultafter the first keepalive.syncSessionMode()writesresolvedModelinto the session object,AgentSessionBaseincludes that in keepalives, and the hub stores it for resume. Evidencecli/src/gemini/runGemini.ts:108, contextcli/src/agent/sessionBase.ts:127,hub/src/sync/sessionCache.ts:184,hub/src/sync/syncEngine.ts:402.
Suggested fix:let sessionModel: string | null = persistedModel ?? null let resolvedModel = sessionModel ?? runtimeConfig.model const syncSessionMode = () => { const sessionInstance = sessionWrapperRef.current if (!sessionInstance) return sessionInstance.setPermissionMode(currentPermissionMode) sessionInstance.setModel(sessionModel) } if (config.model !== undefined) { sessionModel = resolveModel(config.model) resolvedModel = sessionModel ?? runtimeConfig.model applied.model = sessionModel }
Summary
Review mode: follow-up after new commits. One major regression remains unresolved: the Gemini CLI still writes the resolved runtime model into keepalive state, so the hub persists it as an explicit session model and resume no longer behaves like Default.
Testing
Not run (automation): bun is unavailable in this runner (/bin/bash: bun: command not found). Added tests still do not cover keepalive/resume persistence when a Gemini session stays on Default.
HAPI Bot
cli/src/gemini/runGemini.ts
Outdated
| } | ||
| sessionInstance.setPermissionMode(currentPermissionMode); | ||
| logger.debug(`[gemini] Synced session permission mode for keepalive: ${currentPermissionMode}`); | ||
| sessionInstance.setModel(resolvedModel); |
There was a problem hiding this comment.
[MAJOR] syncSessionMode() is still storing resolvedModel in the session, so the keepalive path reports a concrete Gemini model even when the session is supposed to be on Default. The hub then persists that value and resume starts treating the fallback/env/local runtime model as an explicit session override.
Suggested fix:
let sessionModel: string | null = persistedModel ?? null
let resolvedModel = sessionModel ?? runtimeConfig.model
const syncSessionMode = () => {
const sessionInstance = sessionWrapperRef.current
if (!sessionInstance) return
sessionInstance.setPermissionMode(currentPermissionMode)
sessionInstance.setModel(sessionModel)
}
if (config.model !== undefined) {
sessionModel = resolveModel(config.model)
resolvedModel = sessionModel ?? runtimeConfig.model
applied.model = sessionModel
}Separate sessionModel (nullable, for DB persistence) from resolvedModel (always concrete, for runtime). syncSessionMode() now stores sessionModel so the hub correctly clears the model preference on Default selection.
- Test null model propagation through keepalive/syncSessionMode path - Test custom env/config model appears in Gemini model options - Test preset model is not duplicated in options
There was a problem hiding this comment.
Findings
- [Minor] Gemini
Defaultis now labeled asGemini 2.5 Pro, but the CLI still resolves the effective default fromGEMINI_MODEL_ENVor~/.geminiconfig before falling back togemini-2.5-pro. That makes both the new-session selector and the composer menu advertise the wrong model whenever a machine has a custom/default Gemini config. Evidenceweb/src/components/NewSession/types.ts:28, contextcli/src/gemini/utils/config.ts:97,cli/src/gemini/utils/config.ts:100,cli/src/gemini/utils/config.ts:103.
Suggested fix:gemini: [ { value: 'auto', label: 'Default' }, ...GEMINI_MODEL_PRESETS.map((m) => ({ value: m, label: GEMINI_MODEL_LABELS[m] })), ],
Summary
Review mode: follow-up after new commits. One minor correctness issue remains: the new Gemini Default label can misreport which model will actually run, because runtime resolution still prefers env/local Gemini config over the shared fallback constant. No other high-confidence issues found in the updated diff.
Testing
Not run (automation): bun unavailable in this runner (/bin/bash: bun: command not found).
HAPI Bot
| { value: 'gemini-2.5-pro', label: 'Gemini 2.5 Pro' }, | ||
| { value: 'gemini-2.5-flash', label: 'Gemini 2.5 Flash' }, | ||
| { value: 'gemini-2.5-flash-lite', label: 'Gemini 2.5 Flash Lite' }, | ||
| { value: 'auto', label: `Default (${GEMINI_MODEL_LABELS[DEFAULT_GEMINI_MODEL]})` }, |
There was a problem hiding this comment.
[MINOR] Default (${GEMINI_MODEL_LABELS[DEFAULT_GEMINI_MODEL]}) hardcodes Gemini 2.5 Pro into the UI, but resolveGeminiRuntimeConfig() still prefers GEMINI_MODEL_ENV and ~/.gemini config before it falls back to DEFAULT_GEMINI_MODEL (cli/src/gemini/utils/config.ts:97-105). After this change, users on a machine with a custom Gemini default will see the wrong model in both the create-session form and the composer menu, because modelOptions.ts reuses MODEL_OPTIONS.gemini.
Suggested fix:
gemini: [
{ value: 'auto', label: 'Default' },
...GEMINI_MODEL_PRESETS.map((m) => ({ value: m, label: GEMINI_MODEL_LABELS[m] })),
],There was a problem hiding this comment.
Findings
- [Major] Selecting Gemini
Defaultcannot restore the machine default once the session was started or resumed with an explicit model.resolveGeminiRuntimeConfig({ model: opts.model })marks that startup model as explicit, and the new fallback path reusesruntimeConfig.modelwhenmodel: nullis selected, so the next local/remote launcher restart keeps using the old explicit model instead of env/~/.gemini/shared default. Evidencecli/src/gemini/runGemini.ts:65,cli/src/gemini/runGemini.ts:154, contextcli/src/gemini/loop.ts:49,cli/src/gemini/utils/config.ts:97.
Suggested fix:const runtimeConfig = resolveGeminiRuntimeConfig({ model: opts.model }) const machineDefaultModel = resolveGeminiRuntimeConfig().model let sessionModel: string | null = persistedModel ?? null let resolvedModel = sessionModel ?? machineDefaultModel if (config.model !== undefined) { sessionModel = resolveModel(config.model) resolvedModel = sessionModel ?? machineDefaultModel } await geminiLoop({ // ... model: machineDefaultModel, })
Summary
Review mode: follow-up after new commits. One major correctness issue remains: Gemini Default does not actually switch back to the machine/default model after a session starts with an explicit model, so subsequent local/remote relaunches keep using the stale explicit model. No other high-confidence issues found in the updated diff.
Testing
Not run (automation): bun unavailable in this runner (/bin/bash: bun: command not found). Suggested coverage: start Gemini with an explicit model, call set-session-config with model: null, then verify the next launcher restart uses env/local/default resolution.
HAPI Bot
cli/src/gemini/runGemini.ts
Outdated
|
|
||
| if (config.model !== undefined) { | ||
| sessionModel = resolveModel(config.model); | ||
| resolvedModel = sessionModel ?? runtimeConfig.model; |
There was a problem hiding this comment.
[MAJOR] Selecting Default here does not restore the machine default if the session was started or resumed with an explicit Gemini model. runtimeConfig was created from opts.model, so sessionModel = null falls back to that same explicit startup value, and geminiLoop() still uses it on the next local/remote relaunch via opts.model (cli/src/gemini/loop.ts:49-52, cli/src/gemini/utils/config.ts:97-105).
Suggested fix:
const machineDefaultModel = resolveGeminiRuntimeConfig().model
let resolvedModel = sessionModel ?? machineDefaultModel
if (config.model !== undefined) {
sessionModel = resolveModel(config.model)
resolvedModel = sessionModel ?? machineDefaultModel
}
await geminiLoop({
// ...
model: machineDefaultModel,
})The resolved default model depends on env/local config which the web cannot read. Showing a specific model name would be misleading when the actual default differs from the hardcoded label.
…p model When a session started with an explicit model, selecting Default fell back to that startup model instead of the machine default. Now calls resolveGeminiRuntimeConfig() without args to obtain the true machine default (env > local config > hardcoded). Also use plain "Default" label since the web cannot know the resolved machine default.
69d9399 to
70d5743
Compare
There was a problem hiding this comment.
Findings
- [Major] Gemini
Defaultstill relaunches with the stale startup model.runGemini()now recomputesresolvedModelcorrectly whenmodel: nullarrives, butgeminiLoop()still falls back to its originalopts.modelsnapshot whensession.getModel()isnull, so the next local/remote launcher restart reuses the startup model instead of the machine default. Evidencecli/src/gemini/loop.ts:49, contextcli/src/gemini/runGemini.ts:153.
Suggested fix:await geminiLoop({ // ... getDefaultModel: () => resolvedModel, }) const getCurrentModel = (): string | undefined => { const sessionModel = session.getModel() return sessionModel != null ? sessionModel : opts.getDefaultModel() }
Summary
Review mode: follow-up after new commits. One major correctness issue remains: selecting Gemini Default still does not update the model used for the next launcher restart, because geminiLoop() keeps falling back to its startup opts.model snapshot. No other high-confidence issues found in the updated diff.
Testing
Not run (automation): bun not installed in this runner. Suggested coverage: change Gemini to an explicit model, switch back to Default, force a local/remote relaunch, and assert the launcher uses the machine default rather than the startup model.
HAPI Bot
|
|
||
| const getCurrentModel = (): string | undefined => { | ||
| const sessionModel = session.getModel(); | ||
| return sessionModel != null ? sessionModel : opts.model; |
There was a problem hiding this comment.
[MAJOR] opts.model is still the startup snapshot here. After set-session-config({ model: null }), runGemini() updates resolvedModel, but session.getModel() becomes null, so the next relaunch falls back to this stale value instead of the machine default (cli/src/gemini/runGemini.ts:153). Selecting Default after an explicit Gemini model therefore still restarts with the old startup model.
Suggested fix:
await geminiLoop({
// ...
getDefaultModel: () => resolvedModel,
})
const getCurrentModel = (): string | undefined => {
const sessionModel = session.getModel()
return sessionModel != null ? sessionModel : opts.getDefaultModel()
}… model geminiLoop receives machineDefault (from resolveGeminiRuntimeConfig() without args) instead of resolvedModel. This ensures getCurrentModel() in loop.ts falls back to the machine default when session model is null (Default selected), not the explicit model the session was started with.
There was a problem hiding this comment.
Findings
- No high-confidence issues found in the current diff.
Summary
Review mode: follow-up after new commits. No diff-introduced correctness, security, or maintainability regressions found on the latest head. Residual risk/testing gap: Not run in this runner because bun is unavailable, and I did not find route-level coverage added for the new Gemini POST /api/sessions/:id/model path in hub/src/web/routes/sessions.test.ts.
Testing
Not run (automation): bun not installed in this runner. Suggested coverage: add a hub route test for Gemini POST /api/sessions/:id/model, plus an integration path that switches Gemini from an explicit model back to Default and verifies the next launcher restart uses the machine default.
HAPI Bot
Summary
shared/src/modes.tsas a single source of truth (GEMINI_MODEL_LABELS→GEMINI_MODEL_PRESETSderived viaObject.keys(),DEFAULT_GEMINI_MODEL)supportsModelChange()flavor capability function to replace hardcodedisClaudeFlavor()checksMotivation
Gemini sessions have no way to change models mid-conversation — the model change UI, API endpoint, and RPC handler are all gated to Claude-only. Additionally, Gemini model definitions are spread across three locations (
config.ts,types.ts,AcpStdioTransport.ts).This PR consolidates model definitions and extends model selection support to Gemini sessions, consistent with the existing Claude implementation.
Related issues: #302, #221, #326, #123
Design notes
--modelis a process start argument) and matches the Claude behavior.Test plan
runGemini.test.ts: model change via RPC — success, null (Default), validation error, and selective response cases (6 tests)modelOptions.test.ts: flavor-based model option generation for both Claude and Gemini (4 tests)claudeModelOptions.test.tspasses (no regression)POST /model→ 200 confirmed