fix(tui): start the session a sessionless model switch defers into
Launching kigi in a non-project directory (~/Downloads, ~, /tmp — anywhere
`is_project_dir` rejects) leaves the agent view session-less behind the
project-picker question, which only a PLAIN prompt can open. Slash
commands still execute there, so `/model <name> <effort>` stashed its
switch in `deferred_model_switch` — a stash that assumes a create is in
flight — and with none pending it dangled forever with zero feedback:
the picker rendered, the user chose model and effort, and nothing
changed. Not a 0.1.10 regression: 0.1.9 pty-reproduces identically; the
report correlated with the upgrade only because every earlier launch
happened to be from a project directory.
Apply the QueueCommand precedent ("queued slash work bypasses the
picker, so create the deferred session or it never drains") to both
deferral sites — Action::SwitchModel in the router and
set_default_model's no-session branch — via
skip_picker_and_create_session, whose in-flight guard already makes it a
no-op while a create is pending, so the racing-create case is unchanged.
SessionCreated then applies the stash through the existing
apply_deferred_model_switch path; pty-verified end-to-end from
~/Downloads (session created, model changed, effort applied).
This commit is contained in:
@@ -62,6 +62,7 @@ use super::session::lifecycle::{
|
|||||||
clear_startup_actions, dispatch_agent_type_mismatch_answered, dispatch_exit_session,
|
clear_startup_actions, dispatch_agent_type_mismatch_answered, dispatch_exit_session,
|
||||||
dispatch_new_session, dispatch_new_session_inner, dispatch_new_session_with_id,
|
dispatch_new_session, dispatch_new_session_inner, dispatch_new_session_with_id,
|
||||||
dispatch_new_worktree_session, dispatch_trust_folder, open_new_session_question,
|
dispatch_new_worktree_session, dispatch_trust_folder, open_new_session_question,
|
||||||
|
skip_picker_and_create_session,
|
||||||
};
|
};
|
||||||
use super::session::load::{
|
use super::session::load::{
|
||||||
dispatch_cycle_session_source_filter, dispatch_load_session, dispatch_pick_content_session,
|
dispatch_cycle_session_source_filter, dispatch_load_session, dispatch_pick_content_session,
|
||||||
@@ -749,7 +750,12 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec<Effect> {
|
|||||||
};
|
};
|
||||||
let Some(session_id) = agent.session.session_id.clone() else {
|
let Some(session_id) = agent.session.session_id.clone() else {
|
||||||
agent.session.deferred_model_switch = Some((model_id, effort));
|
agent.session.deferred_model_switch = Some((model_id, effort));
|
||||||
return vec![];
|
// No session bound: with a create in flight this is a no-op
|
||||||
|
// and `SessionCreated` applies the stash; with none in flight
|
||||||
|
// (project question pending — only a plain prompt opens it) a
|
||||||
|
// switch would dangle forever, so start the session like the
|
||||||
|
// QueueCommand arm does for queued slash work.
|
||||||
|
return skip_picker_and_create_session(app, id);
|
||||||
};
|
};
|
||||||
agent.session.model_switch_pending = true;
|
agent.session.model_switch_pending = true;
|
||||||
vec![Effect::SwitchModel {
|
vec![Effect::SwitchModel {
|
||||||
|
|||||||
@@ -1519,12 +1519,20 @@ pub(in crate::app::dispatch) fn set_default_model(
|
|||||||
effort: None,
|
effort: None,
|
||||||
prev_model_id: prev_id.clone(),
|
prev_model_id: prev_id.clone(),
|
||||||
});
|
});
|
||||||
} else if let Some(agent) = app.agents.get_mut(&aid) {
|
} else {
|
||||||
// No session id yet — stash for
|
if let Some(agent) = app.agents.get_mut(&aid) {
|
||||||
// `EventLoop::on_session_created` to apply once the session
|
// No session id yet — stash for
|
||||||
// id materialises. Mirrors the deferred-switch handling in
|
// `EventLoop::on_session_created` to apply once the session
|
||||||
// `Action::SwitchModel`.
|
// id materialises. Mirrors the deferred-switch handling in
|
||||||
agent.session.deferred_model_switch = Some((new_id, None));
|
// `Action::SwitchModel`.
|
||||||
|
agent.session.deferred_model_switch = Some((new_id, None));
|
||||||
|
}
|
||||||
|
// With no create in flight (project question pending), the stash
|
||||||
|
// would dangle forever — start the session it drains into. No-op
|
||||||
|
// when a create is already pending.
|
||||||
|
effects.extend(
|
||||||
|
crate::app::dispatch::session::lifecycle::skip_picker_and_create_session(app, aid),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
effects
|
effects
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -481,7 +481,7 @@ fn session_failed_clears_flag_no_fetches() {
|
|||||||
assert!(!app.agents[&id].pending_extensions_fetch);
|
assert!(!app.agents[&id].pending_extensions_fetch);
|
||||||
}
|
}
|
||||||
#[test]
|
#[test]
|
||||||
fn switch_model_without_session_does_nothing() {
|
fn switch_model_without_session_starts_the_session() {
|
||||||
let mut app = test_app_with_agent();
|
let mut app = test_app_with_agent();
|
||||||
let id = AgentId(0);
|
let id = AgentId(0);
|
||||||
app.agents.get_mut(&id).unwrap().session.session_id = None;
|
app.agents.get_mut(&id).unwrap().session.session_id = None;
|
||||||
@@ -493,7 +493,14 @@ fn switch_model_without_session_does_nothing() {
|
|||||||
},
|
},
|
||||||
&mut app,
|
&mut app,
|
||||||
);
|
);
|
||||||
assert!(effects.is_empty());
|
// No create was in flight, so the switch starts the session its stash
|
||||||
|
// drains into (the stash alone dangled forever — the /model-in-Downloads
|
||||||
|
// silent no-op). `model_switch_pending` flips on SessionCreated.
|
||||||
|
assert!(
|
||||||
|
effects
|
||||||
|
.iter()
|
||||||
|
.any(|e| matches!(e, Effect::CreateSession { .. }))
|
||||||
|
);
|
||||||
assert!(!app.agents[&id].session.model_switch_pending);
|
assert!(!app.agents[&id].session.model_switch_pending);
|
||||||
}
|
}
|
||||||
#[test]
|
#[test]
|
||||||
@@ -668,11 +675,17 @@ fn switch_model_deferred_when_no_session_id() {
|
|||||||
},
|
},
|
||||||
&mut app,
|
&mut app,
|
||||||
);
|
);
|
||||||
assert!(effects.is_empty());
|
// Stashed for SessionCreated — and the session it drains into is started
|
||||||
|
// (no create was in flight; a bare stash never drained).
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
app.agents[&id].session.deferred_model_switch,
|
app.agents[&id].session.deferred_model_switch,
|
||||||
Some((model_id, None))
|
Some((model_id, None))
|
||||||
);
|
);
|
||||||
|
assert!(
|
||||||
|
effects
|
||||||
|
.iter()
|
||||||
|
.any(|e| matches!(e, Effect::CreateSession { .. }))
|
||||||
|
);
|
||||||
assert!(!app.agents[&id].session.model_switch_pending);
|
assert!(!app.agents[&id].session.model_switch_pending);
|
||||||
}
|
}
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -977,6 +977,77 @@ fn switch_model_pending_lifecycle() {
|
|||||||
assert!(!app.agents[&id].session.model_switch_pending);
|
assert!(!app.agents[&id].session.model_switch_pending);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A model switch with no session AND no create in flight (the project-picker
|
||||||
|
/// question is pending; only a plain prompt can open it) must start the
|
||||||
|
/// deferred session itself, or the stashed switch dangles forever with zero
|
||||||
|
/// feedback — `/model X eff` in `~/Downloads` looked like "the model never
|
||||||
|
/// changes". Mirrors the `QueueCommand` arm: queued slash work bypasses the
|
||||||
|
/// picker and creates the session so the stash drains.
|
||||||
|
#[test]
|
||||||
|
fn switch_model_without_session_creates_the_deferred_session() {
|
||||||
|
let mut app = test_app_with_agent();
|
||||||
|
let id = AgentId(0);
|
||||||
|
app.agents.get_mut(&id).unwrap().session.session_id = None;
|
||||||
|
// Harness cwd is `/tmp` (a non-project dir); arm the picker gate the way
|
||||||
|
// startup leaves it (the harness pre-marks it shown for other tests).
|
||||||
|
app.project_picker_shown = false;
|
||||||
|
assert!(app.needs_project_picker());
|
||||||
|
|
||||||
|
let model_id = acp::ModelId::new(std::sync::Arc::from("kigi-4.5"));
|
||||||
|
let effects = dispatch(
|
||||||
|
Action::SwitchModel {
|
||||||
|
model_id: model_id.clone(),
|
||||||
|
effort: None,
|
||||||
|
},
|
||||||
|
&mut app,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
app.agents[&id].session.deferred_model_switch,
|
||||||
|
Some((model_id, None)),
|
||||||
|
"switch must stay stashed for SessionCreated to apply"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
effects
|
||||||
|
.iter()
|
||||||
|
.any(|e| matches!(e, Effect::CreateSession { .. })),
|
||||||
|
"sessionless switch must start the session the stash drains into"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Same stash path while a create IS in flight (`mcp_init_progress` set):
|
||||||
|
/// no duplicate `CreateSession` — the pending create applies the stash.
|
||||||
|
#[test]
|
||||||
|
fn switch_model_with_create_in_flight_does_not_duplicate_create() {
|
||||||
|
let mut app = test_app_with_agent();
|
||||||
|
let id = AgentId(0);
|
||||||
|
{
|
||||||
|
let agent = app.agents.get_mut(&id).unwrap();
|
||||||
|
agent.session.session_id = None;
|
||||||
|
agent.mcp_init_progress = Some(crate::app::agent_view::McpInitProgress {
|
||||||
|
total: 0,
|
||||||
|
connected: 0,
|
||||||
|
started_at: std::time::Instant::now(),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
let effects = dispatch(
|
||||||
|
Action::SwitchModel {
|
||||||
|
model_id: acp::ModelId::new(std::sync::Arc::from("kigi-4.5")),
|
||||||
|
effort: None,
|
||||||
|
},
|
||||||
|
&mut app,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(app.agents[&id].session.deferred_model_switch.is_some());
|
||||||
|
assert!(
|
||||||
|
!effects
|
||||||
|
.iter()
|
||||||
|
.any(|e| matches!(e, Effect::CreateSession { .. })),
|
||||||
|
"an in-flight create must not be duplicated"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn no_deferred_switch_means_no_extra_effect() {
|
fn no_deferred_switch_means_no_extra_effect() {
|
||||||
// When there is no deferred model switch, SessionCreated should
|
// When there is no deferred model switch, SessionCreated should
|
||||||
|
|||||||
Reference in New Issue
Block a user