From 0337e3f7f347f95754828746d3eb2293e67f6e8d Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Fri, 26 Jun 2026 15:16:54 +0200 Subject: [PATCH] docs(rfd): Promote D34 to RFD 088 Promote the draft "Unified Editor Service and Inline Reply Widget" from `D34` to `RFD 088`, replacing the draft file with the full numbered RFD at `docs/rfd/088-unified-editor-service-and-inline-reply-widget.md`. The promoted RFD expands significantly on the draft: it adds a fourth editor-invocation path (path D, `jp conversation edit`), introduces `edit_file` alongside `edit_text` on `EditorBackend`, documents the vendored-reedline strategy to meet the RFD 048 `/dev/tty` requirement, adds the `editor.inline.edit_mode` config key, and pins the cancel/empty behavior matrix for menu-less configured-action paths. RFD 045 and RFD 048 are updated with back-references to RFD 088. Priority order moves `045` and `088` to the top and marks both alongside `087` as `in_development`. Signed-off-by: Jean Mertz --- docs/.vitepress/rfd-summaries.json | 8 +- .../045-layered-interrupt-handler-stack.md | 2 + docs/rfd/048-four-channel-output-model.md | 3 +- ...-editor-service-and-inline-reply-widget.md | 838 ++++++++++++++++++ ...-editor-service-and-inline-reply-widget.md | 559 ------------ docs/rfd/priority.json | 11 +- 6 files changed, 853 insertions(+), 568 deletions(-) create mode 100644 docs/rfd/088-unified-editor-service-and-inline-reply-widget.md delete mode 100644 docs/rfd/drafts/D34-unified-editor-service-and-inline-reply-widget.md diff --git a/docs/.vitepress/rfd-summaries.json b/docs/.vitepress/rfd-summaries.json index e2ef5cc0..4002f1b6 100644 --- a/docs/.vitepress/rfd-summaries.json +++ b/docs/.vitepress/rfd-summaries.json @@ -172,7 +172,7 @@ "summary": "Redesign `jp init` to generate schema-driven config with interactive model/mode selection and curated commented fields." }, "045-layered-interrupt-handler-stack.md": { - "hash": "360039e3b3d0bb397ee89854482b53db2232927bde8d86ae37236ded4f19c30b", + "hash": "15419eb7ab5a46ba0403e3fa03648d88a4fbe36a5f9c7a54bc41e4ba9baf6c7f", "summary": "Replace JP's ad-hoc interrupt handling with a layered LIFO handler stack, routing OS signals through scoped notification channels." }, "046-nested-workspace-projection.md": { @@ -184,7 +184,7 @@ "summary": "Add `jp conversation edit` (opens directory in $EDITOR) and `jp conversation path` (prints conversation filesystem path)." }, "048-four-channel-output-model.md": { - "hash": "8dcd19bd41d9813e9397c4d8a166b544d914c492cb4540eca258e4eedb356f00", + "hash": "815391741e98b5ac0d3e8be055c79cfcf275cbbcc3b780fe87672c59bb719f1b", "summary": "Separate JP output into four channels: stdout for responses, stderr for chrome, /dev/tty for prompts, log file for tracing." }, "049-non-interactive-mode-and-detached-prompt-policy.md": { @@ -346,5 +346,9 @@ "087-session-scoped-active-workspace.md": { "hash": "9501443d3fca0de19dab38f696f2865266e420fc3e455755697276f0e1444fee", "summary": "Session-scoped active workspace lets JP commands run from anywhere after selecting a workspace with `jp w use`." + }, + "088-unified-editor-service-and-inline-reply-widget.md": { + "hash": "ed23d4b3ef9ce5d00d623166c48e3b63b0dd13511aa1405a4f3958b9613affff", + "summary": "Consolidate editor invocation paths; add inline reply widget with editor escape." } } diff --git a/docs/rfd/045-layered-interrupt-handler-stack.md b/docs/rfd/045-layered-interrupt-handler-stack.md index cbaa6be1..a02a607d 100644 --- a/docs/rfd/045-layered-interrupt-handler-stack.md +++ b/docs/rfd/045-layered-interrupt-handler-stack.md @@ -4,6 +4,7 @@ - **Category**: Design - **Authors**: Jean Mertz - **Date**: 2025-07-24 +- **Required by**: [RFD 088] ## Summary @@ -671,3 +672,4 @@ Depends on Phases 2–4. [RFD 026]: 026-agent-loop-extraction.md [RFD 027]: 027-client-server-query-architecture.md +[RFD 088]: 088-unified-editor-service-and-inline-reply-widget.md diff --git a/docs/rfd/048-four-channel-output-model.md b/docs/rfd/048-four-channel-output-model.md index ddfea7d5..e84e8f97 100644 --- a/docs/rfd/048-four-channel-output-model.md +++ b/docs/rfd/048-four-channel-output-model.md @@ -6,7 +6,7 @@ - **Date**: 2026-03-17 - **Tracking Issue**: [\#518] - **Required by**: [RFD 029], [RFD 049], [RFD 051] -- **Extended by**: [RFD 086] +- **Extended by**: [RFD 086], [RFD 088] ## Summary @@ -251,3 +251,4 @@ Independent of Phases 1-2. [RFD 049]: 049-non-interactive-mode-and-detached-prompt-policy.md [RFD 051]: 051-sub-agent-workflows.md [RFD 086]: 086-line-oriented-stdin-input-for-cli-arguments.md +[RFD 088]: 088-unified-editor-service-and-inline-reply-widget.md diff --git a/docs/rfd/088-unified-editor-service-and-inline-reply-widget.md b/docs/rfd/088-unified-editor-service-and-inline-reply-widget.md new file mode 100644 index 00000000..6bd25a9e --- /dev/null +++ b/docs/rfd/088-unified-editor-service-and-inline-reply-widget.md @@ -0,0 +1,838 @@ +# RFD 088: Unified Editor Service and Inline Reply Widget + +- **Status**: Discussion +- **Category**: Design +- **Authors**: Jean Mertz +- **Date**: 2026-05-08 +- **Requires**: [RFD 045] +- **Extends**: [RFD 048] + +## Summary + +Consolidate JP's four independent editor-invocation paths onto a single +`EditorBackend` trait — with `edit_text` (string in/out) and `edit_file` +(path-based) methods — that fully respects `EditorConfig`, and replace the +interrupt-menu reply prompt with a richer inline editing widget, built on a +vendored copy of [reedline], that accepts short replies inline, supports +multi-line input, and escalates to the configured editor on demand. +A per-context opt-in (`reply_in_editor`) skips the inline step and opens the +editor immediately, for users who prefer that. + +## Motivation + +JP today opens an external editor through four separate code paths, each with +different fidelity to the user's `EditorConfig`: + +| Path | Where | Accepts | Fidelity | +| ------------------------------ | ---------------------------------------------- | ------------------------------------------------ | ------------------------------------------------------- | +| **A. Duct-based, file-based** | `jp_cli/src/editor.rs::open()` | full `duct::Expression` from `command()` | full | +| **B. `open-editor`-based** | `jp_editor::TerminalEditorBackend` (tool args, | single `Utf8PathBuf` from `EditorConfig::path()` | partial — args dropped by the `Utf8PathBuf` return type | +| | skip-reason, result edit in `prompter.rs`) | | | +| **C. `inquire::Editor`-based** | `TerminalPromptBackend::text_input` (the | nothing — reads `EDITOR` / `VISUAL` directly | none — ignores `editor.cmd`, `editor.envs`, `JP_EDITOR` | +| | `Reply:` prompt in streaming/tool interrupts) | | | +| **D. Duct-based, file-based** | `jp conversation edit` | full `duct::Expression` from `command()` | full — already correct, but unnamed by the original | +| | (`cmd/conversation/edit.rs`) | | "three paths" framing | + +The reported symptom is path C: pressing `r` after a `Ctrl+C` opens whatever +`$EDITOR` resolves to, ignoring JP's editor configuration entirely. +Path B has a less visible companion bug: its `Utf8PathBuf` return type cannot +carry argument vectors, so even with the env-var fix landed in `EditorConfig` +(see [ubiquitous-language: CommandConfig][cmd-cfg]) it loses any flags the user +attached — `EditorConfig::command()` carries them, but the path-based wiring +throws them away. +Migrating path B to `EditorConfig::command()` closes that gap. + +The structural cause is that JP has no first-class concept of "the act of +running the user's editor." +`EditorConfig` describes which editor to use; nothing in the codebase represents +the invocation. +Four call sites each filled the gap differently — paths A and D already honor +`EditorConfig::command()`, paths B and C do not. +The fix is to introduce a first-class `EditorBackend` and migrate the call sites +onto it. + +The interrupt-menu reply UX is also weak independently of the bug. +Today's `r` flow goes straight into `inquire::Editor`'s "press `e` to edit, +`Enter` to submit" two-step prompt, which forces the editor for every reply, +however short. +A first-class inline editor — with a rich editing experience and a `Ctrl+X +Ctrl+E` escape hatch (readline's `edit-and-execute-command` binding) — covers +both short replies and long ones without forcing a process-spawn for the trivial +case. + +## Design + +### User-facing behavior + +#### `r` (Reply) in the streaming-interrupt menu + +When the user presses `Ctrl+C` during streaming and chooses `r`: + +1. An inline reply prompt appears with the cursor inside an editable buffer. +2. The user types a reply directly. + Standard line-editing keybindings work (Ctrl+A/E for line nav, Ctrl+W to kill + word, arrow keys, Ctrl+L to clear, word movement, kill-ring, etc.). +3. **Enter** submits the buffer; an empty buffer follows the call site's + empty-Enter policy (for streaming `r`, returns to the menu — see + [Empty-Enter policy](#empty-enter-policy)). +4. **Shift+Enter** (on terminals that speak the kitty keyboard protocol — + kitty, Ghostty, WezTerm, foot) or **Alt+Enter** (portable fallback) insert a + newline. + The fallback is advertised in the help line. +5. **Ctrl+X Ctrl+E** opens the configured editor seeded with the current buffer + contents. +6. **Ctrl+C** returns the user to the interrupt menu without sending anything. + A second **Ctrl+C** at the menu escalates to graceful shutdown (see [RFD + 045]). + +After the editor closes (when invoked via `Ctrl+X Ctrl+E`): + +- If the editor's output is empty, control returns to the interrupt menu. +- If the editor's output is non-empty, the inline reply prompt re-appears with + the editor's output as the buffer; the user must press Enter to send. + +**Opt-in: straight to the editor.** Setting `interrupt.streaming.reply_in_editor += true` skips the inline widget for `r` and opens the configured editor +immediately, seeded empty. +A non-empty result is sent as-is; an empty result returns to the interrupt menu. +This serves users who always want a full editor for replies and would rather not +pass through the inline step. + +#### `s` (Stop & Reply) in the tool-interrupt menu + +Same flow as `r`, with one difference: an empty Enter or a `Ctrl+C` cancel does +not return to the menu — it falls through to today's +`DEFAULT_TOOL_CANCELLED_RESPONSE` canned message, which is delivered to the LLM. +This preserves the existing "interrupt a tool with no explanation" shortcut. + +The `interrupt.tool_call.reply_in_editor` opt-in applies here too: when set, `s` +opens the editor directly, and an empty editor result falls through to the +canned message. + +#### `jp query` (initial prompt) + +Unchanged behavior. +The user's first prompt opens directly in the configured editor, and an empty +save cancels the query — the existing `editor::edit_query` flow already uses +`EditorConfig::command()` correctly. + +#### Tool argument editing, skip reasoning, result editing + +All three currently route through `ToolPrompter` via `TerminalEditorBackend`, +each gated on a configured editor at its own menu/prompt site: +`permission_options` (`r`/`e`), `prompt_result_confirmation` (the `e` result +option), and the coordinator's `can_prompt` check for `ResultMode::Edit`. +After this RFD all three become `InlineReply` widgets seeded with the current +text — the JSON arguments, the skip-reason placeholder, the tool result — with +`Ctrl+X Ctrl+E` escaping to the configured editor on demand. + +Because `InlineReply` needs only a tty, the permission-menu options `r` ("Skip +and reply") and `e` ("Edit arguments") are no longer gated on a configured +editor; they appear whenever a prompt can be shown, and only the `Ctrl+X Ctrl+E` +escape requires `editor.command`. +The result-delivery confirmation is un-gated the same way: "Edit result first" +(`e`) appears whenever a tty is present, and `ResultMode::Edit` prompts on any +tty rather than requiring an editor. +In every case `Ctrl+X Ctrl+E` with no editor configured is a no-op — the inline +widget stays open. +The `JP_EDITOR="subl -w"` arg-drop bug closes as a side effect of routing that +escape through `EditorConfig::command()`. + +The argument editor keeps its parse-error re-prompt loop: on invalid JSON the +caller re-seeds the `InlineReply` buffer with the user's text and re-prompts, +surfacing the error in the prompt line — no process re-spawn, edits preserved, +and the old "Re-open editor? +y/n" confirmation step drops out. +`Ctrl+X Ctrl+E` still escapes to the full editor for a large rewrite, and its +result re-validates through the same loop. +An emptied buffer abandons the edit and falls back to the Ask prompt with the +arguments unchanged — it never submits empty JSON or runs the tool with cleared +arguments. + +### Architecture + +#### `EditorBackend`: the frontend seam for editor invocation + +Two invariants govern every editor open in JP: + +1. **Any** string-in/string-out edit goes through `EditorBackend`. +2. **Any** editor spawned by a *command-spawning* frontend (terminal, native + app) resolves its invocation through `EditorConfig::command()` and shared + open/wait logic. + +`EditorBackend` gains a second method so it covers both editing shapes a +frontend offers: + +```rust +pub trait EditorBackend: Send + Sync { + /// Ephemeral string editing: content in, edited content out. + fn edit_text(&self, content: &str) -> Result<(EditOutcome, String), EditorError>; + + /// Open the user's editor on the requested path(s) and block until editing + /// is done. The edited content is read back from disk by the caller. + fn edit_file(&self, req: EditRequest<'_>) -> Result; +} + +/// Frontend-agnostic request data for `edit_file`. Backend-specific context +/// (a web session, a native window handle, …) lives in the backend's own +/// fields, set at construction — *not* here — so `EditorBackend` stays +/// object-safe behind `Arc`. +pub struct EditRequest<'a> { + pub paths: &'a [Utf8PathBuf], + /// Working directory for a spawned editor; frontends that don't spawn a + /// local process ignore it. + pub cwd: Option<&'a Utf8Path>, +} + +/// The interaction outcome — the part only the backend can know. +pub enum EditOutcome { + /// User saved and closed (terminal editor exited 0; explicit save in a GUI). + Saved, + /// User aborted (terminal editor exited non-zero; explicit cancel in a GUI). + Cancelled, +} +``` + +`edit_text` returns `(EditOutcome, String)`; on `Cancelled` the string is +meaningless and callers ignore it. +Whether the content was *modified*, *unchanged*, or *emptied* is a content-delta +question the caller answers inline (`new == old`, `new.trim().is_empty()`) — +not the backend's concern, and a `classify` helper that only wraps `==` is not +worth the indirection. +The terminal backend maps a **non-zero editor exit to `Cancelled`**, matching +git's commit-message convention; a true spawn failure (binary not found) is an +`Err(EditorError)`. + +Backends are named by *frontend environment*, not storage medium: + +- `TerminalEditorBackend` — tempfile + `duct` spawn via + `EditorConfig::command()` (the only implementation this RFD ships). +- `WebEditorBackend`, `NativeEditorBackend` — future frontends (per the web-UI + and native-UI RFDs). + A web frontend mediates file editing through its server; a native frontend + spawns the application on the path. + Both implement the same two methods; only the *command-spawning* ones go + through `EditorConfig::command()`. +- `MockEditorBackend` — scripts outcomes for tests, including end-to-end flows + that exercise the editor escape without spawning anything. + +"No editor available" stays modeled as `Option>` +(`None`); there is no `Null` backend. + +The `TerminalEditorBackend`'s tempfile-and-run dance is the logic +`jp_cli/src/editor.rs::open()` performs today; we extract it into `jp_editor` so +there is one implementation. + +`open-editor` is dropped from both `jp_editor` and `jp_llm`. +The dead `ToolError::OpenEditorError` variant in `jp_llm/src/error.rs` is +**deleted** (it is constructed nowhere); editor failures live in +`jp_cli`/`jp_editor` as a generic `EditorError`. +`jp_llm` gains no dependency on `jp_editor` — the LLM crate does not know a +human editor exists. + +#### One construction helper + +A single free function `build_editor_backend(&cfg.editor)` in +`jp_cli/src/editor.rs` (it calls `EditorConfig::command()` and wraps the result +in a `TerminalEditorBackend`) is the *only* way any context obtains an editor, +so the call sites can't drift: + +- **Query startup** (`cmd/query.rs`) builds it for `edit_query`. +- **The turn loop** (`turn_loop.rs`) builds it once per turn and shares the same + `Arc` with `ToolPrompter` and `InterruptHandler`. +- **`jp conversation edit`** (`cmd/conversation/edit.rs`) builds it for its + `edit_file` call. + +The turn-loop site: + +```rust +let editor: Option> = build_editor_backend(&cfg.editor); + +let prompter = Arc::new(ToolPrompter::new( + printer.clone(), + editor.clone(), + prompt_backend.clone(), +)); + +// InterruptHandler::with_backend gets the same editor: +let handler = InterruptHandler::with_backend(backend, editor.clone()); +``` + +Promoting the helper out of `jp_cli` (into `jp_editor` or `jp_config`) is +deferred (YAGNI) — three call sites in one crate don't justify the move yet. + +#### File-based flows route through `edit_file` + +`jp_cli::editor::edit_query` and `jp conversation edit` (path D) keep their +bespoke *surrounding* policy at the call site — `edit_query`'s persistent +`QUERY_MESSAGE.md`, `RevertFileGuard`, and TOML preamble; `conversation edit`'s +validate-and-revert and projection sync. +That policy is the caller's domain and does not belong on the trait. + +The editor open in both now goes through `edit_file`, so they share one +open/wait path with the rest of JP and become frontend-polymorphic for free when +the web/native backends land. +The `EditRequest` carries the working directory (`edit_query` opens with `cwd = +conversation_root`, as it does today); frontends that don't spawn a local +process ignore it. +`edit_file` only promises "open the editor on these path(s) in this directory, +block until done"; the caller reads the files back and applies its own +validation, revert, and content checks. + +`edit_file` reports interaction via `EditOutcome`, and each file caller acts on +`Cancelled` itself: `edit_query` cancels the query (sends nothing); +`conversation edit` restores its pre-edit snapshots and reports — the same +effect its non-zero exit path has today. + +#### `InlineReply` widget in `jp_inquire` + +A new widget alongside `InlineSelect`, built on a vendored copy of [reedline] +(see [Terminal ownership](#terminal-ownership-and-the-vendored-reedline) below): + +```rust +pub struct InlineReply { + message: String, + initial_text: String, + help_message: Option, +} + +impl InlineReply { + pub fn new(message: impl Into) -> Self; + pub fn with_initial_text(self, text: impl Into) -> Self; + pub fn with_help_message(self, msg: impl Into) -> Self; + pub fn prompt(&self, writer: &mut dyn Write) -> Result; +} + +pub enum ReplyOutcome { + /// User pressed Enter on a non-empty buffer (or empty, where the caller's + /// policy permits it). + Submit(String), + /// User cancelled the prompt with `Ctrl+C`. The caller decides what this + /// means — return to a menu, use a fallback, or escalate. + Cancelled, + /// User pressed Ctrl+X Ctrl+E. Caller opens the editor seeded with + /// `current_text`. + OpenEditor { current_text: String }, +} +``` + +Reedline provides the rich editing experience out of the box: emacs-style +keybindings, multi-line input, cursor navigation, kill-ring, undo, word +movement, plus the unicode-width / line-wrap / paste / resize handling that is +expensive to reimplement correctly. +JP keeps every familiar default binding, including the Meta/Alt-based ones +(`Meta+B/F/D`, `Meta+Backspace`, …). +Custom bindings layered on top: + +| Key | Action | Mechanism | +| ------------- | ---------------- | ------------------------------------------------------------ | +| Enter | submit (default) | reedline default | +| Shift+Enter | newline | `EditCommand::InsertNewline` (kitty protocol; incl. Ghostty) | +| Alt+Enter | newline | same edit command (portable fallback) | +| Ctrl+X Ctrl+E | open editor | custom `ReedlineEvent` that returns `OpenEditor` | +| Ctrl+C | cancel | reedline `Signal::CtrlC`, surfaced as `Cancelled` | + +`Ctrl+C` is the single, mode-independent cancel: in raw mode it arrives as byte +`0x03`, reedline returns `Signal::CtrlC`, and the widget maps it to `Cancelled`. +We deliberately do **not** bind `Esc` (the Meta prefix in emacs mode, the +insert→normal toggle in vi mode) or `Ctrl+Q` (intercepted by some terminal +emulators). + +The widget enables `KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES` during +the prompt so `Shift+Enter` works on kitty-protocol terminals (kitty, Ghostty, +WezTerm, foot); `Alt+Enter` covers the rest and is advertised in the help line. + +**Edit mode is configurable.** Reedline ships `Emacs` (default) and `Vi` edit +modes, selected via `editor.inline.edit_mode` (see [Configuration +changes](#configuration-changes)). +The custom bindings above are registered into each mode's keymap separately, +since reedline keeps per-mode keybinding tables; in vi mode JP also enables +`with_cursor_config` so the cursor shape tracks insert/normal. + +The widget itself does **not** open the editor. +It signals intent via `OpenEditor`; the caller (the `InterruptHandler`, or +`ToolPrompter`) owns the editor decision. +This keeps `jp_inquire` free of editor concerns. + +#### Terminal ownership and the vendored reedline + +RFD 048 (Four-Channel Output Model, Implemented) requires interactive prompts to +render on `/dev/tty`, so they survive `jp query | jq` and `jp query 2> err.txt`. +Reedline does not meet this against the published crate: + +- **Output.** Reedline's painter is hardcoded to **stderr** (`pub type W = + BufWriter`, built in `Reedline::create()`). + Rendering to stderr breaks `2> err.txt` — the prompt is swallowed. +- **Cursor probing.** Reedline calls `crossterm::cursor::position()` during + prompt init and resize, and crossterm hardcodes that `ESC[6n` query to process + **stdout** (`cursor/sys/unix.rs`). + Under `| jq` that contaminates the data stream, and the query does not + round-trip when stdout is redirected. +- **Input is fine.** Reedline reads via crossterm's event source, whose + `tty_fd()` uses stdin when it is a TTY and otherwise opens `/dev/tty`; raw + mode uses the same fd. + Input already respects `/dev/tty`. + +Closing the output gap needs an upstream reedline change; closing the cursor +probe needs an upstream *crossterm* change (or for reedline to stop calling +`position()`) — two upstreams, one foundational. +Rather than gate this RFD on them, JP **vendors reedline** at +`crates/contrib/reedline` and carries two local patches, both reachable now that +the call sites are ours: + +1. Type-erase the painter writer (`W = BufWriter>`) and + add `Reedline::with_output(...)`, pointed at the same `/dev/tty` writer + `Printer` uses for its `Tty` target (`Printer::prompt_writer()`). +2. Replace reedline's two `cursor::position()` calls with a helper that writes + the `ESC[6n` query to that tty writer and reads the reply back through the + (already `/dev/tty`) event source. + +The `with_output` patch is clean and should be upstreamed, shrinking the +standing local delta to the cursor-probe change. +The widget drains `Printer` before taking over the terminal and restores after, +as `InlineSelect` does today. + +#### `PromptBackend::inline_reply` + +Replaces the removed `text_input` method: + +```rust +pub trait PromptBackend: Send + Sync { + // ... existing methods ... + + fn inline_reply( + &self, + message: &str, + initial_text: &str, + writer: &mut dyn Write, + ) -> Result; +} +``` + +Like the other `PromptBackend` methods, `inline_reply` is **writer-aware**: the +`jp_cli` call site passes `printer.prompt_writer()` (the `/dev/tty` target), and +`TerminalPromptBackend` feeds that writer to the vendored reedline's +`with_output`. +`jp_inquire` stays writer-agnostic and gains no `jp_printer` dependency — the +RFD 048 writer-passing boundary is preserved. + +`MockPromptBackend` gains `with_reply_outcomes(impl IntoIterator)` so tests can script the entire flow including editor escapes: +`[OpenEditor { ... }, Submit("done")]`. + +#### `InterruptHandler` becomes a loop + +Today's `handle_streaming_interrupt` and `handle_tool_interrupt` are +single-shot: they show the menu, get a choice, return. +The new design is a loop, so `Cancelled` from the inline reply returns to the +menu: + +```rust +pub fn handle_streaming_interrupt( + &self, + stream_alive: bool, + writer: &mut dyn Write, +) -> InterruptAction { + loop { + // A cancelled menu (Ctrl+C) escalates to graceful shutdown. Escalation + // is RFD 045's `InterruptOutcome::Escalated` — a handler outcome, not a + // new `InterruptAction` variant — so the real handler returns through + // 045's outcome type; this sketch elides that as `escalate()`. + let choice = match self.backend.inline_select(...) { + Ok(c) => c, + Err(_) => return escalate(), + }; + match choice { + 'c' if stream_alive => return InterruptAction::Resume, + 'c' => return InterruptAction::Continue, + 's' => return InterruptAction::Stop, + 'a' => return InterruptAction::Abort, + 'r' => match self.collect_reply("Reply:", self.reply_in_editor, writer) { + ReplyResult::Reply(text) => return InterruptAction::Reply(text), + ReplyResult::Back => continue, // back to menu + }, + _ => unreachable!(), + } + } +} + +fn collect_reply( + &self, + message: &str, + reply_in_editor: bool, + writer: &mut dyn Write, +) -> ReplyResult { + // Straight-to-editor opt-in: skip the inline widget and open the editor + // directly, seeded empty. + if reply_in_editor { + let Some(editor) = self.editor.as_ref() else { + // No editor configured: fall back to the inline widget rather than + // silently doing nothing (see Configuration changes). + return self.collect_reply(message, false, writer); + }; + return match editor.edit_text("") { + Ok((EditOutcome::Saved, text)) if !text.trim().is_empty() => { + ReplyResult::Reply(text) + } + Ok(_) => ReplyResult::Back, // empty or cancelled + Err(e) => { self.report(e); ReplyResult::Back } + }; + } + + let mut buffer = String::new(); + loop { + // Prompt errors and Ctrl+C are handled explicitly, never swallowed by + // `.ok()?` — the regression RFD 045 warns about. + match self.backend.inline_reply(message, &buffer, writer) { + Ok(ReplyOutcome::Submit(text)) if !text.is_empty() => { + return ReplyResult::Reply(text) + } + Ok(ReplyOutcome::Submit(_)) => return ReplyResult::Back, // empty + Ok(ReplyOutcome::Cancelled) => return ReplyResult::Back, // Ctrl+C + Ok(ReplyOutcome::OpenEditor { current_text }) => { + let Some(editor) = self.editor.as_ref() else { continue }; + match editor.edit_text(¤t_text) { + Ok((EditOutcome::Saved, edited)) + if !edited.trim().is_empty() => + { + buffer = edited; // re-seed the inline prompt + } + Ok(_) => return ReplyResult::Back, // empty or cancelled + Err(e) => { self.report(e); return ReplyResult::Back } + } + } + Err(e) => { self.report(e); return ReplyResult::Back } + } + } +} +``` + +`collect_reply` returns `ReplyResult { Reply(String), Back }` rather than an +`Option`, so prompt errors and `Ctrl+C` are handled explicitly instead of being +swallowed by `.ok()?` — the regression [RFD 045] warns about. +The `writer` (the `/dev/tty` prompt target) is threaded from the menu loop into +`collect_reply` and on to `inline_reply`, matching the writer-passing pattern of +the other `PromptBackend` methods. +The tool-interrupt `s` flow uses the same helper but substitutes +`DEFAULT_TOOL_CANCELLED_RESPONSE` for `Back`, preserving today's canned-message +semantics. + +This sketch is the `action = prompt` path. +The `[interrupt].*.action` config wraps it: a non-`prompt` action skips the menu +and runs directly, and a configured `reply` / `stop_reply` calls `collect_reply` +directly — the cancel fallback for those menu-less paths is pinned in +[Configuration changes](#configuration-changes), not left to implementation. +The `reply_in_editor` argument is read from the same per-context config struct. + +### Empty-Enter policy + +The widget always returns `Submit(text)` on Enter — empty or not. +Per-call-site policy: + +| Call site | Empty-text policy | +| ------------------------------------ | ---------------------------------------------------------------- | +| Streaming `r` | Empty → `Cancelled` (back to menu) | +| Tool `s` | Empty → fall through to canned `DEFAULT_TOOL_CANCELLED_RESPONSE` | +| Tool permission `r` (skip reason) | Empty → `None` (skip with no reason) | +| Tool permission `e` (edit arguments) | Empty → fall back to Ask (args unchanged) | +| Tool result edit | Empty → `None` (fall back to Ask) | + +Keeping the policy out of the widget matches the project's "code where it +belongs" principle — the meaning of "empty" is the caller's domain. + +### Configuration changes + +Two keys are added under `interrupt.{streaming,tool_call}` and one under +`editor.inline`: + +```toml +[interrupt.streaming] +reply_in_editor = false # r opens the editor directly, skipping the widget + +[interrupt.tool_call] +reply_in_editor = false # s opens the editor directly, skipping the widget + +[editor.inline] +edit_mode = "emacs" # "emacs" | "vi" +``` + +- **`reply_in_editor`** — defaults to `false`. + When `true`, the reply path opens the configured editor **instead of** showing + `InlineReply`; a non-empty saved result is sent immediately, an empty or + cancelled result returns to the menu. + If no editor is configured it falls back to the inline widget (never a silent + no-op). + A non-zero editor exit is treated as `Cancelled`. + In non-interactive / no-tty mode there is no prompt, so the key has no effect. +- **`editor.inline.edit_mode`** — selects reedline's edit mode for the inline + widget: the *editing style* of the inline buffer, orthogonal to which external + editor `Ctrl+X Ctrl+E` opens (that is `editor.command`). + +**Cancel / empty behavior matrix.** Defined for every context, including the +menu-less configured-action paths, so nothing is left to implementation: + +| Context | Menu? | Empty Enter | Ctrl+C | +| ------------------------ | ----- | ------------------- | ----------------------------- | +| streaming `r` | yes | back to menu | back to menu (2nd → escalate) | +| streaming `action=reply` | no | resume the response | resume the response | +| tool `s` | yes | canned message | back to menu (2nd → escalate) | +| tool `action=stop_reply` | no | canned message | canned message | + +"Escalate" is RFD 045's graceful shutdown. +The menu-less configured-action rows have no menu to return to, so cancel +resolves to the context's natural fallback. + +`Ctrl+C` inside `InlineReply` is a *local* reply cancellation (back to the +menu), not RFD 045 prompt escalation; only `Ctrl+C` at the interrupt menu +escalates. +The reply widget sits one level below the menu, so its cancel pops up one level, +and a second cancel at the menu escalates. + +### Ubiquitous-language additions + +Two terms enter the glossary: + +- **EditorBackend** — the frontend seam for invoking the user's configured + editor, with `edit_text` (string in/out) and `edit_file` (path-based) methods. + Each frontend (terminal, web, native, mock) is one implementation. +- **InlineReply** — the `jp_inquire` widget for short replies in interrupt + menus; supports inline typing with a `Ctrl+X Ctrl+E` escape to the + `EditorBackend`. + +## Drawbacks + +- **Vendored reedline.** JP carries a patched copy of reedline at + `crates/contrib/reedline` (see [Terminal + ownership](#terminal-ownership-and-the-vendored-reedline)), a maintenance line + item: tracking upstream releases and rebasing the two local patches (Lehman's + Law). + The `with_output` patch is intended for upstream, to shrink the standing delta + to the cursor-probe change. + The vendored crate pulls in `nu-ansi-term`, `unicode-segmentation`, and + `unicode-width`; `crossterm`, `serde`, and `strip-ansi-escapes` are already in + the tree. +- **Writer threading into reedline.** `PromptBackend::inline_reply` stays + writer-aware like its siblings (`&mut dyn Write`), but the vendored reedline + wants to own its output. + The `with_output` patch is shaped to render to the borrowed + `Printer::prompt_writer()` the call site passes, so `jp_inquire` never depends + on `jp_printer` and the RFD 048 writer-passing boundary is preserved. +- **More code in `jp_editor`.** Replacing the `open-editor` one-liner with a + duct-based tempfile dance is 50–80 LOC of real terminal-process plumbing + (Tesler's Law: the complexity has to live somewhere; the right somewhere is + here). +- **Behavior change for the `r` flow.** By default `r` opens the inline reply + widget instead of the `inquire::Editor` "press `e`" two-step. + (The old flow never went *straight* to the editor either — it always showed + that intermediate prompt; `reply_in_editor` is what makes straight-to-editor + possible for the first time.) + JP is pre-release, so the default change is acceptable; flagged for + completeness. + +## Alternatives + +### Alt 1: targeted patch — wire JP's editor into `inquire::Editor` + +Configure `inquire::Editor` with `with_editor_command` / `with_args` from the +resolved JP config. +**Rejected.** Inquire's `Editor` only takes a single binary plus arg slice; it +cannot represent shell-style `cmd: Some("foo && bar")`. +Adds a parameter to `text_input` that no other prompt method has. +Doesn't fix path B. Solves the symptom without addressing the design. + +### Alt 2: strangle `text_input` only, leave `EditorBackend` shape unchanged + +Push editor invocation up to the `InterruptHandler` call site using the existing +`EditorBackend` trait. +Closes path C but leaves path B's silent arg-drop in place. +**Rejected** in favor of doing both together — they touch the same wiring and +splitting them means rewriting the wiring twice. + +### Alt 3: defer to RFD 080 + +[RFD 080] restructures editor invocation around config resolution. +**Rejected as a substitute, accepted as orthogonal.** RFD 080 is about *which* +editor config wins; this RFD is about *how the resolved editor is invoked*. +Both can land independently. + +The env-var parsing fix (shlex-split values like `JP_EDITOR="code -w"`) already +landed at the `EditorConfig` layer ahead of this RFD, so path B's remaining +defect is purely the `Utf8PathBuf`-return-type discarding of args. +Migrating to `EditorConfig::command()` closes it without any further +config-shape work. + +### Alt 4: build a custom inline editor on raw crossterm + +Hand-roll a multi-line editor in `jp_inquire`, writing through +`Printer::prompt_writer()` directly (which would satisfy RFD 048 without any +vendoring). +**Rejected.** Reedline (used by nushell) has solved the expensive, quirky parts +— unicode width, line wrapping, multi-line cursor navigation, bracketed paste, +kitty-protocol disambiguation, resize, undo, kill-ring, and pluggable emacs/vi +edit modes. +A naive clone would hit those head-first. +Vendoring and patching reedline's two I/O seams (output writer, cursor probe) is +a far smaller and safer investment than reimplementing that surface. +The `/dev/tty` requirement does not force a hand-roll, because the vendored copy +renders through `Printer`'s tty writer. + +## Non-Goals + +- **Frontend backends beyond terminal.** Only `TerminalEditorBackend` ships + here. + `WebEditorBackend` / `NativeEditorBackend` are designed-for on the trait but + implemented by their own RFDs when the web/native UIs land. +- **Backward compatibility for the `r` flow.** Pre-release; UX changes are fair + game. +- **Editor selection at runtime.** This RFD does not introduce per-context + editor *selection* (e.g., a different external editor for inline replies vs. + the query prompt). + One external editor is configured; one is used. + (`reply_in_editor` controls *whether* a reply uses that editor, and + `editor.inline.edit_mode` controls the *inline widget's* editing style — both + separate axes from *which* external editor opens, so neither conflicts with + this non-goal.) +- **Arrow-key UX inside `InlineSelect`** or other existing widgets. + Out of scope. + +## Risks and Open Questions + +- **Vendored-reedline / `Printer` coordination.** The vendored reedline renders + through `Printer::prompt_writer()` (the `/dev/tty` target), but JP's `Printer` + still synchronizes streamed output, tool renderings, and prompt output through + a shared queue. + The widget must drain `Printer` before taking over the terminal and restore + cleanly after, as `InlineSelect` does today via `Printer::flush_instant()` / + `Printer::prompt_writer()`. + Validate during implementation. +- **Vendored-reedline patch surface.** The cursor-probe patch (rerouting + `cursor::position()` to the tty writer) has no upstream equivalent yet; verify + it behaves under `| jq` and `2> err.txt`, and that `terminal::size()` reads + the tty fd rather than stdout. +- **Reedline's prompt rendering.** Reedline's `Prompt` trait is opinionated + about how the prompt prefix renders (with built-in indicators for vi mode, + history search, etc.). + The widget's `Prompt` impl must match JP's existing prompt style (the + `[c,r,s,a,?]?`-style line). + Likely doable in 30 LOC but worth a spike. +- **`PromptBackend::inline_reply` returning `OpenEditor` from a mock.** Tests + need to script editor-escape flows. + The mock implementation is straightforward — script a vector of + `ReplyOutcome` values — but verify it composes cleanly with the existing + `MockEditorBackend` for full end-to-end tests of the loop. +- **Edit-mode keymap coverage.** The custom bindings (`Ctrl+X Ctrl+E`, newline, + `Ctrl+C`) must be registered into both the emacs and vi keymaps, and the vi + normal-mode path tested. + (The cancel-semantics question for menu-less configured actions is resolved in + [Configuration changes](#configuration-changes), not left open here.) + +## Implementation Plan + +### Phase 1: structural — `EditorBackend` becomes canonical + +- Add `edit_text` and `edit_file` to `EditorBackend`, plus the `EditOutcome` and + `EditRequest` types. + Re-shape `TerminalEditorBackend` around `duct::Expression`, extracting the + tempfile-and-run dance from `jp_cli/src/editor.rs::open()`; map a non-zero + editor exit to `EditOutcome::Cancelled`. +- Drop `open-editor` from `jp_editor` and `jp_llm`. + **Delete** the dead `ToolError::OpenEditorError` variant; export a generic + `EditorError` from `jp_editor`. + `jp_llm` gains no `jp_editor` dependency. +- Add `build_editor_backend` helper in `jp_cli/src/editor.rs`. +- Update `ToolPrompter` to receive `Option>` instead of + `Option`; update both `turn_loop.rs` construction sites. +- Route `edit_query` and `jp conversation edit` through `edit_file`, keeping + their surrounding policy at the call site. +- Update tests using `cfg.editor.path()` style construction. + +Reviewable independently. +Closes path B's arg-drop bug as a side effect; no user-visible UX change yet. + +Estimated diff: ~300 LOC. + +### Phase 2: `InlineReply` widget + +- Vendor reedline at `crates/contrib/reedline`; apply the two I/O patches + (type-erased painter writer + `with_output`; tty-routed cursor probe). +- Implement `InlineReply` on the vendored reedline with the keybindings and + `ReplyOutcome` enum described above, rendering through + `Printer::prompt_writer()`. +- Wire `editor.inline.edit_mode` to reedline's `Emacs`/`Vi` modes, registering + the custom bindings into each keymap. +- Implement a minimal `Prompt` impl that matches JP's prompt-line style. +- Snapshot tests for keybinding behavior using reedline's testable input stream + (or a thin shim). + +Reviewable independently. +No call-site changes yet — pure addition. + +Estimated diff: ~250 LOC. + +### Phase 3: `PromptBackend` integration and `InterruptHandler` rewiring + +- Remove `text_input` from `PromptBackend`. + Drop the `editor` feature on `inquire`. +- Add `inline_reply` to `PromptBackend` and update `TerminalPromptBackend` and + `MockPromptBackend` (with `with_reply_outcomes`). +- Migrate the `ToolPrompter` argument, skip-reasoning, and result edits to + `InlineReply` (seeded text + `Ctrl+X Ctrl+E` escape). + Un-gate all three editor-dependence points so only the escape needs + `editor.command`: `permission_options` (`r`/`e`), the `e` option in + `prompt_result_confirmation`, and the `prompter.has_editor()` term in + `coordinator.rs`'s `can_prompt` gate (so `ResultMode::Edit` prompts on any + tty). +- Add `editor: Option>` and `reply_in_editor` to + `InterruptHandler`; thread the editor through both + `InterruptHandler::with_backend` call sites in `interrupt/signals.rs`. +- Rewrite `handle_streaming_interrupt` and `handle_tool_interrupt` as loops with + `collect_reply` returning `ReplyResult`; map reedline `Signal::CtrlC` to + `Cancelled` and a cancelled *menu* to RFD 045's `Escalated`. +- Add `interrupt.{streaming,tool_call}.reply_in_editor` to `jp_config` and apply + the cancel/empty behavior matrix (incl. the menu-less configured-action rows). +- Update existing handler tests; add tests for `Cancelled → menu → submit`, + `OpenEditor → empty → menu`, the `reply_in_editor` straight-to-editor path, + and the configured-action cancel fallbacks. + +Depends on Phases 1 and 2. +Closes path C. Ships the new `r` UX. + +Estimated diff: ~400 LOC, mostly tests. + +### Phase 4: glossary and docs + +- Add **EditorBackend** and **InlineReply** to + `docs/architecture/ubiquitous-language.md`. +- Document `interrupt.*.reply_in_editor` and `editor.inline.edit_mode` in the + config reference. +- Update any user-facing docs that describe the `r` flow. + +Reviewable independently after Phase 3. + +## References + +- [RFD 045]: Layered Interrupt Handler Stack — the Ctrl+C escalation direction + the inline reply hooks into +- [RFD 048]: Four-Channel Output Model — the `/dev/tty` requirement the + vendored reedline must satisfy +- [RFD 080]: Editor as a Config Source — orthogonal concern; resolves *which* + editor config wins, not *how* the editor is invoked +- [reedline] — line-editor crate, vendored at `crates/contrib/reedline` +- `crates/jp_editor/src/lib.rs` — current `EditorBackend` trait +- `crates/jp_inquire/src/prompt.rs` — current `PromptBackend` trait, including + the `text_input` method to be removed +- `crates/jp_cli/src/cmd/query/interrupt/handler.rs` — current + `InterruptHandler` (path C) +- `crates/jp_cli/src/cmd/query/tool/prompter.rs` — current `ToolPrompter` + construction (path B) +- `crates/jp_cli/src/editor.rs` — current `editor::open()` and + `editor::edit_query` (path A) +- `crates/jp_config/src/editor.rs` — `EditorConfig::command()` and `path()` +- `crates/jp_config/src/interrupt.rs` — the `interrupt.{streaming,tool_call}` + config; the `action` field ships separately, `reply_in_editor` is added here + +[RFD 045]: 045-layered-interrupt-handler-stack.md +[RFD 048]: 048-four-channel-output-model.md +[RFD 080]: 080-editor-as-a-config-source.md +[cmd-cfg]: ../architecture/ubiquitous-language.md#commandconfig +[reedline]: https://crates.io/crates/reedline diff --git a/docs/rfd/drafts/D34-unified-editor-service-and-inline-reply-widget.md b/docs/rfd/drafts/D34-unified-editor-service-and-inline-reply-widget.md deleted file mode 100644 index 358d3260..00000000 --- a/docs/rfd/drafts/D34-unified-editor-service-and-inline-reply-widget.md +++ /dev/null @@ -1,559 +0,0 @@ -# RFD D34: Unified Editor Service and Inline Reply Widget - -- **Status**: Draft -- **Category**: Design -- **Authors**: Jean Mertz -- **Date**: 2026-05-08 - -## Summary - -Consolidate JP's three independent editor-invocation paths onto a single -`EditorBackend` service that fully respects `EditorConfig`, and replace the -interrupt-menu reply prompt with a richer inline editing widget that accepts -short replies inline, supports multi-line input, and escalates to the configured -editor on demand. -A per-context opt-in (`reply_in_editor`) skips the inline step and opens the -editor immediately, for users who prefer that. - -## Motivation - -JP today opens an external editor through three separate code paths, each with -different fidelity to the user's `EditorConfig`: - -| Path | Where | Accepts | Fidelity | -| ------------------------------ | ------------------------------------------ | ----------------------------------- | -------------------------------------- | -| **A. Duct-based, file-based** | `jp_cli/src/editor.rs::open()` | full `duct::Expression` from | full | -| | | `EditorConfig::command()` (incl. | | -| | | shell-style `cmd`) | | -| **B. `open-editor`-based** | `jp_editor::TerminalEditorBackend` (used | single `Utf8PathBuf` from | partial — args dropped by the | -| | in `prompter.rs` for tool argument | `EditorConfig::path()` | `Utf8PathBuf` return type, even though | -| | editing, skip-reason, result edit) | | `EditorConfig` now carries them | -| **C. `inquire::Editor`-based** | \`jp\_inquire::prompt::TerminalPromptBacke | nothing — reads `EDITOR` / `VISUAL` | none — ignores `editor.cmd`, | -| | nd::text\_input\` (used by | directly via `inquire` | `editor.envs`, `JP_EDITOR` | -| | `InterruptHandler` for the `Reply:` | | | -| | prompt in streaming and tool interrupts) | | | - -The reported symptom is path C: pressing `r` after a `Ctrl+C` opens whatever -`$EDITOR` resolves to, ignoring JP's editor configuration entirely. -Path B has a less visible companion bug: its `Utf8PathBuf` return type cannot -carry argument vectors, so even with the env-var fix landed in `EditorConfig` -(see [ubiquitous-language: CommandConfig][cmd-cfg]) it loses any flags the user -attached — `EditorConfig::command()` carries them, but the path-based wiring -throws them away. -Migrating path B to `EditorConfig::command()` closes that gap. - -The structural cause is that JP has no first-class concept of "the act of -running the user's editor." -`EditorConfig` describes which editor to use; nothing in the codebase represents -the invocation. -Three call sites each filled the gap differently. -The fix is to introduce that concept and migrate the call sites onto it. - -The interrupt-menu reply UX is also weak independently of the bug. -Today's `r` flow goes straight into `inquire::Editor`'s "press `e` to edit, -`Enter` to submit" two-step prompt, which forces the editor for every reply, -however short. -A first-class inline editor — with a rich editing experience and a `Ctrl+E` -escape hatch — covers both short replies and long ones without forcing a -process-spawn for the trivial case. - -## Design - -### User-facing behavior - -#### `r` (Reply) in the streaming-interrupt menu - -When the user presses `Ctrl+C` during streaming and chooses `r`: - -1. An inline reply prompt appears with the cursor inside an editable buffer. -2. The user types a reply directly. - Standard line-editing keybindings work (Ctrl+A/E for line nav, Ctrl+W to kill - word, arrow keys, Ctrl+L to clear, word movement, kill-ring, etc.). -3. **Enter** submits a non-empty buffer; on an empty buffer Enter is ignored. -4. **Shift+Enter** (on terminals supporting the kitty keyboard protocol), - **Alt+Enter**, or **Ctrl+J** insert a newline. - The portable fallback is advertised in the help line. -5. **Ctrl+E** opens the configured editor seeded with the current buffer - contents. -6. **Esc** returns the user to the interrupt menu without sending anything. - -After the editor closes (when invoked via `Ctrl+E`): - -- If the editor's output is empty, control returns to the interrupt menu. -- If the editor's output is non-empty, the inline reply prompt re-appears with - the editor's output as the buffer; the user must press Enter to send. - -**Opt-in: straight to the editor.** Setting `interrupt.streaming.reply_in_editor -= true` skips the inline widget for `r` and opens the configured editor -immediately, seeded empty. -A non-empty result is sent as-is; an empty result returns to the interrupt menu. -This serves users who always want a full editor for replies and would rather not -pass through the inline step. - -#### `s` (Stop & Reply) in the tool-interrupt menu - -Same flow as `r`, with one difference: an empty Enter or Esc does not return to -the menu — it falls through to today's `DEFAULT_TOOL_CANCELLED_RESPONSE` canned -message, which is delivered to the LLM. -This preserves the existing "interrupt a tool with no explanation" shortcut. - -The `interrupt.tool_call.reply_in_editor` opt-in applies here too: when set, `s` -opens the editor directly, and an empty editor result falls through to the -canned message. - -#### `jp query` (initial prompt) - -Unchanged behavior. -The user's first prompt opens directly in the configured editor, and an empty -save cancels the query — the existing `editor::edit_query` flow already uses -`EditorConfig::command()` correctly. - -#### Tool argument editing, skip reasoning, result editing - -All three currently route through `ToolPrompter` via `TerminalEditorBackend`. -After this RFD they use the same `Arc` as everything else, -which means a `JP_EDITOR="subl -w"` value finally honors the `-w` flag (the -silent arg-drop bug closes as a side effect). - -### Architecture - -#### `EditorBackend` becomes the canonical editor service - -`jp_editor::EditorBackend` keeps its existing trait shape: - -```rust -pub trait EditorBackend: Send + Sync { - fn edit(&self, content: &str) -> Result; -} -``` - -The terminal implementation changes from path-based to invocation-based: - -```rust -pub struct TerminalEditorBackend { - cmd: duct::Expression, -} - -impl EditorBackend for TerminalEditorBackend { - fn edit(&self, content: &str) -> Result { - // Write `content` to a tempfile, run `cmd` with the tempfile path - // appended as an argument, read the result back, delete the tempfile. - } -} -``` - -The tempfile-and-run logic is the same dance that `jp_cli/src/editor.rs::open()` -performs today; we extract it into `jp_editor` so there is one implementation. - -`open-editor` is removed as a dependency from both `jp_editor` and `jp_llm`. -The `ToolError::OpenEditorError` variant in `jp_llm/src/error.rs` becomes a -generic `EditorError` exported by `jp_editor`. - -#### One construction site - -`turn_loop.rs` builds the editor backend once per turn alongside the existing -`prompter`: - -```rust -let editor: Option> = build_editor_backend(&cfg.editor); - -let prompter = Arc::new(ToolPrompter::new( - printer.clone(), - editor.clone(), - prompt_backend.clone(), -)); - -// InterruptHandler::with_backend gets the same editor: -let handler = InterruptHandler::with_backend(backend, editor.clone()); -``` - -`build_editor_backend` is a thin free function in `jp_cli/src/editor.rs`. -It calls `EditorConfig::command()` and wraps the result in -`TerminalEditorBackend`. -Promoting the function to `jp_editor` or `jp_config` is deferred until a second -consumer needs it (YAGNI). - -#### `jp_cli::editor::edit_query` keeps its specialized form - -The query editor flow has genuinely different semantics: a persistent -`QUERY_MESSAGE.md` file in the conversation root, a `RevertFileGuard`, custom -CWD support, and a TOML preamble for inline config edits. -Forcing it through the `String → String` `EditorBackend` shape would lose -those. - -`edit_query` continues to call `EditorConfig::command()` directly. -The `build_editor_backend` helper and `edit_query` share the same `command()` -source, so they stay consistent. -If a future RFD adds a richer trait method (`edit_file_with_seed`, etc.), -`edit_query` can migrate then. - -#### `InlineReply` widget in `jp_inquire` - -A new widget alongside `InlineSelect`, built on [reedline]: - -```rust -pub struct InlineReply { - message: String, - initial_text: String, - help_message: Option, -} - -impl InlineReply { - pub fn new(message: impl Into) -> Self; - pub fn with_initial_text(self, text: impl Into) -> Self; - pub fn with_help_message(self, msg: impl Into) -> Self; - pub fn prompt(&self) -> Result; -} - -pub enum ReplyOutcome { - /// User pressed Enter on a non-empty buffer (or empty, where the caller's - /// policy permits it). - Submit(String), - /// User pressed Esc. - Cancelled, - /// User pressed Ctrl+E. Caller opens the editor seeded with `current_text`. - OpenEditor { current_text: String }, -} -``` - -Reedline provides the rich editing experience out of the box: emacs-style -keybindings, multi-line cursor navigation, kill-ring, undo, word movement. -Custom bindings layered on top: - -| Key | Action | Mechanism | -| ----------- | ---------------- | ------------------------------------------------------------------ | -| Enter | submit (default) | reedline default | -| Shift+Enter | newline | reedline `EditCommand::InsertChar('\n')` (requires kitty protocol) | -| Alt+Enter | newline | same edit command (universal portable) | -| Ctrl+J | newline | same edit command (readline convention) | -| Esc | cancel | mapped to a custom `ReedlineEvent` that returns `Cancelled` | -| Ctrl+E | open editor | mapped to a custom `ReedlineEvent` that returns `OpenEditor` | - -The widget enables `KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES` during -the prompt so Shift+Enter works on supporting terminals; Alt+Enter and Ctrl+J -cover the rest. -The help line advertises the portable fallback. - -The widget itself does **not** open the editor. -It signals intent via `OpenEditor`; the caller (the `InterruptHandler`, or -`ToolPrompter` if the widget is reused there later) owns the editor decision. -This keeps `jp_inquire` free of editor concerns. - -#### `PromptBackend::inline_reply` - -Replaces the removed `text_input` method: - -```rust -pub trait PromptBackend: Send + Sync { - // ... existing methods ... - - fn inline_reply( - &self, - message: &str, - initial_text: &str, - ) -> Result; -} -``` - -`MockPromptBackend` gains `with_reply_outcomes(impl IntoIterator)` so tests can script the entire flow including editor escapes: -`[OpenEditor { ... }, Submit("done")]`. - -#### `InterruptHandler` becomes a loop - -Today's `handle_streaming_interrupt` and `handle_tool_interrupt` are -single-shot: they show the menu, get a choice, return. -The new design is a loop, so `Cancelled` from the inline reply returns to the -menu: - -```rust -pub fn handle_streaming_interrupt( - &self, - stream_alive: bool, -) -> InterruptAction { - loop { - let choice = self.backend.inline_select(...).unwrap_or('s'); - match choice { - 'c' if stream_alive => return InterruptAction::Resume, - 'c' => return InterruptAction::Continue, - 's' => return InterruptAction::Stop, - 'a' => return InterruptAction::Abort, - 'r' => match self.collect_reply("Reply:") { - Some(text) => return InterruptAction::Reply(text), - None => continue, // back to menu - }, - _ => unreachable!(), - } - } -} - -fn collect_reply(&self, message: &str, reply_in_editor: bool) -> Option { - // Straight-to-editor opt-in: skip the inline widget and open the editor - // directly, seeded empty. A non-empty result is sent as-is. - if reply_in_editor { - let editor = self.editor.as_ref()?; - let edited = editor.edit("").ok()?; - return (!edited.trim().is_empty()).then_some(edited); - } - - let mut buffer = String::new(); - loop { - match self.backend.inline_reply(message, &buffer).ok()? { - ReplyOutcome::Submit(text) if !text.is_empty() => return Some(text), - ReplyOutcome::Submit(_) => return None, // empty: back to menu - ReplyOutcome::Cancelled => return None, - ReplyOutcome::OpenEditor { current_text } => { - let editor = self.editor.as_ref()?; - let edited = editor.edit(¤t_text).ok()?; - if edited.trim().is_empty() { - return None; // empty editor: back to menu - } - buffer = edited; // re-seed the inline prompt - } - } - } -} -``` - -The tool-interrupt `s` flow uses the same `collect_reply` helper but substitutes -`DEFAULT_TOOL_CANCELLED_RESPONSE` for `None`, preserving today's canned-message -semantics. - -This sketch is the `action = prompt` path. -The `[interrupt].*.action` config (shipped separately) wraps it: a non-`prompt` -action skips the menu and runs directly, and a configured `reply` / `stop_reply` -calls `collect_reply` directly. -The `reply_in_editor` argument is read from the same per-context config struct. - -### Empty-Enter policy - -The widget always returns `Submit(text)` on Enter — empty or not. -Per-call-site policy: - -| Call site | Empty-text policy | -| ---------------------------------------------- | ---------------------------------------------------------------- | -| Streaming `r` | Empty → `Cancelled` (back to menu) | -| Tool `s` | Empty → fall through to canned `DEFAULT_TOOL_CANCELLED_RESPONSE` | -| Tool permission `r` (`prompter.rs::edit_text`) | Empty → `None` (skip with no reason) | - -Keeping the policy out of the widget matches the project's "code where it -belongs" principle — the meaning of "empty" is the caller's domain. - -### Ubiquitous-language additions - -Two terms enter the glossary: - -- **EditorBackend** — the trait abstracting "open the user's editor with - content X, get content back." - The single seam for ephemeral string-in/string-out editing. -- **InlineReply** — the `jp_inquire` widget for short replies in interrupt - menus; supports inline typing with a `Ctrl+E` escape to the `EditorBackend`. - -## Drawbacks - -- **Reedline dependency.** Adds ~6 transitive dependencies to `jp_inquire` - (`reedline`, `nu-ansi-term`, `unicode-segmentation`, `unicode-width`, plus - small utilities). - `crossterm`, `serde`, and `strip-ansi-escapes` are already in the tree. -- **Reedline draws directly to stdout** rather than accepting a `&mut dyn - Write`. - The `PromptBackend::inline_reply` writer parameter is dropped (the existing - widgets use the writer for the prompt-line prefix; reedline owns its own - prompt rendering via the `Prompt` trait). - This is consistent with how `inquire::Editor` already behaves today. -- **More code in `jp_editor`.** Replacing the `open-editor` one-liner with a - duct-based tempfile dance is 50–80 LOC of real terminal-process plumbing - (Tesler's Law: the complexity has to live somewhere; the right somewhere is - here). -- **Behavior change for the `r` flow.** By default `r` opens the inline reply - widget instead of the `inquire::Editor` "press `e`" two-step. - (The old flow never went *straight* to the editor either — it always showed - that intermediate prompt; `reply_in_editor` is what makes straight-to-editor - possible for the first time.) - JP is pre-release, so the default change is acceptable; flagged for - completeness. - -## Alternatives - -### Alt 1: targeted patch — wire JP's editor into `inquire::Editor` - -Configure `inquire::Editor` with `with_editor_command` / `with_args` from the -resolved JP config. -**Rejected.** Inquire's `Editor` only takes a single binary plus arg slice; it -cannot represent shell-style `cmd: Some("foo && bar")`. -Adds a parameter to `text_input` that no other prompt method has. -Doesn't fix path B. Solves the symptom without addressing the design. - -### Alt 2: strangle `text_input` only, leave `EditorBackend` shape unchanged - -Push editor invocation up to the `InterruptHandler` call site using the existing -`EditorBackend` trait. -Closes path C but leaves path B's silent arg-drop in place. -**Rejected** in favor of doing both together — they touch the same wiring and -splitting them means rewriting the wiring twice. - -### Alt 3: defer to RFD 080 - -[RFD 080] restructures editor invocation around config resolution. -**Rejected as a substitute, accepted as orthogonal.** RFD 080 is about *which* -editor config wins; this RFD is about *how the resolved editor is invoked*. -Both can land independently. - -The env-var parsing fix (shlex-split values like `JP_EDITOR="code -w"`) already -landed at the `EditorConfig` layer ahead of this RFD, so path B's remaining -defect is purely the `Utf8PathBuf`-return-type discarding of args. -Migrating to `EditorConfig::command()` closes it without any further -config-shape work. - -### Alt 4: build a custom inline editor on raw crossterm - -Avoid the reedline dependency by hand-rolling a multi-line editor in -`jp_inquire`. -**Rejected.** Reedline is well-maintained (used by nushell) and gives us -emacs-style line editing, kill-ring, multi-line cursor navigation, and undo for -free. -Reimplementing those badly is a worse use of time than the dependency cost. - -## Non-Goals - -- **Full unification of path A.** `jp_cli::editor::edit_query` keeps its - specialized form. - Folding it into `EditorBackend` requires either a richer trait method or a - separate trait. - Deferred. -- **Backward compatibility for the `r` flow.** Pre-release; UX changes are fair - game. -- **Editor selection at runtime.** This RFD does not introduce per-context - editor configuration (e.g., a different editor for inline replies vs. the - query prompt). - One editor is configured; one editor is used. - (`reply_in_editor` controls *whether* a reply uses that editor, not *which* - editor — a separate axis, so it does not conflict with this non-goal.) -- **Arrow-key UX inside `InlineSelect`** or other existing widgets. - Out of scope. - -## Risks and Open Questions - -- **Reedline / `Printer` coordination.** Reedline draws directly to stdout. - JP's `Printer` synchronizes streamed LLM output, tool renderings, and prompt - output through a shared queue. - The widget needs to drain `Printer` before taking over the terminal and - restore cleanly after. - The current `InlineSelect` widget already handles this via - `Printer::flush_instant()` and `Printer::prompt_writer()`; the `InlineReply` - widget needs the same treatment. - Validate during implementation. -- **Reedline's prompt rendering.** Reedline's `Prompt` trait is opinionated - about how the prompt prefix renders (with built-in indicators for vi mode, - history search, etc.). - The widget's `Prompt` impl must match JP's existing prompt style (the - `[c,r,s,a,?]?`-style line). - Likely doable in 30 LOC but worth a spike. -- **`Ctrl+E` collision.** Emacs-style line editing binds `Ctrl+E` to "move to - end of line." - Overriding it in this widget breaks the muscle memory of users who expect that - binding inside reedline. - Considered acceptable for a reply prompt (the buffer is short; "move to end of - line" matters less than "open editor"), but flagged. -- **`PromptBackend::inline_reply` returning `OpenEditor` from a mock.** Tests - need to script editor-escape flows. - The mock implementation is straightforward — script a vector of - `ReplyOutcome` values — but verify it composes cleanly with the existing - `MockEditorBackend` for full end-to-end tests of the loop. -- **Cancel semantics when `reply` / `stop_reply` is a *configured* action.** The - `[interrupt].*.action` config (shipped separately) can make `reply` run - without ever showing the menu. - There is then no menu to return to on `Esc` or empty input, unlike the - menu-driven `r`. - The handler needs a defined fallback — likely `Resume` for streaming and the - canned message for tools. - Pin this down during implementation. - -## Implementation Plan - -### Phase 1: structural — `EditorBackend` becomes canonical - -- Re-shape `TerminalEditorBackend` around `duct::Expression`. - Extract the tempfile-and-run dance from `jp_cli/src/editor.rs::open()`. -- Drop `open-editor` from `jp_editor` and `jp_llm`. - Replace `ToolError::OpenEditorError` with a generic `EditorError` exported by - `jp_editor`. -- Add `build_editor_backend` helper in `jp_cli/src/editor.rs`. -- Update `ToolPrompter` to receive `Option>` instead of - `Option`; update both `turn_loop.rs` construction sites. -- Update tests using `cfg.editor.path()` style construction. - -Reviewable independently. -Closes path B's arg-drop bug as a side effect; no user-visible UX change yet. - -Estimated diff: ~300 LOC. - -### Phase 2: `InlineReply` widget - -- Add `reedline` dependency to `jp_inquire`. -- Implement `InlineReply` widget with the keybindings and `ReplyOutcome` enum - described above. -- Implement a minimal `Prompt` impl that matches JP's prompt-line style. -- Snapshot tests for keybinding behavior using reedline's testable input stream - (or a thin shim). - -Reviewable independently. -No call-site changes yet — pure addition. - -Estimated diff: ~250 LOC. - -### Phase 3: `PromptBackend` integration and `InterruptHandler` rewiring - -- Remove `text_input` from `PromptBackend`. - Drop the `editor` feature on `inquire`. -- Add `inline_reply` method to `PromptBackend` and update - `TerminalPromptBackend` and `MockPromptBackend`. -- Add `editor: Option>` field to `InterruptHandler`; - thread the editor through both `InterruptHandler::with_backend` call sites in - `interrupt/signals.rs`. -- Rewrite `handle_streaming_interrupt` and `handle_tool_interrupt` as loops with - the `collect_reply` helper. -- Read `reply_in_editor` from the existing `interrupt.{streaming,tool_call}` - config and pass it into `collect_reply`; resolve the configured-action cancel - fallback (see Risks). -- Update existing handler tests; add tests for the `Cancelled → menu → - submit`, `OpenEditor → empty → menu`, and `reply_in_editor` - straight-to-editor paths. - -Depends on Phases 1 and 2. -Closes path C. Ships the new `r` UX. - -Estimated diff: ~400 LOC, mostly tests. - -### Phase 4: glossary and docs - -- Add **EditorBackend** and **InlineReply** to - `docs/architecture/ubiquitous-language.md`. -- Update any user-facing docs that describe the `r` flow. - -Reviewable independently after Phase 3. - -## References - -- [RFD 045]: Layered Interrupt Handler Stack — context for the existing - interrupt-handler architecture -- [RFD 080]: Editor as a Config Source — orthogonal concern; resolves *which* - editor config wins, not *how* the editor is invoked -- [reedline] — line-editor crate proposed for `InlineReply` -- `crates/jp_editor/src/lib.rs` — current `EditorBackend` trait -- `crates/jp_inquire/src/prompt.rs` — current `PromptBackend` trait, including - the `text_input` method to be removed -- `crates/jp_cli/src/cmd/query/interrupt/handler.rs` — current - `InterruptHandler` (path C) -- `crates/jp_cli/src/cmd/query/tool/prompter.rs` — current `ToolPrompter` - construction (path B) -- `crates/jp_cli/src/editor.rs` — current `editor::open()` and - `editor::edit_query` (path A) -- `crates/jp_config/src/editor.rs` — `EditorConfig::command()` and `path()` -- `crates/jp_config/src/interrupt.rs` — the `interrupt.{streaming,tool_call}` - config; the `action` field ships separately, `reply_in_editor` is added here - -[RFD 045]: ../045-layered-interrupt-handler-stack.md -[RFD 080]: ../080-editor-as-a-config-source.md -[cmd-cfg]: ../../architecture/ubiquitous-language.md#commandconfig -[reedline]: https://crates.io/crates/reedline diff --git a/docs/rfd/priority.json b/docs/rfd/priority.json index c48bb629..64de320b 100644 --- a/docs/rfd/priority.json +++ b/docs/rfd/priority.json @@ -1,7 +1,7 @@ { "order": [ - "031", - "064", + "045", + "088", "087", "065", "039", @@ -36,7 +36,6 @@ "040", "043", "044", - "045", "049", "051", "059", @@ -85,7 +84,6 @@ "D30", "D31", "D32", - "D34", "D35", "D36", "D37", @@ -118,7 +116,8 @@ "D55" ], "in_development": [ - "065", - "087" + "045", + "087", + "088" ] }