diff --git a/HANDOFF.md b/HANDOFF.md index c2c4b57..ef3c55b 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -48,6 +48,7 @@ P0~P5 직렬. P6~P9 P5 이후 병렬 가능. - **2026-05-02 P9 도그푸딩 후속 (p9-fb-16)** — TUI Ask conversation UI. `AskState` 가 `turns: Vec` + `current_question` + `conversation_id` + `last_answer` 로 재설계. answer area 가 transcript (`Q1/A1`, `Q2/A2`, ...) 로 갈음, 매 Enter 가 이전 turns 를 `history` 로 worker 에 전달 (`ask_with_history`). conversation_id 는 첫 submit 시 timestamp-based 자동 생성 (`conv_`). `Ctrl-L` 가 turns + conversation_id 초기화 (in-flight worker 는 그대로 finish, 결과는 새 conversation 의 stale turn 으로 silently 폐기). spec: `tasks/p9/p9-fb-16-tui-ask-conversation.md`. - **2026-05-03 P9 도그푸딩 후속 (p9-fb-20)** — `kebab ask` 의 CLI citation block. 답변 출력 후 `근거:` 절 — `[N] # (score=)` 한 줄씩. `--show-citations` (default ON) / `--hide-citations` (pipe 시 답변 본문만) flag. `--json` 모드는 무영향 (citations 가 항상 wire payload 에 포함). spec p9-fb-20 의 \"TUI citation pane + jump\" 부분은 P9-3 의 기존 `render_citations_or_explain` 가 일부 cover — 추가 기능 (turn 별 fold + Enter/o jump + i inspect) 은 후속 task 로 미룸 (사용자 도그푸딩 priority 5위 의 핵심 = full path 가독성 = CLI block 으로 충족). spec: `tasks/p9/p9-fb-20-citation-surface.md`. - **2026-05-03 P9 도그푸딩 후속 (p9-fb-07)** — Markdown title fallback chain. `kebab-normalize::derive_title(frontmatter_title, &[Block], file_stem)` — 1) frontmatter title → 2) 첫 H1 → 3) 첫 H2 → 4) 첫 paragraph 80 chars → 5) 파일 stem (모든 단계 NFC 정규화, 빈 문자열 절대 반환 안 함, 마지막 sentinel `"untitled"`). `build_canonical_document` 가 lift 후 helper 호출. parser_version 상수 `pulldown-cmark-0.x` → `md-frontmatter-v2` bump — 기존 doc 은 `doc_id` 가 갱신되므로 다음 ingest 가 자동 재처리 (idempotent upsert, design §9 cascade). spec: `tasks/p9/p9-fb-07-md-title-fallback.md`. +- **2026-05-03 P9 도그푸딩 후속 (p9-fb-09)** — TUI external editor return restore. Search `g` 키 (citation jump) 후 TUI 화면이 깨지는 버그 수정. `kebab-tui::editor::with_external_program(&mut TuiTerminal, Command)` helper 가 suspend (LeaveAlternateScreen + Show cursor + disable_raw_mode) → spawn → restore (enable_raw_mode + EnterAlternateScreen + Hide cursor + `terminal.clear()`) 시퀀스를 RAII guard 로 atomic 하게 묶음. `App.pending_editor: Option` + `App.force_redraw: bool` 추가 — 키 핸들러는 EditorRequest enqueue 만, 실제 spawn 은 run loop 가 `TuiTerminal` 핸들 들고 처리. 후속 task (p9-fb-20 의 citation jump 등) 가 같은 helper 위에 build. spec: `tasks/p9/p9-fb-09-tui-editor-restore.md`. ## 다음 task 후보 diff --git a/README.md b/README.md index 459daeb..3e1476f 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ kebab doctor | `kebab inspect doc ` / `kebab inspect chunk ` | raw record 보기 | | `kebab ask "" [--show-citations / --hide-citations]` | RAG 답변 + 근거 인용. 답변 후 `근거:` block 으로 full path / line range / score 한 줄씩 (default ON — `--hide-citations` 로 끄기, pipe 시 유용). 근거 부족 시 거절. Ollama 필요 | | `kebab doctor` | 설정/모델/DB 헬스 체크 | -| `kebab tui` | Ratatui 셸 (Library + Search + Ask + Inspect 패널, desktop 진행 중). Library 에서 `r` 키로 background ingest 시작 — 화면 하단 status bar 가 진행 표시, 완료/abort 시 final 라인 잠시 유지 후 자동 hide. ingest 진행 중 `Esc` / `Ctrl-C` 가 cancel signal (그 외에는 quit). Ask 패널은 multi-turn — 같은 conversation 안에서 Q1/A1, Q2/A2 transcript 누적, 다음 질문이 이전 턴을 history 로 받아 답변. `Ctrl-L` 로 새 conversation 시작 | +| `kebab tui` | Ratatui 셸 (Library + Search + Ask + Inspect 패널, desktop 진행 중). Library 에서 `r` 키로 background ingest 시작 — 화면 하단 status bar 가 진행 표시, 완료/abort 시 final 라인 잠시 유지 후 자동 hide. ingest 진행 중 `Esc` / `Ctrl-C` 가 cancel signal (그 외에는 quit). Ask 패널은 multi-turn — 같은 conversation 안에서 Q1/A1, Q2/A2 transcript 누적, 다음 질문이 이전 턴을 history 로 받아 답변. `Ctrl-L` 로 새 conversation 시작. Search 의 `g` 키가 `$EDITOR` (기본 `vi`) 로 hit 의 citation 위치 열기 — 종료 후 TUI 화면이 자동으로 깨끗이 redraw | | `kebab reset [--all / --data-only / --vector-only / --config-only] [--yes]` | XDG 데이터 wipe. **Irreversible.** TTY 면 confirm prompt, 아니면 `--yes` 필수. `--vector-only` 는 SQLite `embedding_records` 도 함께 truncate (orphan 방지) | | `kebab eval run / compare` | golden query 회귀 측정 | diff --git a/crates/kebab-tui/src/app.rs b/crates/kebab-tui/src/app.rs index 117e2ba..43497fe 100644 --- a/crates/kebab-tui/src/app.rs +++ b/crates/kebab-tui/src/app.rs @@ -259,6 +259,35 @@ pub struct App { /// or by a future pane's quit key. The run loop drains this on /// each tick. pub(crate) should_quit: bool, + /// p9-fb-09: deferred external-program request. A pane's key + /// handler enqueues an `EditorRequest` here when the user wants + /// to spawn `$EDITOR` (e.g. Search `g` jumps to a citation in + /// vim) — the actual suspend / spawn / restore happens in the + /// run loop, where the `TuiTerminal` handle is in scope. + /// Drained every tick after the key dispatch. + /// + /// `pub(crate)` because the enqueue/take invariant ("set by a + /// key handler, drained by the next run-loop tick") only holds + /// for in-crate callers; external mutation could leave a stale + /// request that never gets serviced. + pub(crate) pending_editor: Option, + /// p9-fb-09: when set, the next run-loop draw runs + /// `terminal.clear()` first so any leftover screen content from + /// a suspension (post-editor, future config-reload, …) is wiped + /// before Ratatui's diff renders the new frame. Reset back to + /// false after the clear. Independent of `pending_editor` — + /// any future code path that needs a forced redraw can flip + /// this flag. + pub(crate) force_redraw: bool, +} + +/// p9-fb-09: external-program spawn request. Posted by a pane's key +/// handler, serviced by the run loop on the next tick. +#[derive(Clone, Debug)] +pub struct EditorRequest { + pub citation: kebab_core::Citation, + pub editor_env: String, + pub workspace_root: std::path::PathBuf, } impl App { @@ -276,9 +305,20 @@ impl App { ingest_state: None, error_overlay: None, should_quit: false, + pending_editor: None, + force_redraw: false, }) } + /// Read-only accessor for the in-flight external-program request. + /// Tests and future external observers (e.g. integration smokes) + /// use this to assert that a key dispatch enqueued a spawn — + /// mutating the slot stays `pub(crate)` to preserve the + /// "set-then-drained-on-next-tick" invariant. + pub fn pending_editor(&self) -> Option<&EditorRequest> { + self.pending_editor.as_ref() + } + /// Blocking event loop. Returns when the user quits or a fatal /// error escapes the loop (terminal raw-mode is restored either /// way via the `Terminal` Drop guard). diff --git a/crates/kebab-tui/src/editor.rs b/crates/kebab-tui/src/editor.rs new file mode 100644 index 0000000..b60fa63 --- /dev/null +++ b/crates/kebab-tui/src/editor.rs @@ -0,0 +1,132 @@ +//! p9-fb-09: external-program suspend/restore helper. +//! +//! Spawning `$EDITOR` (or any other foreground child) from the TUI +//! requires a careful dance: leave the alternate screen, drop raw +//! mode, hand the terminal to the child, then on return re-enter the +//! alternate screen, re-enable raw mode, AND clear the framebuffer so +//! Ratatui's next draw doesn't paint on top of stale text from before +//! the suspension. +//! +//! Earlier `kebab-tui::search::jump_to_citation` did the suspend half +//! correctly via a RAII guard but skipped the post-resume `clear()` — +//! the frame from before the editor stayed visible underneath the new +//! draw, producing the "TUI 화면이 깨짐" report (도그푸딩 item 7). +//! +//! `with_external_program` centralizes the dance so any future call +//! site (citation jump, `$VISUAL` invocation, etc.) inherits the fix +//! automatically. Callers pass the `Command` (already configured) and +//! get back the child's `ExitStatus` if the spawn succeeded. + +use std::process::{Command, ExitStatus}; + +use anyhow::{Context, Result}; +use crossterm::cursor::{Hide, Show}; +use crossterm::execute; +use crossterm::terminal::{ + EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode, +}; + +use crate::terminal::TuiTerminal; + +/// Suspend the TUI (leave alt screen, drop raw mode, show cursor), +/// run `cmd` to completion in the host terminal, then restore the +/// TUI (re-enter alt screen, re-enable raw mode, hide cursor) and +/// `clear()` the framebuffer so the next `draw` repaints from a +/// blank canvas instead of layering on top of stale glyphs. +/// +/// The restore happens via a RAII guard so a panic inside the child +/// spawn (or in this function before the explicit restore) still +/// puts the terminal back into raw + alternate-screen mode — the +/// shell would otherwise be left in a corrupt state. +/// +/// On success, returns the child's `ExitStatus`. The caller decides +/// whether a non-zero exit is an error (editor was cancelled vs. +/// crashed) — this helper only fails if the spawn itself fails. +pub(crate) fn with_external_program( + terminal: &mut TuiTerminal, + mut cmd: Command, +) -> Result { + suspend_tui()?; + + // RAII guard: regardless of how we leave (panic, error, normal + // return) the terminal goes back into raw + alt-screen mode and + // the framebuffer is cleared. + struct Restore<'a> { + terminal: &'a mut TuiTerminal, + } + impl<'a> Drop for Restore<'a> { + fn drop(&mut self) { + // Best-effort: errors here would clobber an in-flight + // panic if propagated. Match the conservative posture in + // `TuiTerminal::Drop` — log via `tracing` and continue. + if let Err(e) = resume_tui(self.terminal) { + tracing::error!(target: "kebab-tui", error = ?e, "TUI restore failed"); + } + } + } + let restore = Restore { terminal }; + + let status = cmd + .status() + .with_context(|| format!("spawn child program: {:?}", cmd.get_program()))?; + + drop(restore); + Ok(status) +} + +/// Leave the alternate screen, disable raw mode, and show the cursor +/// so a child process inherits a "normal" terminal. +fn suspend_tui() -> Result<()> { + let mut out = std::io::stdout(); + execute!(out, LeaveAlternateScreen, Show).context("crossterm: LeaveAlternateScreen + Show")?; + disable_raw_mode().context("crossterm: disable_raw_mode")?; + Ok(()) +} + +/// Re-enter the alternate screen, re-enable raw mode, hide the +/// cursor, and `terminal.clear()` so Ratatui draws a fresh frame +/// without inheriting whatever was on screen before the suspension. +fn resume_tui(terminal: &mut TuiTerminal) -> Result<()> { + enable_raw_mode().context("crossterm: enable_raw_mode")?; + let mut out = std::io::stdout(); + execute!(out, EnterAlternateScreen, Hide).context("crossterm: EnterAlternateScreen + Hide")?; + terminal + .inner + .clear() + .context("ratatui: terminal.clear after editor return")?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::process::Command; + + /// Sanity check on the OS layer that `with_external_program` + /// builds on top of: a missing program path makes `Command:: + /// status()` fail with `ENOENT`, which the helper wraps with + /// `with_context(|| format!("spawn child program: {:?}", ...))` + /// so the error chain points at the program name. + /// + /// We can't construct a `TuiTerminal` in a unit test (no real + /// terminal), so the helper end-to-end is verified by the + /// dogfooding loop in the spec rather than here. This test + /// only pins the OS behavior the helper assumes — if a future + /// libc / Rust update changes which `ErrorKind` is returned for + /// `ENOENT`, the helper's error message stays meaningful but + /// this test catches the platform regression first. + #[test] + fn command_status_returns_not_found_for_missing_program() { + let mut cmd = Command::new("/nonexistent/kebab-test-binary-xxx"); + cmd.arg("dummy-arg"); + let result = cmd.status(); + assert!(result.is_err(), "expected ENOENT-like failure"); + let err = result.unwrap_err(); + assert!( + matches!( + err.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied + ), + "unexpected error kind: {err:?}", + ); + } +} diff --git a/crates/kebab-tui/src/lib.rs b/crates/kebab-tui/src/lib.rs index d97880c..f7c36bf 100644 --- a/crates/kebab-tui/src/lib.rs +++ b/crates/kebab-tui/src/lib.rs @@ -14,6 +14,7 @@ mod app; mod ask; +mod editor; mod error_popup; mod ingest_progress; mod inspect; @@ -33,4 +34,9 @@ pub use ingest_progress::{ }; pub use inspect::{enter_inspect, handle_key_inspect, render_inspect}; pub use library::{handle_key_library, render_library}; -pub use search::{build_jump_command, handle_key_search, jump_to_citation, render_search}; +// `editor::with_external_program` and `search::jump_to_citation` +// stay `pub(crate)` — they take the internal `TuiTerminal` handle, +// which is intentionally module-private (its `Drop` lifecycle is the +// only safe constructor path for raw mode + alt-screen). External +// callers stage editor spawns via `App.pending_editor` instead. +pub use search::{build_jump_command, handle_key_search, render_search}; diff --git a/crates/kebab-tui/src/run.rs b/crates/kebab-tui/src/run.rs index 03d816d..ab98466 100644 --- a/crates/kebab-tui/src/run.rs +++ b/crates/kebab-tui/src/run.rs @@ -111,6 +111,16 @@ pub(crate) fn run_loop(app: &mut App) -> Result<()> { } } + // p9-fb-09: any code path (editor return, future reset + // helper, …) that toggled `force_redraw` gets a fresh + // framebuffer for this draw — without it, residual content + // from before the suspension would layer through Ratatui's + // diff and produce a corrupted-looking screen. + if app.force_redraw { + terminal.inner.clear()?; + app.force_redraw = false; + } + terminal.inner.draw(|f| render_root(f, app))?; if event::poll(POLL_INTERVAL)? { @@ -151,6 +161,24 @@ pub(crate) fn run_loop(app: &mut App) -> Result<()> { _ => {} } } + + // p9-fb-09: drain any pending external-program request that + // a key handler enqueued. The actual suspend / spawn / + // restore needs the `TuiTerminal` handle, which is only in + // scope here. After return, `force_redraw` is set so the + // next iteration's draw paints from a clean canvas. + if let Some(req) = app.pending_editor.take() { + let result = crate::search::jump_to_citation( + &mut terminal, + &req.citation, + &req.editor_env, + &req.workspace_root, + ); + app.force_redraw = true; + if let Err(e) = result { + app.error_overlay = Some(ErrorOverlay::from_anyhow(&e)); + } + } } Ok(()) diff --git a/crates/kebab-tui/src/search.rs b/crates/kebab-tui/src/search.rs index dfd7f93..9eb8869 100644 --- a/crates/kebab-tui/src/search.rs +++ b/crates/kebab-tui/src/search.rs @@ -27,7 +27,6 @@ use std::process::Command; use std::time::Duration; use crate::app::{App, KeyOutcome, Pane, SearchState}; -use crate::error_popup::ErrorOverlay; /// Debounce window after the last keystroke before re-searching. /// Matches the spec's 200 ms. @@ -223,15 +222,22 @@ pub fn handle_key_search(state: &mut App, key: KeyEvent) -> KeyOutcome { } }; if has_hits { + // p9-fb-09: enqueue the spawn for the run loop. Calling + // `jump_to_citation` directly here would not have access + // to the TuiTerminal handle, so the post-resume + // `terminal.clear()` couldn't happen — leaving the + // previous frame leaking through the new draw. let editor = std::env::var("EDITOR").unwrap_or_else(|_| "vi".into()); // `~/...` / `${XDG_…}` expansion via `kebab-config::expand_path` // — same helper used by the markdown / image / PDF ingest // paths (HOTFIXES 2026-05-02 P9-4 follow-up). let workspace_root = kebab_config::expand_path(&state.config.workspace.root, ""); - if let Err(e) = jump_to_citation(&citation.unwrap(), &editor, &workspace_root) { - state.error_overlay = Some(ErrorOverlay::from_anyhow(&e)); - } + state.pending_editor = Some(crate::app::EditorRequest { + citation: citation.unwrap(), + editor_env: editor, + workspace_root, + }); } return KeyOutcome::Continue; } @@ -384,40 +390,26 @@ pub fn build_jump_command( (program, args) } -/// Suspend the TUI raw mode, spawn `$EDITOR`, restore raw mode on -/// return. Errors propagate; raw-mode restore happens via a guard so -/// a panic during the editor child does not strand the user in a -/// corrupt terminal. -pub fn jump_to_citation( +/// Suspend the TUI, spawn `$EDITOR`, restore the TUI on return. +/// +/// p9-fb-09: delegates the suspend/restore dance to +/// [`crate::editor::with_external_program`] so the post-resume +/// `terminal.clear()` lands consistently — without it, the previous +/// frame leaked through the new draw and the user saw a corrupted +/// screen on return (도그푸딩 item 7). +/// +/// Errors propagate; the helper's RAII guard restores the terminal +/// even on panic. +pub(crate) fn jump_to_citation( + terminal: &mut crate::terminal::TuiTerminal, citation: &Citation, editor_env: &str, workspace_root: &Path, ) -> anyhow::Result<()> { - use crossterm::execute; - use crossterm::terminal::{ - EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode, - }; - let (program, args) = build_jump_command(citation, editor_env, workspace_root); - - // Suspend. - let _ = execute!(std::io::stdout(), LeaveAlternateScreen); - let _ = disable_raw_mode(); - - // RAII guard re-enters even on panic. - struct RawModeRestore; - impl Drop for RawModeRestore { - fn drop(&mut self) { - let _ = enable_raw_mode(); - let _ = execute!(std::io::stdout(), EnterAlternateScreen); - } - } - let _restore = RawModeRestore; - - let status = Command::new(&program) - .args(&args) - .status() - .map_err(|e| anyhow::anyhow!("spawn {program} failed: {e}"))?; + let mut cmd = Command::new(&program); + cmd.args(&args); + let status = crate::editor::with_external_program(terminal, cmd)?; if !status.success() { anyhow::bail!("{program} exited with {status:?}"); } diff --git a/crates/kebab-tui/tests/search.rs b/crates/kebab-tui/tests/search.rs index a24c808..358dc04 100644 --- a/crates/kebab-tui/tests/search.rs +++ b/crates/kebab-tui/tests/search.rs @@ -287,6 +287,53 @@ fn shift_g_does_not_trigger_editor_jump() { assert_eq!(app.search.as_ref().unwrap().input, "G"); } +/// p9-fb-09 — `g` on a hit enqueues an `EditorRequest` on `App.pending_editor` +/// rather than spawning the child synchronously. The run loop services the +/// queue with the `TuiTerminal` handle in scope so the post-resume +/// `terminal.clear()` can land (preventing the corrupted-redraw bug). +#[test] +fn g_key_enqueues_pending_editor_request() { + let mut app = fresh_app(); + { + let s = app.search.as_mut().unwrap(); + s.hits = vec![make_hit(1, "notes/x.md", "snippet", line_citation("notes/x.md", 42))]; + s.selected_hit = 0; + } + assert!(app.pending_editor().is_none(), "queue starts empty"); + let outcome = handle_key_search( + &mut app, + KeyEvent::new(KeyCode::Char('g'), KeyModifiers::NONE), + ); + assert_eq!(outcome, KeyOutcome::Continue); + let req = app + .pending_editor() + .expect("g on a hit must enqueue an EditorRequest"); + match &req.citation { + Citation::Line { path, start, .. } => { + assert_eq!(path.0, "notes/x.md"); + assert_eq!(*start, 42); + } + other => panic!("unexpected citation variant: {other:?}"), + } + // editor_env reads $EDITOR — fall back to "vi" for tests. + assert!(!req.editor_env.is_empty(), "editor_env must be populated"); +} + +/// p9-fb-09 — `g` with no hits is a no-op; the queue stays empty. +#[test] +fn g_key_with_no_hits_does_not_enqueue() { + let mut app = fresh_app(); + // Search slot present, hits empty. + let _outcome = handle_key_search( + &mut app, + KeyEvent::new(KeyCode::Char('g'), KeyModifiers::NONE), + ); + assert!( + app.pending_editor().is_none(), + "g with no hits must not enqueue" + ); +} + #[test] fn no_search_state_returns_to_library() { let mut config = Config::defaults(); diff --git a/tasks/p9/p9-fb-09-tui-editor-restore.md b/tasks/p9/p9-fb-09-tui-editor-restore.md index ff35401..a797110 100644 --- a/tasks/p9/p9-fb-09-tui-editor-restore.md +++ b/tasks/p9/p9-fb-09-tui-editor-restore.md @@ -3,7 +3,7 @@ phase: P9 component: kebab-tui task_id: p9-fb-09 title: "External editor return — terminal restore + force redraw" -status: planned +status: in_progress depends_on: [] unblocks: [] contract_source: ../../docs/superpowers/specs/2026-04-27-kebab-final-form-design.md