Move ModelSupportsReasoning calls to async bubbletea commands#1749
Move ModelSupportsReasoning calls to async bubbletea commands#1749dgageot merged 2 commits intodocker:mainfrom
Conversation
The modelsdev.ModelSupportsReasoning function can block for up to 30s on a cold cache (HTTP fetch to models.dev). Calling it synchronously in Update() froze the TUI. - sidebar.SetAgentInfo now returns tea.Cmd; the reasoning check runs in a goroutine and reports back via reasoningSupportResultMsg - handleToggleThinking fires an async command and handles the result in a separate handleToggleThinkingResult method - All call sites (runtime_events.go, tui.go) updated to propagate the returned commands Assisted-By: cagent
Assisted-By: cagent
There was a problem hiding this comment.
Review Summary
Found 2 medium-severity issues in the async command handling:
- Session state race condition - The thinking toggle may be applied to the wrong session if the user switches sessions while the async check is in progress
- Stale reasoning support status - When switching agents, the old agent's reasoning support status is displayed until the new async check completes
Both issues are related to state consistency when async operations complete after the user has changed context (session or agent).
| return a, notification.InfoCmd("Thinking/reasoning is not supported for the current model") | ||
| } | ||
|
|
||
| sess := a.application.Session() |
There was a problem hiding this comment.
Session state race condition
The handleToggleThinkingResult function gets the current session without verifying it's the same session that initiated the thinking toggle. If a user:
- Toggles thinking on Session A
- Quickly switches to Session B before the async check completes
- The async result arrives and toggles thinking on Session B instead of Session A
This causes the thinking flag to be applied to the wrong session.
Suggested fix: Capture the session ID when starting the async check and include it in ToggleThinkingResultMsg. Then verify the session ID matches before applying the toggle:
func (a *appModel) handleToggleThinking() (tea.Model, tea.Cmd) {
// ...
sessionID := a.application.Session().ID
return a, func() tea.Msg {
supported := modelsdev.ModelSupportsReasoning(ctx, currentModel)
return messages.ToggleThinkingResultMsg{
Supported: supported,
SessionID: sessionID,
}
}
}
func (a *appModel) handleToggleThinkingResult(msg messages.ToggleThinkingResultMsg) (tea.Model, tea.Cmd) {
sess := a.application.Session()
if sess.ID != msg.SessionID {
return a, nil // Session changed, discard result
}
// ... rest of logic
}
``` <!-- cagent-review -->| case *runtime.AgentInfoEvent: | ||
| m.SetAgentInfo(msg.AgentName, msg.Model, msg.Description) | ||
| case reasoningSupportResultMsg: | ||
| if msg.modelID == m.agentModel { |
There was a problem hiding this comment.
Stale reasoning support status displayed when switching agents
The guard if msg.modelID == m.agentModel correctly prevents stale async results from updating the wrong agent's state. However, when SetAgentInfo is called with a new agent, the reasoningSupported flag retains the value from the previous agent until the new async check completes.
Scenario:
- Agent A does not support reasoning →
reasoningSupported = false - Switch to Agent B (which supports reasoning)
- Until the async check completes, the UI shows reasoning as not supported (stale value from Agent A)
- The initial default is
true(fail-open), so we should reset to this default
Suggested fix: Reset reasoningSupported to the fail-open default when setting a new agent:
func (m *model) SetAgentInfo(agentName, modelID, description string) tea.Cmd {
m.currentAgent = agentName
m.agentModel = modelID
m.agentDescription = description
m.reasoningSupported = true // Reset to fail-open default
if m.cancelReasoningCheck != nil {
m.cancelReasoningCheck()
}
ctx, cancel := context.WithCancel(context.Background())
m.cancelReasoningCheck = cancel
// ... rest of function
}
``` <!-- cagent-review -->
The modelsdev.ModelSupportsReasoning function can block for up to 30s on
a cold cache (HTTP fetch to models.dev). Calling it synchronously in
Update() froze the TUI.
a goroutine and reports back via reasoningSupportResultMsg
a separate handleToggleThinkingResult method
returned commands
Also update AGENTS.md to steer agents into the right direction for the TUI.