review(p9-3): 회차 1 지적 반영
Esc 후 재질문 시 detached prior worker + 새 worker 동시 in-flight 가능 했음. Ollama endpoint 에 두 요청 동시 발사 → 응답 시간 두 배 + stream 혼동. spawn_ask_worker 진입 시 `s.thread.is_some()` 검사 추가, 이전 worker 가 still alive 면 Enter 무시. input bar 의 busy 텍스트 가 세 상태 (streaming / awaiting prior / idle) 분리 표시 — 사용자가 Enter 가 왜 안 먹히는지 즉시 확인. 회귀 테스트 `enter_with_detached_prior_thread_is_blocked` 추가 — never- ending 더미 thread 를 hand-install 후 Enter no-op 검증, 종료 시 thread take() 로 leak 명시 (test process 종료 시 OS 가 reap). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -51,7 +51,18 @@ pub fn render_ask(f: &mut Frame, area: Rect, state: &App) {
|
|||||||
|
|
||||||
fn render_input(f: &mut Frame, area: Rect, s: &AskState) {
|
fn render_input(f: &mut Frame, area: Rect, s: &AskState) {
|
||||||
let mode_badge = if s.explain { " explain" } else { "" };
|
let mode_badge = if s.explain { " explain" } else { "" };
|
||||||
let busy = if s.streaming { " streaming…" } else { "" };
|
// Distinguish three async states for the operator:
|
||||||
|
// - currently streaming (worker still emitting tokens)
|
||||||
|
// - prior worker detached (Esc-cancelled, no rx attached but
|
||||||
|
// thread has not finished yet — Enter is blocked until it ends)
|
||||||
|
// - idle
|
||||||
|
let busy = if s.streaming {
|
||||||
|
" streaming…"
|
||||||
|
} else if s.thread.is_some() {
|
||||||
|
" awaiting prior answer (Enter blocked)"
|
||||||
|
} else {
|
||||||
|
""
|
||||||
|
};
|
||||||
let line = Line::from(vec![
|
let line = Line::from(vec![
|
||||||
Span::styled("? ", Style::default().fg(Color::Cyan)),
|
Span::styled("? ", Style::default().fg(Color::Cyan)),
|
||||||
Span::raw(s.input.as_str()),
|
Span::raw(s.input.as_str()),
|
||||||
@@ -200,10 +211,18 @@ pub fn handle_key_ask(state: &mut App, key: KeyEvent) -> KeyOutcome {
|
|||||||
KeyOutcome::SwitchPane(Pane::Library)
|
KeyOutcome::SwitchPane(Pane::Library)
|
||||||
}
|
}
|
||||||
(KeyCode::Enter, _) => {
|
(KeyCode::Enter, _) => {
|
||||||
|
// Submission gates:
|
||||||
|
// - empty input → no-op
|
||||||
|
// - already streaming → no-op (same worker is in flight)
|
||||||
|
// - prior worker still attached (e.g. user pressed Esc
|
||||||
|
// then re-entered Ask before that thread finished) →
|
||||||
|
// no-op. Otherwise the new worker would race the
|
||||||
|
// detached one against the same Ollama endpoint and
|
||||||
|
// the stream output would interleave.
|
||||||
if state
|
if state
|
||||||
.ask
|
.ask
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.map(|s| s.streaming || s.input.trim().is_empty())
|
.map(|s| s.streaming || s.thread.is_some() || s.input.trim().is_empty())
|
||||||
.unwrap_or(true)
|
.unwrap_or(true)
|
||||||
{
|
{
|
||||||
return KeyOutcome::Continue;
|
return KeyOutcome::Continue;
|
||||||
|
|||||||
@@ -328,6 +328,41 @@ fn explain_toggle_changes_panel_title() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn enter_with_detached_prior_thread_is_blocked() {
|
||||||
|
// R1 fix: after Esc, the prior worker is detached (thread still
|
||||||
|
// running, rx cleared, streaming=false). A new Enter must NOT
|
||||||
|
// spawn a second worker against the same Ollama endpoint until
|
||||||
|
// the prior thread finishes.
|
||||||
|
let mut app = fresh_app();
|
||||||
|
{
|
||||||
|
let s = app.ask.as_mut().unwrap();
|
||||||
|
s.input = "another question".into();
|
||||||
|
s.streaming = false;
|
||||||
|
// Simulate a detached prior worker by hand-installing a
|
||||||
|
// never-ending JoinHandle. (We can't easily make a sleeping
|
||||||
|
// thread without timing flakiness; an empty-loop shim works.)
|
||||||
|
s.thread = Some(std::thread::spawn(|| {
|
||||||
|
// Loop until the test drops the JoinHandle's owner via
|
||||||
|
// App going out of scope. is_finished() will report
|
||||||
|
// false until then.
|
||||||
|
loop {
|
||||||
|
std::thread::sleep(std::time::Duration::from_millis(100));
|
||||||
|
}
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
let outcome = handle_key_ask(
|
||||||
|
&mut app,
|
||||||
|
KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE),
|
||||||
|
);
|
||||||
|
// Enter is a no-op while a prior thread is attached.
|
||||||
|
assert_eq!(outcome, KeyOutcome::Continue);
|
||||||
|
let s = app.ask.as_ref().unwrap();
|
||||||
|
assert!(!s.streaming, "no second worker spawned");
|
||||||
|
// Detach so the never-ending thread can be reaped on test exit.
|
||||||
|
let _leaked = app.ask.as_mut().unwrap().thread.take();
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn no_ask_state_returns_to_library() {
|
fn no_ask_state_returns_to_library() {
|
||||||
let mut config = Config::defaults();
|
let mut config = Config::defaults();
|
||||||
|
|||||||
Reference in New Issue
Block a user