From ad7bd7d309064eebf8070a6655f69354f2673ac5 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sat, 2 May 2026 15:27:39 +0000 Subject: [PATCH] =?UTF-8?q?review(p9-3):=20=ED=9A=8C=EC=B0=A8=201=20?= =?UTF-8?q?=EC=A7=80=EC=A0=81=20=EB=B0=98=EC=98=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- crates/kebab-tui/src/ask.rs | 23 +++++++++++++++++++++-- crates/kebab-tui/tests/ask.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/crates/kebab-tui/src/ask.rs b/crates/kebab-tui/src/ask.rs index e3c62bb..ccc034e 100644 --- a/crates/kebab-tui/src/ask.rs +++ b/crates/kebab-tui/src/ask.rs @@ -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) { 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![ Span::styled("? ", Style::default().fg(Color::Cyan)), 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) } (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 .ask .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) { return KeyOutcome::Continue; diff --git a/crates/kebab-tui/tests/ask.rs b/crates/kebab-tui/tests/ask.rs index 3f17082..e2d6c44 100644 --- a/crates/kebab-tui/tests/ask.rs +++ b/crates/kebab-tui/tests/ask.rs @@ -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] fn no_ask_state_returns_to_library() { let mut config = Config::defaults();