From 9f2316e45dd0427e2e4eddfe824fa627db296dd4 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Mon, 29 Jun 2026 18:11:14 +0200 Subject: [PATCH 1/3] feat(editor, inquire, cli, config): Add inline reply widget and unified editor backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JP now accepts replies inline — without requiring a configured external editor. Choosing `r` while streaming or `s` while tools run opens a rich, multi-line prompt built on `reedline` instead of spawning an editor. `Ctrl+X` inside the widget escapes to the external editor and re-seeds the inline buffer with the result; `Ctrl+C` cancels and returns to the interrupt menu. The same inline widget drives the tool argument-edit (`e`), result-edit (`e` in the delivery confirmation), and skip-reasoning (`r`) flows in `ToolPrompter`. These options are now always offered regardless of whether `editor.cmd` is configured; the editor is the escape hatch, not the only path. Two new config keys control the new behaviour: ```toml [editor.inline] edit_mode = "emacs" # or "vi" [interrupt.streaming] reply_in_editor = false # open editor directly, skipping the widget [interrupt.tool_call] reply_in_editor = false ``` The `--edit` flag on `jp query` is simplified to a plain boolean (`--edit` / `--no-edit` / `-E`); the previous `--edit=` and `--edit=false` forms are removed. The `open-editor` crate is replaced by a new `EditorBackend` trait in `jp_editor` with two methods: `edit_text` (ephemeral string-in/out via a temp file) and `edit_file` (open the editor on caller-owned paths). `EditOutcome` replaces the old success/error duality: a non-zero exit now maps to `EditOutcome::Cancelled` rather than an error, letting callers handle an aborted editor gracefully instead of propagating a failure. `EditorConfig::path()` is removed; all call sites use `build_editor_backend()`. The `PromptBackend` trait's `text_input` method is replaced by `inline_reply`, which returns a `ReplyOutcome` (`Submit`, `Cancelled`, or `OpenEditor { current_text }`). `MockPromptBackend` gains `with_reply_outcomes` to script full reply flows — including editor escapes — in tests. `Printer::owned_prompt_writer` is added so components that need to own their output stream (the line editor) can get a `Box` that still routes through the printer's serialized worker. This implements RFD 088. Signed-off-by: Jean Mertz --- .config/supply-chain/audits.toml | 5 + .config/supply-chain/config.toml | 11 +- .config/supply-chain/imports.lock | 89 ++ Cargo.lock | 91 +- Cargo.toml | 2 +- crates/jp_cli/src/cmd/conversation/edit.rs | 44 +- crates/jp_cli/src/cmd/query.rs | 53 +- crates/jp_cli/src/cmd/query/interrupt.rs | 2 +- .../jp_cli/src/cmd/query/interrupt/handler.rs | 245 ++++-- .../src/cmd/query/interrupt/handler_tests.rs | 365 +++++--- .../jp_cli/src/cmd/query/interrupt/signals.rs | 20 +- .../src/cmd/query/interrupt/signals_tests.rs | 31 +- .../jp_cli/src/cmd/query/tool/coordinator.rs | 13 +- .../src/cmd/query/tool/coordinator_tests.rs | 20 +- crates/jp_cli/src/cmd/query/tool/prompter.rs | 255 +++--- .../src/cmd/query/tool/prompter_tests.rs | 821 +++++------------- crates/jp_cli/src/cmd/query/turn_loop.rs | 13 +- crates/jp_cli/src/editor.rs | 102 +-- crates/jp_config/src/editor.rs | 99 ++- crates/jp_config/src/editor_tests.rs | 43 +- crates/jp_config/src/interrupt.rs | 30 + .../jp_config__tests__app_config_fields.snap | 3 + ...ig__tests__partial_app_config_default.snap | 5 + ...ts__partial_app_config_default_values.snap | 9 + ...s__partial_app_config_empty_serialize.snap | 5 + crates/jp_editor/Cargo.toml | 4 +- crates/jp_editor/src/lib.rs | 166 +++- crates/jp_editor/tests/terminal.rs | 57 ++ crates/jp_inquire/Cargo.toml | 4 +- crates/jp_inquire/src/inline_reply.rs | 232 +++++ crates/jp_inquire/src/inline_reply_tests.rs | 95 ++ crates/jp_inquire/src/lib.rs | 2 + crates/jp_inquire/src/prompt.rs | 66 +- crates/jp_llm/Cargo.toml | 1 - crates/jp_llm/src/error.rs | 7 - ...ompletion_stream__conversation_stream.snap | 11 +- ...image_attachment__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...daptive_thinking__conversation_stream.snap | 11 +- ...s_4_6_max_effort__conversation_stream.snap | 11 +- ...edacted_thinking__conversation_stream.snap | 11 +- ...request_chaining__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...ompletion_stream__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...ompletion_stream__conversation_stream.snap | 11 +- ...mini_3_reasoning__conversation_stream.snap | 11 +- ...image_attachment__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...ompletion_stream__conversation_stream.snap | 11 +- ...image_attachment__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...ompletion_stream__conversation_stream.snap | 11 +- ...image_attachment__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...ompletion_stream__conversation_stream.snap | 11 +- ...image_attachment__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...r_event_metadata__conversation_stream.snap | 11 +- ...r_event_metadata__conversation_stream.snap | 11 +- ...r_event_metadata__conversation_stream.snap | 11 +- ...ompletion_stream__conversation_stream.snap | 11 +- ...image_attachment__conversation_stream.snap | 11 +- ...urn_conversation__conversation_stream.snap | 11 +- ...tructured_output__conversation_stream.snap | 11 +- ...t_tool_call_auto__conversation_stream.snap | 11 +- ...ol_call_function__conversation_stream.snap | 11 +- ...l_call_reasoning__conversation_stream.snap | 11 +- ...red_no_reasoning__conversation_stream.snap | 11 +- ...quired_reasoning__conversation_stream.snap | 11 +- ...tool_call_stream__conversation_stream.snap | 11 +- ...r_event_metadata__conversation_stream.snap | 11 +- crates/jp_printer/src/printer.rs | 60 ++ deny.toml | 1 + docs/architecture/ubiquitous-language.md | 25 + docs/features.md | 6 +- .../045-layered-interrupt-handler-stack.md | 41 +- ...-editor-service-and-inline-reply-widget.md | 34 +- justfile | 9 +- 120 files changed, 2577 insertions(+), 1467 deletions(-) create mode 100644 crates/jp_editor/tests/terminal.rs create mode 100644 crates/jp_inquire/src/inline_reply.rs create mode 100644 crates/jp_inquire/src/inline_reply_tests.rs diff --git a/.config/supply-chain/audits.toml b/.config/supply-chain/audits.toml index 0a7f5fde..67731b0e 100644 --- a/.config/supply-chain/audits.toml +++ b/.config/supply-chain/audits.toml @@ -81,6 +81,11 @@ who = "Jean Mertz " criteria = "safe-to-deploy" version = "0.19.0" +[[audits.itertools]] +who = "Jean Mertz " +criteria = "safe-to-deploy" +version = "0.13.0" + [[audits.libbz2-rs-sys]] who = "Jean Mertz " criteria = "safe-to-deploy" diff --git a/.config/supply-chain/config.toml b/.config/supply-chain/config.toml index 84402393..c73e73ed 100644 --- a/.config/supply-chain/config.toml +++ b/.config/supply-chain/config.toml @@ -40,6 +40,9 @@ audit-as-crates-io = true [policy.openai_responses] audit-as-crates-io = false +[policy.reedline] +audit-as-crates-io = true + [policy.saphyr] audit-as-crates-io = true @@ -419,10 +422,6 @@ criteria = "safe-to-deploy" version = "0.3.3" criteria = "safe-to-deploy" -[[exemptions.open-editor]] -version = "1.1.0" -criteria = "safe-to-deploy" - [[exemptions.os_pipe]] version = "1.2.2" criteria = "safe-to-deploy" @@ -483,6 +482,10 @@ criteria = "safe-to-deploy" version = "0.5.2" criteria = "safe-to-deploy" +[[exemptions.reedline]] +version = "0.48.0@git:d6a56e266758c5a28d7b635b20611dfb50c6acfb" +criteria = "safe-to-deploy" + [[exemptions.relative-path]] version = "2.0.1" criteria = "safe-to-deploy" diff --git a/.config/supply-chain/imports.lock b/.config/supply-chain/imports.lock index 5e9cea2b..0eb42106 100644 --- a/.config/supply-chain/imports.lock +++ b/.config/supply-chain/imports.lock @@ -1261,6 +1261,24 @@ criteria = "safe-to-deploy" delta = "2.1.1 -> 2.3.0" notes = "Minor refactoring, nothing new." +[[audits.bytecode-alliance.audits.fd-lock]] +who = "Pat Hickey " +criteria = "safe-to-deploy" +version = "3.0.9" +notes = "This crate uses unsafe to make Windows syscalls, to borrow an Fd with an appropriate lifetime, and to zero a windows API structure that appears to have a valid representation with zeroed memory." + +[[audits.bytecode-alliance.audits.fd-lock]] +who = "Pat Hickey " +criteria = "safe-to-deploy" +delta = "3.0.9 -> 3.0.10" +notes = "Just a dependency version bump" + +[[audits.bytecode-alliance.audits.fd-lock]] +who = "Dan Gohman " +criteria = "safe-to-deploy" +delta = "3.0.10 -> 3.0.12" +notes = "Just a dependency version bump" + [[audits.bytecode-alliance.audits.foldhash]] who = "Alex Crichton " criteria = "safe-to-deploy" @@ -2444,6 +2462,28 @@ Previously reviewed during security review and the audit is grandparented in. """ aggregated-from = "https://chromium.googlesource.com/chromium/src/+/main/third_party/rust/chromium_crates_io/supply-chain/audits.toml?format=TEXT" +[[audits.google.audits.strum]] +who = "danakj@chromium.org" +criteria = "safe-to-deploy" +version = "0.25.0" +notes = """ +Reviewed in https://crrev.com/c/5171063 + +Previously reviewed during security review and the audit is grandparented in. +""" +aggregated-from = "https://chromium.googlesource.com/chromium/src/+/main/third_party/rust/chromium_crates_io/supply-chain/audits.toml?format=TEXT" + +[[audits.google.audits.strum_macros]] +who = "danakj@chromium.org" +criteria = "safe-to-deploy" +version = "0.25.3" +notes = """ +Reviewed in https://crrev.com/c/5171063 + +Previously reviewed during security review and the audit is grandparented in. +""" +aggregated-from = "https://chromium.googlesource.com/chromium/src/+/main/third_party/rust/chromium_crates_io/supply-chain/audits.toml?format=TEXT" + [[audits.google.audits.synstructure]] who = "Manish Goregaokar " criteria = "safe-to-deploy" @@ -3130,6 +3170,19 @@ delta = "2.1.0 -> 2.1.1" notes = "Fairly trivial changes, no chance of security regression." aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" +[[audits.mozilla.audits.fd-lock]] +who = "Jan-Erik Rediger " +criteria = "safe-to-deploy" +delta = "3.0.12 -> 3.0.13" +notes = "Dependency updates only" +aggregated-from = "https://raw.githubusercontent.com/mozilla/glean/main/supply-chain/audits.toml" + +[[audits.mozilla.audits.fd-lock]] +who = "Jan-Erik Rediger " +criteria = "safe-to-deploy" +delta = "3.0.13 -> 4.0.4" +aggregated-from = "https://raw.githubusercontent.com/mozilla/glean/main/supply-chain/audits.toml" + [[audits.mozilla.audits.fnv]] who = "Bobby Holley " criteria = "safe-to-deploy" @@ -3606,6 +3659,30 @@ criteria = "safe-to-deploy" delta = "0.10.0 -> 0.11.1" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" +[[audits.mozilla.audits.strum]] +who = "Teodor Tanasoaia " +criteria = "safe-to-deploy" +delta = "0.25.0 -> 0.26.3" +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + +[[audits.mozilla.audits.strum]] +who = "Erich Gubler " +criteria = "safe-to-deploy" +delta = "0.26.3 -> 0.27.1" +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + +[[audits.mozilla.audits.strum_macros]] +who = "Teodor Tanasoaia " +criteria = "safe-to-deploy" +delta = "0.25.3 -> 0.26.4" +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + +[[audits.mozilla.audits.strum_macros]] +who = "Erich Gubler " +criteria = "safe-to-deploy" +delta = "0.26.4 -> 0.27.1" +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + [[audits.mozilla.audits.subtle]] who = "Simon Friedberger " criteria = "safe-to-deploy" @@ -4036,6 +4113,18 @@ This reverts a previous change that lowered #[forbid(unsafe_code)] to #[deny(uns """ aggregated-from = "https://raw.githubusercontent.com/zcash/librustzcash/main/supply-chain/audits.toml" +[[audits.zcash.audits.strum]] +who = "Jack Grigg " +criteria = "safe-to-deploy" +delta = "0.27.1 -> 0.27.2" +aggregated-from = "https://raw.githubusercontent.com/zcash/librustzcash/main/supply-chain/audits.toml" + +[[audits.zcash.audits.strum_macros]] +who = "Jack Grigg " +criteria = "safe-to-deploy" +delta = "0.27.1 -> 0.27.2" +aggregated-from = "https://raw.githubusercontent.com/zcash/librustzcash/main/supply-chain/audits.toml" + [[audits.zcash.audits.thread_local]] who = "Jack Grigg " criteria = "safe-to-deploy" diff --git a/Cargo.lock b/Cargo.lock index cd5b260c..12d94864 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -305,6 +305,9 @@ name = "bitflags" version = "2.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2261d10cca569e4643e526d8dc2e62e433cc8aba21ab764233731f8d369bf394" +dependencies = [ + "serde", +] [[package]] name = "block-buffer" @@ -736,6 +739,7 @@ dependencies = [ "mio", "parking_lot", "rustix", + "serde", "signal-hook", "signal-hook-mio", "winapi", @@ -963,7 +967,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.61.0", + "windows-sys 0.60.2", ] [[package]] @@ -1106,7 +1110,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.0", + "windows-sys 0.60.2", ] [[package]] @@ -1181,6 +1185,17 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "fd-lock" +version = "4.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" +dependencies = [ + "cfg-if", + "rustix", + "windows-sys 0.59.0", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -1902,7 +1917,6 @@ dependencies = [ "bitflags", "crossterm", "dyn-clone", - "tempfile", "unicode-segmentation", "unicode-width", ] @@ -1961,6 +1975,15 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.15" @@ -2276,9 +2299,11 @@ name = "jp_editor" version = "0.1.0" dependencies = [ "camino", - "open-editor", + "camino-tempfile", + "duct", "serde", "serde_json", + "thiserror 2.0.18", ] [[package]] @@ -2311,8 +2336,10 @@ dependencies = [ name = "jp_inquire" version = "0.1.0" dependencies = [ + "crossterm", "inquire", "parking_lot", + "reedline", ] [[package]] @@ -2340,7 +2367,6 @@ dependencies = [ "jp_tool", "minijinja", "ollama-rs", - "open-editor", "openai_responses", "paste", "quick-xml", @@ -2890,15 +2916,6 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4895175b425cb1f87721b59f0f286c2092bd4af812243672510e1ac53e2e0ad" -[[package]] -name = "open-editor" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9fd068aaa1a8a41846aa26201e1796842f9083b5436f08cf505b7ca6767c40e5" -dependencies = [ - "which", -] - [[package]] name = "openai_responses" version = "0.1.6" @@ -3359,6 +3376,25 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "reedline" +version = "0.48.0" +source = "git+https://github.com/JeanMertz/reedline?branch=changes#d6a56e266758c5a28d7b635b20611dfb50c6acfb" +dependencies = [ + "chrono", + "crossterm", + "fd-lock", + "itertools", + "nu-ansi-term", + "serde", + "strip-ansi-escapes", + "strum", + "thiserror 2.0.18", + "unicase", + "unicode-segmentation", + "unicode-width", +] + [[package]] name = "ref-cast" version = "1.0.24" @@ -3603,7 +3639,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.0", + "windows-sys 0.60.2", ] [[package]] @@ -4249,6 +4285,27 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "strum" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af23d6f6c1a224baef9d3f61e287d2761385a5b88fdab4eb4c6f11aeb54c4bcf" +dependencies = [ + "strum_macros", +] + +[[package]] +name = "strum_macros" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7695ce3845ea4b33927c055a39dc438a45b059f7c1b3d91d38d10355fb8cbca7" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "subtle" version = "2.6.1" @@ -4323,7 +4380,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix", - "windows-sys 0.61.0", + "windows-sys 0.60.2", ] [[package]] @@ -5148,7 +5205,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.0", + "windows-sys 0.60.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 0ce35e16..1b5c5401 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,6 @@ libc = { version = "0.2", default-features = false } linkme = { version = "0.3", default-features = false } minijinja = { version = "2", default-features = false } ollama-rs = { version = "0.3", default-features = false } -open-editor = { version = "1", default-features = false } openai_responses = { version = "0.1", default-features = false } parking_lot = { version = "0.12", default-features = false, features = ["arc_lock"] } paste = { version = "1", default-features = false } @@ -101,6 +100,7 @@ quick-xml = { version = "0.39", default-features = false } quote = { version = "1", default-features = false } ra-ap-rustc_lexer = { version = "0.167", default-features = false } rayon = { version = "1", default-features = false } +reedline = { git = "https://github.com/JeanMertz/reedline", branch = "changes", default-features = false } # fork: adds `Reedline::with_output` + `/dev/tty` cursor/escape routing (RFD 088) reflink-copy = { version = "0.1", default-features = false } regex = { version = "1", default-features = false } relative-path = { version = "2", default-features = false } diff --git a/crates/jp_cli/src/cmd/conversation/edit.rs b/crates/jp_cli/src/cmd/conversation/edit.rs index d82fa3f7..c6c07d8e 100644 --- a/crates/jp_cli/src/cmd/conversation/edit.rs +++ b/crates/jp_cli/src/cmd/conversation/edit.rs @@ -3,7 +3,6 @@ use std::{fmt::Write as _, fs, str::FromStr, time::Duration}; use camino::Utf8PathBuf; use chrono::Utc; use crossterm::style::Stylize as _; -use duct::Expression; use inquire::Confirm; use jp_config::{ AppConfig, PartialAppConfig, ToPartial as _, model::id::PartialModelIdOrAliasConfig, @@ -13,6 +12,7 @@ use jp_conversation::{ event::{ChatRequest, ChatResponse}, thread::ThreadBuilder, }; +use jp_editor::{EditOutcome, EditRequest}; use jp_llm::{ event::Event, event_builder::EventBuilder, @@ -35,6 +35,7 @@ use crate::{ lock::{LockOutcome, LockRequest, acquire_lock}, }, ctx::Ctx, + editor::build_editor_backend, error::Error, }; @@ -129,7 +130,7 @@ impl Edit { /// rather than prompting. fn run_open_editor(self, ctx: &mut Ctx, handles: &[ConversationHandle]) -> Output { let config = ctx.config(); - let cmd = config.editor.command().ok_or(Error::MissingEditor)?; + let editor = build_editor_backend(&config.editor).ok_or(Error::MissingEditor)?; let fs = ctx .fs_backend @@ -156,7 +157,22 @@ impl Edit { .collect(); loop { - Self::spawn_editor(&cmd, &paths)?; + let outcome = editor + .edit_file(EditRequest { + paths: &paths, + cwd: None, + }) + .map_err(|error| Error::Editor(error.to_string()))?; + + if outcome == EditOutcome::Cancelled { + // The editor aborted (non-zero exit). Discard any partial edits + // and leave the conversation unchanged. + Self::restore_snapshots(&snapshots)?; + return Err(Error::Editor( + "the editor was cancelled; the conversation is unchanged".to_owned(), + ) + .into()); + } let Err(error) = self.validate_edits(&fs, &ids) else { break; @@ -215,28 +231,6 @@ impl Edit { Ok(()) } - /// Spawn `$EDITOR` on the given files. - fn spawn_editor(cmd: &Expression, paths: &[Utf8PathBuf]) -> Output { - let args: Vec = paths.iter().map(|p| p.as_str().to_owned()).collect(); - let output = cmd - .clone() - .before_spawn(move |cmd| { - for arg in &args { - cmd.arg(arg); - } - Ok(()) - }) - .unchecked() - .run()?; - - if !output.status.success() { - return Err( - Error::Editor(format!("Editor exited with error: {}", output.status)).into(), - ); - } - Ok(()) - } - /// Restore files to their pre-edit content, removing ones that were absent. fn restore_snapshots(snapshots: &[(Utf8PathBuf, Option)]) -> Output { for (path, content) in snapshots { diff --git a/crates/jp_cli/src/cmd/query.rs b/crates/jp_cli/src/cmd/query.rs index a9eff8d6..4c92f794 100644 --- a/crates/jp_cli/src/cmd/query.rs +++ b/crates/jp_cli/src/cmd/query.rs @@ -124,7 +124,7 @@ use crate::{ lock::{LockRequest, acquire_lock}, }, ctx::IntoPartialAppConfig, - editor::{self, Editor}, + editor, error::{Error, Result}, output::print_json, parser::AttachmentUrlOrPath, @@ -211,20 +211,16 @@ pub(crate) struct Query { #[arg(short = 'a', long = "attachment", alias = "attach")] attachments: Vec, - /// Whether and how to edit the query. + /// Open an editor to compose the query. /// - /// Setting this flag to `true`, omitting it, or using it as a boolean flag - /// (e.g. - /// `--edit`) will use the default editor configured elsewhere, or return an - /// error if no editor is configured and one is required. + /// Forces the editor open even when a query is provided inline (or piped on + /// stdin). + /// Use `--no-edit` (`-E`) to suppress it. /// - /// If set to `false`, the editor will be disabled (similar to `--no-edit`), - /// which might result in an error if the editor is required. - /// - /// If set to any other value, it will be used as the command to open the - /// editor. + /// To change which editor is used, set `editor.cmd` in your config (or pass + /// `--cfg editor.cmd=...`). #[arg(short = 'e', long = "edit", conflicts_with = "no_edit")] - edit: Option>, + edit: bool, /// Do not edit the query. /// @@ -771,10 +767,10 @@ impl Query { return Ok((false, PartialAppConfig::empty())); } - let editor = match config.editor.command() { + let backend = match editor::build_editor_backend(&config.editor) { None if !request.is_empty() => return Ok((false, PartialAppConfig::empty())), None => return Err(Error::MissingEditor), - Some(cmd) => cmd, + Some(backend) => backend, }; let (content, editor_provided_config) = editor::edit_query( @@ -782,7 +778,7 @@ impl Query { conversation_root, stream, request.as_str(), - editor, + backend.as_ref(), None, )?; request.content = content; @@ -871,17 +867,17 @@ impl Query { /// This can be used for example when requesting a tool call without needing /// additional context to be provided. fn force_no_edit(&self) -> bool { - self.no_edit || matches!(self.edit, Some(Some(Editor::Disabled))) + self.no_edit } /// Returns `true` if editing is explicitly enabled. /// - /// This means the `--edit` flag was provided (but not `--edit=false`), or + /// This means the `--edit` flag was provided (and not `--no-edit`), or /// `--quote` was provided (which implies editing). /// In either case the editor should be opened, regardless of whether a /// query is provided as an argument. fn force_edit(&self) -> bool { - !self.force_no_edit() && (self.edit.is_some() || self.quote) + !self.force_no_edit() && (self.edit || self.quote) } #[must_use] @@ -1258,8 +1254,8 @@ impl IntoPartialAppConfig for Query { local: _, no_local: _, attachments, - edit, - no_edit, + edit: _, + no_edit: _, quote: _, tool_use, no_tool_use, @@ -1280,7 +1276,6 @@ impl IntoPartialAppConfig for Query { } = &self; apply_model(&mut partial, model.as_deref(), merged_config); - apply_editor(&mut partial, edit.as_ref().map(|v| v.as_ref()), *no_edit); // Inject builtin tool configs before tool-enable processing. for (name, config) in tool::builtins::all() { @@ -1391,22 +1386,6 @@ fn apply_model(partial: &mut PartialAppConfig, model: Option<&str>, _: Option<&P partial.assistant.model.id = id.into(); } -/// Apply the CLI editor configuration to the partial configuration. -fn apply_editor(partial: &mut PartialAppConfig, editor: Option>, no_edit: bool) { - let Some(Some(editor)) = editor else { - return; - }; - - match (no_edit, editor) { - (true, _) | (_, Editor::Disabled) => { - partial.editor.cmd = None; - partial.editor.envs = None; - } - (_, Editor::Default) => {} - (_, Editor::Command(cmd)) => partial.editor.cmd = Some(cmd.clone()), - } -} - fn apply_enable_tools( partial: &mut PartialAppConfig, directives: &ToolDirectives, diff --git a/crates/jp_cli/src/cmd/query/interrupt.rs b/crates/jp_cli/src/cmd/query/interrupt.rs index 09166765..fd22f08f 100644 --- a/crates/jp_cli/src/cmd/query/interrupt.rs +++ b/crates/jp_cli/src/cmd/query/interrupt.rs @@ -6,5 +6,5 @@ pub(crate) mod handler; pub(crate) mod signals; -pub(crate) use handler::InterruptAction; +pub(crate) use handler::{InterruptAction, reply_edit_mode}; pub(crate) use signals::{LoopAction, handle_llm_event, handle_streaming_signal}; diff --git a/crates/jp_cli/src/cmd/query/interrupt/handler.rs b/crates/jp_cli/src/cmd/query/interrupt/handler.rs index 0551f270..779b4dda 100644 --- a/crates/jp_cli/src/cmd/query/interrupt/handler.rs +++ b/crates/jp_cli/src/cmd/query/interrupt/handler.rs @@ -1,12 +1,21 @@ //! Interrupt handling for the query stream pipeline. //! //! When the user presses Ctrl+C during a query, the `InterruptHandler` presents -//! a context-aware menu based on the current state (streaing vs tool +//! a context-aware menu based on the current state (streaming vs tool //! execution). //! //! The handler returns an [`InterruptAction`] that the caller can use to //! determine the next step. //! +//! ## Replies +//! +//! Choosing to reply (`r` while streaming, `s` while tools run) opens the +//! inline reply widget ([`jp_inquire::InlineReply`]), which renders to the +//! caller's `/dev/tty` writer and offers a `Ctrl+X` escape to the configured +//! external editor. +//! Setting `interrupt.{streaming,tool_call}.reply_in_editor` skips the inline +//! step and opens the editor directly. +//! //! ## Testing //! //! The handler uses dependency injection via [`PromptBackend`] to enable @@ -17,15 +26,21 @@ //! [`MockPromptBackend`]: jp_inquire::prompt::MockPromptBackend //! [`TerminalPromptBackend`]: jp_inquire::prompt::TerminalPromptBackend -use std::io::Write; +use std::sync::Arc; -use jp_config::interrupt::{ - StreamingInterruptAction, StreamingInterruptConfig, ToolInterruptAction, ToolInterruptConfig, +use jp_config::{ + editor::InlineEditMode, + interrupt::{ + StreamingInterruptAction, StreamingInterruptConfig, ToolInterruptAction, + ToolInterruptConfig, + }, }; +use jp_editor::{EditOutcome, EditorBackend}; use jp_inquire::{ - InlineOption, + InlineOption, ReplyEditMode, ReplyOutcome, prompt::{PromptBackend, TerminalPromptBackend}, }; +use jp_printer::Printer; /// Default response sent to the LLM when the user cancels a tool without /// supplying a custom message. @@ -36,6 +51,14 @@ const DEFAULT_TOOL_CANCELLED_RESPONSE: &str = indoc::concatdoc! {" in the conversation.\ "}; +/// Map the configured inline edit mode onto the reply widget's edit mode. +pub(crate) fn reply_edit_mode(mode: InlineEditMode) -> ReplyEditMode { + match mode { + InlineEditMode::Emacs => ReplyEditMode::Emacs, + InlineEditMode::Vi => ReplyEditMode::Vi, + } +} + /// Actions that can be taken after an interrupt. #[derive(Debug, Clone, PartialEq, Eq)] pub enum InterruptAction { @@ -68,6 +91,18 @@ pub enum InterruptAction { ToolCancelled { response: String }, } +/// Outcome of collecting a reply from the user. +enum ReplyResult { + /// The user submitted a non-empty reply. + Reply(String), + + /// The user backed out: an empty submission, a cancel, or an emptied or + /// aborted editor. + /// The call site decides what backing out means (return to the menu, use a + /// canned message, …). + Back, +} + /// Handles user interrupts (Ctrl+C) during query execution. /// /// This handler presents interactive menus and returns the user's chosen @@ -77,71 +112,90 @@ pub enum InterruptAction { /// Uses [`PromptBackend`] for dependency injection, enabling testing without a /// TTY. pub struct InterruptHandler { + /// Backend that renders the menu and the inline reply prompt. backend: P, + + /// The configured editor, used for the `Ctrl+X` escape and the + /// `reply_in_editor` opt-in. + /// `None` when no editor is configured; the inline widget still works. + editor: Option>, + + /// The inline reply buffer's editing style. + edit_mode: ReplyEditMode, } impl Default for InterruptHandler { fn default() -> Self { - Self::new() + Self::with_backend(TerminalPromptBackend, None, ReplyEditMode::Emacs) } } -impl InterruptHandler { - /// Create a new interrupt handler. - pub fn new() -> Self { +impl InterruptHandler

{ + /// Create an interrupt handler with a custom prompt backend, an optional + /// editor (for the reply escape hatch), and the inline edit mode. + pub fn with_backend( + backend: P, + editor: Option>, + edit_mode: ReplyEditMode, + ) -> Self { Self { - backend: TerminalPromptBackend, + backend, + editor, + edit_mode, } } -} - -impl InterruptHandler

{ - /// Create an interrupt handler with a custom prompt backend. - pub fn with_backend(backend: P) -> Self { - Self { backend } - } /// Handle an interrupt during LLM streaming. /// /// When `config.action` is `prompt` the interrupt menu is shown; otherwise /// the configured action runs directly without a menu. - /// A `reply` still prompts for the reply text. + /// Choosing `reply` collects a reply; backing out of a menu-driven reply + /// returns to the menu, while a configured (menu-less) `reply` resumes. pub fn handle_streaming_interrupt( &self, config: &StreamingInterruptConfig, - writer: &mut dyn Write, + printer: &Printer, stream_alive: bool, ) -> InterruptAction { - let choice = match config.action { - StreamingInterruptAction::Prompt => { - let options = vec![ - InlineOption::new('c', "Continue"), - InlineOption::new('r', "Reply (stop & respond)"), - InlineOption::new('s', "Stop (save & exit)"), - InlineOption::new('a', "Abort (discard & exit)"), - ]; + let menu = config.action == StreamingInterruptAction::Prompt; - self.backend - .inline_select("Interrupted", options, None, writer) - .unwrap_or('s') - } - StreamingInterruptAction::Continue => 'c', - StreamingInterruptAction::Reply => 'r', - StreamingInterruptAction::Stop => 's', - StreamingInterruptAction::Abort => 'a', - }; + loop { + let choice = match config.action { + StreamingInterruptAction::Prompt => { + let options = vec![ + InlineOption::new('c', "Continue"), + InlineOption::new('r', "Reply (stop & respond)"), + InlineOption::new('s', "Stop (save & exit)"), + InlineOption::new('a', "Abort (discard & exit)"), + ]; - match choice { - 'c' if stream_alive => InterruptAction::Resume, - 'c' => InterruptAction::Continue, - 'r' => InterruptAction::Reply( - self.backend - .text_input("Reply:", writer) - .unwrap_or_default(), - ), - 's' => InterruptAction::Stop, - 'a' => InterruptAction::Abort, - _ => unreachable!("unexpected choice"), + // A cancelled menu falls back to a graceful stop. (RFD 045's + // `Escalated` outcome is not yet implemented; this is the + // graceful-shutdown stand-in.) + self.backend + .inline_select("Interrupted", options, None, &mut printer.prompt_writer()) + .unwrap_or('s') + } + StreamingInterruptAction::Continue => 'c', + StreamingInterruptAction::Reply => 'r', + StreamingInterruptAction::Stop => 's', + StreamingInterruptAction::Abort => 'a', + }; + + 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:", config.reply_in_editor, printer) { + ReplyResult::Reply(text) => return InterruptAction::Reply(text), + // Backing out of a menu-driven reply re-shows the menu (the + // loop iterates); a configured (menu-less) reply resumes. + ReplyResult::Back if menu => {} + ReplyResult::Back => return InterruptAction::Resume, + }, + _ => unreachable!("unexpected interrupt choice"), + } } } @@ -149,16 +203,15 @@ impl InterruptHandler

{ /// /// Presents a menu with options to stop & reply, restart, or continue /// waiting. - /// When the user chooses "Stop & Reply", they can supply a custom message. - /// An empty input produces a canned default. + /// When the user chooses "Stop & Reply", they can supply a custom message; + /// an empty or cancelled reply produces the canned default. /// /// When `config.action` is `prompt` the interrupt menu is shown; otherwise /// the configured action runs directly without a menu. - /// A `stop_reply` still prompts for the reply text. pub fn handle_tool_interrupt( &self, config: &ToolInterruptConfig, - writer: &mut dyn Write, + printer: &Printer, ) -> InterruptAction { let choice = match config.action { ToolInterruptAction::Prompt => { @@ -169,7 +222,7 @@ impl InterruptHandler

{ ]; self.backend - .inline_select("Interrupted", options, None, writer) + .inline_select("Interrupted", options, None, &mut printer.prompt_writer()) .unwrap_or('c') } ToolInterruptAction::Continue => 'c', @@ -179,22 +232,88 @@ impl InterruptHandler

{ match choice { 'c' => InterruptAction::Resume, + 'r' => InterruptAction::RestartTool, 's' => { - let response = self - .backend - .text_input("Reply:", writer) - .unwrap_or_default(); - - let response = if response.trim().is_empty() { - DEFAULT_TOOL_CANCELLED_RESPONSE.to_owned() - } else { - response + // An empty or cancelled reply falls through to the canned + // message, preserving the "interrupt a tool with no + // explanation" shortcut. + let response = match self.collect_reply("Reply:", config.reply_in_editor, printer) { + ReplyResult::Reply(text) => text, + ReplyResult::Back => DEFAULT_TOOL_CANCELLED_RESPONSE.to_owned(), }; InterruptAction::ToolCancelled { response } } - 'r' => InterruptAction::RestartTool, - _ => unreachable!(), + _ => unreachable!("unexpected interrupt choice"), + } + } + + /// Collect a reply, honoring the straight-to-editor opt-in. + /// + /// With `reply_in_editor` set and an editor configured, opens the editor + /// seeded empty; otherwise collects through the inline widget. + fn collect_reply( + &self, + message: &str, + reply_in_editor: bool, + printer: &Printer, + ) -> ReplyResult { + 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. + return self.collect_reply_inline(message, printer); + }; + + return match editor.edit_text("") { + Ok((EditOutcome::Saved, text)) if !text.trim().is_empty() => { + ReplyResult::Reply(text) + } + // Empty, cancelled, or a spawn failure: back out. + _ => ReplyResult::Back, + }; + } + + self.collect_reply_inline(message, printer) + } + + /// Collect a reply through the inline widget, looping on the editor escape. + /// + /// Prompt errors and `Ctrl+C` are handled explicitly (never swallowed): a + /// non-`Submit` outcome or an error backs out. + fn collect_reply_inline(&self, message: &str, printer: &Printer) -> ReplyResult { + let mut buffer = String::new(); + loop { + let output = printer.owned_prompt_writer(); + match self + .backend + .inline_reply(message, &buffer, self.edit_mode, output) + { + Ok(ReplyOutcome::OpenEditor { current_text }) => { + let Some(editor) = self.editor.as_ref() else { + // No editor configured: the escape is a no-op, the + // widget stays open with the buffer intact. + continue; + }; + + match editor.edit_text(¤t_text) { + Ok((EditOutcome::Saved, edited)) if !edited.trim().is_empty() => { + // Re-seed the inline prompt with the editor's output. + buffer = edited; + } + // Emptied, cancelled, or failed: back out. + _ => return ReplyResult::Back, + } + } + Ok(ReplyOutcome::Submit(text)) if !text.is_empty() => { + return ReplyResult::Reply(text); + } + // An empty submission, a `Ctrl+C` cancel, or a prompt error all + // back out. + Ok(ReplyOutcome::Submit(_) | ReplyOutcome::Cancelled) | Err(_) => { + return ReplyResult::Back; + } + } } } } diff --git a/crates/jp_cli/src/cmd/query/interrupt/handler_tests.rs b/crates/jp_cli/src/cmd/query/interrupt/handler_tests.rs index e8f900b7..952e741d 100644 --- a/crates/jp_cli/src/cmd/query/interrupt/handler_tests.rs +++ b/crates/jp_cli/src/cmd/query/interrupt/handler_tests.rs @@ -1,4 +1,7 @@ -use jp_inquire::prompt::MockPromptBackend; +use std::sync::Arc; + +use jp_editor::MockEditorBackend; +use jp_inquire::{ReplyEditMode, ReplyOutcome, prompt::MockPromptBackend}; use jp_printer::{OutputFormat, Printer}; use super::*; @@ -8,185 +11,231 @@ fn make_printer() -> Printer { printer } -/// Streaming config with the given action. +/// Streaming config with the given action and no straight-to-editor opt-in. fn streaming(action: StreamingInterruptAction) -> StreamingInterruptConfig { - StreamingInterruptConfig { action } + StreamingInterruptConfig { + action, + reply_in_editor: false, + } } -/// Tool config with the given action. +/// Tool config with the given action and no straight-to-editor opt-in. fn tool(action: ToolInterruptAction) -> ToolInterruptConfig { - ToolInterruptConfig { action } + ToolInterruptConfig { + action, + reply_in_editor: false, + } } -#[test] -fn test_streaming_interrupt_stop() { - let backend = MockPromptBackend::new().with_inline_responses(['s']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); +/// Build a handler with no editor configured (emacs inline editing). +fn handler(backend: MockPromptBackend) -> InterruptHandler { + InterruptHandler::with_backend(backend, None, ReplyEditMode::Emacs) +} +/// Build a handler with a (mock) editor for the reply escape hatch. +fn handler_with_editor( + backend: MockPromptBackend, + editor: MockEditorBackend, +) -> InterruptHandler { + InterruptHandler::with_backend(backend, Some(Arc::new(editor)), ReplyEditMode::Emacs) +} + +#[test] +fn streaming_interrupt_stop() { + let handler = handler(MockPromptBackend::new().with_inline_responses(['s'])); let action = handler.handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Stop); } #[test] -fn test_streaming_interrupt_abort() { - let backend = MockPromptBackend::new().with_inline_responses(['a']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - +fn streaming_interrupt_abort() { + let handler = handler(MockPromptBackend::new().with_inline_responses(['a'])); let action = handler.handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Abort); } #[test] -fn test_streaming_interrupt_reply() { +fn streaming_interrupt_reply_submits() { let backend = MockPromptBackend::new() .with_inline_responses(['r']) - .with_text_responses(["my reply message"]); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_streaming_interrupt( + .with_reply_outcomes([ReplyOutcome::Submit("my reply message".into())]); + let action = handler(backend).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Reply("my reply message".into())); } #[test] -fn test_streaming_interrupt_reply_empty_on_cancel() { - // No text response - simulates user canceling the text input - let backend = MockPromptBackend::new().with_inline_responses(['r']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); +fn streaming_interrupt_reply_cancel_returns_to_menu() { + // `r` → cancel the reply → back to the menu → `s` → Stop. + let backend = MockPromptBackend::new() + .with_inline_responses(['r', 's']) + .with_reply_outcomes([ReplyOutcome::Cancelled]); + let action = handler(backend).handle_streaming_interrupt( + &streaming(StreamingInterruptAction::Prompt), + &make_printer(), + true, + ); + assert_eq!(action, InterruptAction::Stop); +} - let action = handler.handle_streaming_interrupt( +#[test] +fn streaming_interrupt_reply_empty_returns_to_menu_then_submits() { + // `r` → empty submit → back to menu → `r` → submit "second try". + let backend = MockPromptBackend::new() + .with_inline_responses(['r', 'r']) + .with_reply_outcomes([ + ReplyOutcome::Submit(String::new()), + ReplyOutcome::Submit("second try".into()), + ]); + let action = handler(backend).handle_streaming_interrupt( + &streaming(StreamingInterruptAction::Prompt), + &make_printer(), + true, + ); + assert_eq!(action, InterruptAction::Reply("second try".into())); +} + +#[test] +fn streaming_interrupt_open_editor_re_seeds_then_submits() { + // `r` → Ctrl+X → editor returns text → re-seeded inline prompt → submit. + let backend = MockPromptBackend::new() + .with_inline_responses(['r']) + .with_reply_outcomes([ + ReplyOutcome::OpenEditor { + current_text: "draft".into(), + }, + ReplyOutcome::Submit("from the editor, edited inline".into()), + ]); + let editor = MockEditorBackend::always("from the editor"); + let action = handler_with_editor(backend, editor).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), true, ); - assert_eq!(action, InterruptAction::Reply(String::new())); + assert_eq!( + action, + InterruptAction::Reply("from the editor, edited inline".into()) + ); } #[test] -fn test_streaming_interrupt_continue_stream_alive() { - let backend = MockPromptBackend::new().with_inline_responses(['c']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); +fn streaming_interrupt_open_editor_empty_returns_to_menu() { + // `r` → Ctrl+X → editor emptied → back to menu → `s` → Stop. + let backend = MockPromptBackend::new() + .with_inline_responses(['r', 's']) + .with_reply_outcomes([ReplyOutcome::OpenEditor { + current_text: String::new(), + }]); + let editor = MockEditorBackend::empty(); + let action = handler_with_editor(backend, editor).handle_streaming_interrupt( + &streaming(StreamingInterruptAction::Prompt), + &make_printer(), + true, + ); + assert_eq!(action, InterruptAction::Stop); +} +#[test] +fn streaming_interrupt_continue_stream_alive() { + let handler = handler(MockPromptBackend::new().with_inline_responses(['c'])); let action = handler.handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Resume); } #[test] -fn test_streaming_interrupt_continue_stream_dead() { - let backend = MockPromptBackend::new().with_inline_responses(['c']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - +fn streaming_interrupt_continue_stream_dead() { + let handler = handler(MockPromptBackend::new().with_inline_responses(['c'])); let action = handler.handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), false, ); assert_eq!(action, InterruptAction::Continue); } #[test] -fn test_streaming_interrupt_defaults_to_stop_on_error() { - // No responses - will error and default to Stop - let backend = MockPromptBackend::new(); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_streaming_interrupt( +fn streaming_interrupt_defaults_to_stop_on_error() { + // No responses: the menu errors and falls back to Stop. + let action = handler(MockPromptBackend::new()).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Prompt), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Stop); } #[test] -fn test_tool_interrupt_stop_with_custom_response() { +fn tool_interrupt_stop_with_custom_response() { let backend = MockPromptBackend::new() .with_inline_responses(['s']) - .with_text_responses(["don't run this tool"]); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &mut writer); + .with_reply_outcomes([ReplyOutcome::Submit("don't run this tool".into())]); + let action = + handler(backend).handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &make_printer()); assert_eq!(action, InterruptAction::ToolCancelled { response: "don't run this tool".into() }); } #[test] -fn test_tool_interrupt_stop_empty_uses_canned_response() { - // No text response — simulates user pressing Enter on empty input - let backend = MockPromptBackend::new().with_inline_responses(['s']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &mut writer); +fn tool_interrupt_stop_empty_uses_canned_response() { + // An empty reply falls through to the canned rejection message. + let backend = MockPromptBackend::new() + .with_inline_responses(['s']) + .with_reply_outcomes([ReplyOutcome::Submit(String::new())]); + let action = + handler(backend).handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &make_printer()); assert!( matches!(action, InterruptAction::ToolCancelled { response } if response.contains("intentionally rejected")) ); } #[test] -fn test_tool_interrupt_restart() { - let backend = MockPromptBackend::new().with_inline_responses(['r']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); +fn tool_interrupt_stop_cancel_uses_canned_response() { + // A cancelled reply also falls through to the canned message (the + // "interrupt a tool with no explanation" shortcut). + let backend = MockPromptBackend::new() + .with_inline_responses(['s']) + .with_reply_outcomes([ReplyOutcome::Cancelled]); + let action = + handler(backend).handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &make_printer()); + assert!( + matches!(action, InterruptAction::ToolCancelled { response } if response.contains("intentionally rejected")) + ); +} - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &mut writer); +#[test] +fn tool_interrupt_restart() { + let handler = handler(MockPromptBackend::new().with_inline_responses(['r'])); + let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &make_printer()); assert_eq!(action, InterruptAction::RestartTool); } #[test] -fn test_tool_interrupt_continue() { - let backend = MockPromptBackend::new().with_inline_responses(['c']); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &mut writer); +fn tool_interrupt_continue() { + let handler = handler(MockPromptBackend::new().with_inline_responses(['c'])); + let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &make_printer()); assert_eq!(action, InterruptAction::Resume); } #[test] -fn test_tool_interrupt_defaults_to_continue_on_error() { - // No responses - will error and default to Continue - let backend = MockPromptBackend::new(); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &mut writer); +fn tool_interrupt_defaults_to_continue_on_error() { + let action = handler(MockPromptBackend::new()) + .handle_tool_interrupt(&tool(ToolInterruptAction::Prompt), &make_printer()); assert_eq!(action, InterruptAction::Resume); } @@ -195,13 +244,9 @@ fn test_tool_interrupt_defaults_to_continue_on_error() { #[test] fn configured_streaming_stop_skips_menu() { - let handler = InterruptHandler::with_backend(MockPromptBackend::new()); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_streaming_interrupt( + let action = handler(MockPromptBackend::new()).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Stop), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Stop); @@ -209,13 +254,9 @@ fn configured_streaming_stop_skips_menu() { #[test] fn configured_streaming_abort_skips_menu() { - let handler = InterruptHandler::with_backend(MockPromptBackend::new()); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_streaming_interrupt( + let action = handler(MockPromptBackend::new()).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Abort), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Abort); @@ -223,20 +264,16 @@ fn configured_streaming_abort_skips_menu() { #[test] fn configured_streaming_continue_tracks_stream_liveness() { - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let alive = InterruptHandler::with_backend(MockPromptBackend::new()) - .handle_streaming_interrupt( - &streaming(StreamingInterruptAction::Continue), - &mut writer, - true, - ); + let alive = handler(MockPromptBackend::new()).handle_streaming_interrupt( + &streaming(StreamingInterruptAction::Continue), + &make_printer(), + true, + ); assert_eq!(alive, InterruptAction::Resume); - let dead = InterruptHandler::with_backend(MockPromptBackend::new()).handle_streaming_interrupt( + let dead = handler(MockPromptBackend::new()).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Continue), - &mut writer, + &make_printer(), false, ); assert_eq!(dead, InterruptAction::Continue); @@ -244,38 +281,96 @@ fn configured_streaming_continue_tracks_stream_liveness() { #[test] fn configured_streaming_reply_uses_inline_prompt() { - let backend = MockPromptBackend::new().with_text_responses(["changed my mind"]); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_streaming_interrupt( + // A menu-less `reply` collects through the inline widget; backing out + // resumes rather than returning to a (non-existent) menu. + let backend = MockPromptBackend::new() + .with_reply_outcomes([ReplyOutcome::Submit("changed my mind".into())]); + let action = handler(backend).handle_streaming_interrupt( &streaming(StreamingInterruptAction::Reply), - &mut writer, + &make_printer(), true, ); assert_eq!(action, InterruptAction::Reply("changed my mind".into())); } #[test] -fn configured_tool_restart_skips_menu() { - let handler = InterruptHandler::with_backend(MockPromptBackend::new()); - let printer = make_printer(); - let mut writer = printer.out_writer(); +fn configured_streaming_reply_cancel_resumes() { + let backend = MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Cancelled]); + let action = handler(backend).handle_streaming_interrupt( + &streaming(StreamingInterruptAction::Reply), + &make_printer(), + true, + ); + assert_eq!(action, InterruptAction::Resume); +} - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::Restart), &mut writer); +#[test] +fn configured_tool_restart_skips_menu() { + let action = handler(MockPromptBackend::new()) + .handle_tool_interrupt(&tool(ToolInterruptAction::Restart), &make_printer()); assert_eq!(action, InterruptAction::RestartTool); } #[test] fn configured_tool_stop_reply_uses_inline_prompt() { - let backend = MockPromptBackend::new().with_text_responses(["use ripgrep"]); - let handler = InterruptHandler::with_backend(backend); - let printer = make_printer(); - let mut writer = printer.out_writer(); - - let action = handler.handle_tool_interrupt(&tool(ToolInterruptAction::StopReply), &mut writer); + let backend = + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit("use ripgrep".into())]); + let action = handler(backend) + .handle_tool_interrupt(&tool(ToolInterruptAction::StopReply), &make_printer()); assert_eq!(action, InterruptAction::ToolCancelled { response: "use ripgrep".into() }); } + +#[test] +fn reply_in_editor_opens_editor_directly() { + // With `reply_in_editor`, `r` skips the inline widget and sends the editor + // result directly. No reply outcomes are scripted on the prompt backend. + let backend = MockPromptBackend::new().with_inline_responses(['r']); + let editor = MockEditorBackend::always("written in the editor"); + let config = StreamingInterruptConfig { + action: StreamingInterruptAction::Prompt, + reply_in_editor: true, + }; + let action = handler_with_editor(backend, editor).handle_streaming_interrupt( + &config, + &make_printer(), + true, + ); + assert_eq!( + action, + InterruptAction::Reply("written in the editor".into()) + ); +} + +#[test] +fn reply_in_editor_empty_returns_to_menu() { + // `reply_in_editor` with an empty editor result returns to the menu. + let backend = MockPromptBackend::new().with_inline_responses(['r', 's']); + let editor = MockEditorBackend::empty(); + let config = StreamingInterruptConfig { + action: StreamingInterruptAction::Prompt, + reply_in_editor: true, + }; + let action = handler_with_editor(backend, editor).handle_streaming_interrupt( + &config, + &make_printer(), + true, + ); + assert_eq!(action, InterruptAction::Stop); +} + +#[test] +fn reply_in_editor_without_editor_falls_back_to_inline() { + // `reply_in_editor` is set but no editor configured: fall back to the + // inline widget rather than doing nothing. + let backend = MockPromptBackend::new() + .with_inline_responses(['r']) + .with_reply_outcomes([ReplyOutcome::Submit("typed inline".into())]); + let config = StreamingInterruptConfig { + action: StreamingInterruptAction::Prompt, + reply_in_editor: true, + }; + let action = handler(backend).handle_streaming_interrupt(&config, &make_printer(), true); + assert_eq!(action, InterruptAction::Reply("typed inline".into())); +} diff --git a/crates/jp_cli/src/cmd/query/interrupt/signals.rs b/crates/jp_cli/src/cmd/query/interrupt/signals.rs index b5760f4f..53a7b790 100644 --- a/crates/jp_cli/src/cmd/query/interrupt/signals.rs +++ b/crates/jp_cli/src/cmd/query/interrupt/signals.rs @@ -8,9 +8,12 @@ //! 2. Delegates state transitions to the `TurnCoordinator` state machine //! 3. Returns a `LoopAction` for the caller to handle control flow +use std::sync::Arc; + use jp_config::interrupt::{StreamingInterruptConfig, ToolInterruptConfig}; use jp_conversation::ConversationStream; -use jp_inquire::prompt::PromptBackend; +use jp_editor::EditorBackend; +use jp_inquire::{ReplyEditMode, prompt::PromptBackend}; use jp_llm::event::{Event, EventMatcher, EventPatch, FinishReason, PatchAction}; use jp_printer::Printer; use tokio_util::sync::CancellationToken; @@ -51,6 +54,8 @@ pub fn handle_streaming_signal( conversation_stream: &mut ConversationStream, printer: &Printer, backend: &dyn PromptBackend, + editor: Option>, + edit_mode: ReplyEditMode, config: &StreamingInterruptConfig, llm_stream_finished: bool, ) -> LoopAction<()> { @@ -73,11 +78,8 @@ pub fn handle_streaming_signal( turn_coordinator.flush_renderer(); printer.flush_instant(); - let action = InterruptHandler::with_backend(backend).handle_streaming_interrupt( - config, - &mut printer.prompt_writer(), - !llm_stream_finished, - ); + let action = InterruptHandler::with_backend(backend, editor, edit_mode) + .handle_streaming_interrupt(config, printer, !llm_stream_finished); // `Resume` means "keep waiting for the current stream." The state // machine is a no-op for it, and we must NOT break the inner loop: @@ -227,6 +229,8 @@ pub fn handle_tool_signal( is_prompting: bool, printer: &Printer, backend: &dyn PromptBackend, + editor: Option>, + edit_mode: ReplyEditMode, config: &ToolInterruptConfig, ) -> ToolSignalResult { match signal { @@ -247,8 +251,8 @@ pub fn handle_tool_signal( return ToolSignalResult::Continue; } - let action = InterruptHandler::with_backend(backend) - .handle_tool_interrupt(config, &mut printer.prompt_writer()); + let action = InterruptHandler::with_backend(backend, editor, edit_mode) + .handle_tool_interrupt(config, printer); // Notify the state machine (reserved for future state transitions). turn_coordinator.handle_tool_interrupt(&action); diff --git a/crates/jp_cli/src/cmd/query/interrupt/signals_tests.rs b/crates/jp_cli/src/cmd/query/interrupt/signals_tests.rs index 3c1f9772..9e32220c 100644 --- a/crates/jp_cli/src/cmd/query/interrupt/signals_tests.rs +++ b/crates/jp_cli/src/cmd/query/interrupt/signals_tests.rs @@ -3,7 +3,10 @@ use std::sync::Arc; use assert_matches::assert_matches; use jp_config::AppConfig; use jp_conversation::{ConversationStream, event::ChatRequest}; -use jp_inquire::prompt::{MockPromptBackend, TerminalPromptBackend}; +use jp_inquire::{ + ReplyEditMode, ReplyOutcome, + prompt::{MockPromptBackend, TerminalPromptBackend}, +}; use jp_printer::{OutputFormat, Printer}; use super::*; @@ -56,6 +59,8 @@ fn streaming_signal_resume_continues_without_breaking_loop() { &mut stream, &printer, &backend, + None, + ReplyEditMode::Emacs, &streaming_prompt(), false, // stream NOT finished -> stream alive -> Resume path ); @@ -91,6 +96,8 @@ fn streaming_signal_continue_breaks_for_prefill_request() { &mut stream, &printer, &backend, + None, + ReplyEditMode::Emacs, &streaming_prompt(), true, // stream finished -> dead -> Continue path ); @@ -114,6 +121,8 @@ fn streaming_signal_quit_breaks_for_persist() { &mut stream, &printer, &TerminalPromptBackend, + None, + ReplyEditMode::Emacs, &streaming_prompt(), false, // stream not finished ); @@ -136,6 +145,8 @@ fn tool_signal_quit_cancels_and_continues() { false, // not prompting &printer, &TerminalPromptBackend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -160,6 +171,8 @@ fn regression_streaming_quit_must_not_skip_persistence() { &mut stream, &printer, &TerminalPromptBackend, + None, + ReplyEditMode::Emacs, &streaming_prompt(), false, // stream not finished ); @@ -187,6 +200,8 @@ fn regression_tool_quit_must_not_skip_persistence() { false, // not prompting &printer, &TerminalPromptBackend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -218,6 +233,8 @@ fn tool_signal_shutdown_restart_returns_restart() { false, // not prompting &printer, &backend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -246,6 +263,8 @@ fn tool_signal_shutdown_cancelled_returns_cancelled_with_canned_response() { false, // not prompting &printer, &backend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -267,7 +286,7 @@ fn tool_signal_shutdown_cancelled_with_custom_response() { // Mock user selecting 's' (Stop & Reply) then typing a message let backend = MockPromptBackend::new() .with_inline_responses(['s']) - .with_text_responses(["wrong tool, use grep instead"]); + .with_reply_outcomes([ReplyOutcome::Submit("wrong tool, use grep instead".into())]); let result = handle_tool_signal( SignalTo::Shutdown, @@ -276,6 +295,8 @@ fn tool_signal_shutdown_cancelled_with_custom_response() { false, // not prompting &printer, &backend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -303,6 +324,8 @@ fn tool_signal_shutdown_resume_continues_without_cancel() { false, // not prompting &printer, &backend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -331,6 +354,8 @@ fn tool_signal_shutdown_suppressed_when_prompting() { true, // prompting &printer, &backend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); @@ -360,6 +385,8 @@ fn tool_signal_shutdown_not_suppressed_when_not_prompting() { false, // not prompting &printer, &backend, + None, + ReplyEditMode::Emacs, &tool_prompt(), ); diff --git a/crates/jp_cli/src/cmd/query/tool/coordinator.rs b/crates/jp_cli/src/cmd/query/tool/coordinator.rs index 9163206f..43d371e1 100644 --- a/crates/jp_cli/src/cmd/query/tool/coordinator.rs +++ b/crates/jp_cli/src/cmd/query/tool/coordinator.rs @@ -97,7 +97,8 @@ use jp_conversation::{ SelectOption, ToolCallRequest, ToolCallResponse, }, }; -use jp_inquire::prompt::PromptBackend; +use jp_editor::EditorBackend; +use jp_inquire::{ReplyEditMode, prompt::PromptBackend}; use jp_llm::tool::executor::{Executor, ExecutorResult, ExecutorSource, PermissionInfo}; use jp_mcp::Client; use jp_printer::Printer; @@ -828,6 +829,8 @@ impl ToolCoordinator { turn_state: &mut TurnState, printer: &Printer, prompt_backend: &dyn PromptBackend, + editor: Option>, + edit_mode: ReplyEditMode, inquiry_backend: Arc, conv: &ConversationMut, mcp_client: &Client, @@ -1106,6 +1109,8 @@ impl ToolCoordinator { self.is_prompting(), printer, prompt_backend, + editor.clone(), + edit_mode, &self.interrupt_config, ) { ToolSignalResult::Continue => {} @@ -1312,8 +1317,10 @@ impl ToolCoordinator { }); } result_mode @ (ResultMode::Ask | ResultMode::Edit) => { - let can_prompt = - is_tty && (result_mode == ResultMode::Ask || prompter.has_editor()); + // Both Ask and Edit prompt on any tty: the Edit flow + // uses the inline widget, which no longer requires a + // configured editor. + let can_prompt = is_tty; if can_prompt { if *prompt_active { pending_prompts.push_back(PendingPrompt::ResultMode { diff --git a/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs b/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs index 9af9c10f..706bc42d 100644 --- a/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs +++ b/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs @@ -1,8 +1,7 @@ use async_trait::async_trait; use camino::Utf8PathBuf; use jp_config::conversation::tool::{ToolConfig, ToolSource, style::PartialDisplayStyleConfig}; -use jp_editor::MockEditorBackend; -use jp_inquire::prompt::MockPromptBackend; +use jp_inquire::{ReplyOutcome, prompt::MockPromptBackend}; use jp_printer::{ErrChannel, OutputFormat, Printer}; use schematic::Config as _; @@ -535,16 +534,15 @@ async fn test_resolve_tool_call_decision_invalidates_prerender_on_edit() { }, }); - // The editor returns post-edit args `path: src/bar.rs`. The prompt - // backend picks `e` so the prompter routes through the editor. + // The prompt backend picks `e` (edit arguments) and supplies the post-edit + // JSON through the inline reply widget. let post_edit = serde_json::json!({"path": "src/bar.rs"}); - let editor = MockEditorBackend::json(&post_edit); - let prompt_backend = MockPromptBackend::new().with_inline_responses(['e']); - let prompter = ToolPrompter::with_backends( - printer.clone(), - Some(Arc::new(editor)), - Arc::new(prompt_backend), - ); + let prompt_backend = MockPromptBackend::new() + .with_inline_responses(['e']) + .with_reply_outcomes([ReplyOutcome::Submit( + serde_json::to_string(&post_edit).unwrap(), + )]); + let prompter = ToolPrompter::with_backends(printer.clone(), None, Arc::new(prompt_backend)); let mut turn_state = TurnState::default(); diff --git a/crates/jp_cli/src/cmd/query/tool/prompter.rs b/crates/jp_cli/src/cmd/query/tool/prompter.rs index 111a973e..5546517e 100644 --- a/crates/jp_cli/src/cmd/query/tool/prompter.rs +++ b/crates/jp_cli/src/cmd/query/tool/prompter.rs @@ -12,12 +12,11 @@ use std::{io::Write as _, sync::Arc}; -use camino::Utf8PathBuf; use crossterm::style::Stylize as _; use jp_config::conversation::tool::{RunMode, ToolSource}; use jp_conversation::event::{InquiryId, SelectOption}; -use jp_editor::{EditorBackend, TerminalEditorBackend}; -use jp_inquire::{InlineOption, prompt::PromptBackend}; +use jp_editor::{EditOutcome, EditorBackend}; +use jp_inquire::{InlineOption, ReplyEditMode, ReplyOutcome, prompt::PromptBackend}; use jp_llm::tool::executor::PermissionInfo; use jp_printer::Printer; use jp_tool::AnswerType; @@ -75,7 +74,17 @@ enum EditResult { /// User emptied the content (signals fallback to Ask). Emptied, - /// User cancelled the edit (e.g., JSON error + declined retry). + /// User cancelled the edit (`Ctrl+C`). + Cancelled, +} + +/// Outcome of an inline edit (the reply widget plus the `Ctrl+X` editor +/// escape). +enum InlineEditResult { + /// User pressed Enter; carries the buffer (possibly empty). + Submitted(String), + + /// User cancelled with `Ctrl+C`. Cancelled, } @@ -87,13 +96,16 @@ enum EditResult { /// Uses type-erased backends (`Arc`) to allow runtime injection of /// mock backends for testing. pub struct ToolPrompter { - /// Editor backend for edit modes. - /// If `None`, edit mode falls back to Ask. + /// Editor backend for the `Ctrl+X` escape from the inline editor. + /// `None` when no editor is configured; the inline widget still works. editor: Option>, /// Prompt backend for interactive prompts. prompt_backend: Arc, + /// Editing style for the inline reply widget used by the edit prompts. + edit_mode: ReplyEditMode, + printer: Arc, } @@ -104,15 +116,14 @@ impl ToolPrompter { /// using the real editor backend. pub fn with_prompt_backend( printer: Arc, - editor_path: Option, + editor: Option>, prompt_backend: Arc, + edit_mode: ReplyEditMode, ) -> Self { - let editor = editor_path - .map(|path| Arc::new(TerminalEditorBackend { path }) as Arc); - Self { editor, prompt_backend, + edit_mode, printer, } } @@ -127,21 +138,7 @@ impl ToolPrompter { Self { editor, prompt_backend, - printer, - } - } - - /// Creates a prompter with a custom editor backend. - #[cfg(test)] - pub fn with_editor_backend( - printer: Arc, - backend: impl EditorBackend + 'static, - ) -> Self { - use jp_inquire::prompt::TerminalPromptBackend; - - Self { - editor: Some(Arc::new(backend)), - prompt_backend: Arc::new(TerminalPromptBackend), + edit_mode: ReplyEditMode::Emacs, printer, } } @@ -151,7 +148,7 @@ impl ToolPrompter { /// Based on the `run_mode`, this may: /// /// - Show an interactive prompt (Ask) - /// - Open an editor for arguments (Edit) + /// - Edit the arguments inline (Edit) /// - Return immediately (Unattended) /// /// # Returns @@ -183,23 +180,20 @@ impl ToolPrompter { /// Builds the select options for the permission prompt. /// - /// The available options depend on whether an editor is configured. /// Returns `SelectOption`s that can be rendered as an inline select. - fn permission_options(&self) -> Vec { - let mut opts = vec![ + fn permission_options() -> Vec { + // `r` (skip & reply) and `e` (edit arguments) drive the inline reply + // widget, which needs only a tty — they no longer require a configured + // editor (the `Ctrl+X` escape does, but it is a no-op without one). + vec![ SelectOption::new("y", "Run tool"), SelectOption::new("Y", "Run tool, remember for this turn"), SelectOption::new("n", "Skip running tool"), SelectOption::new("N", "Skip running tool, remember for this turn"), SelectOption::new("p", "Print arguments as JSON"), - ]; - - if self.editor.is_some() { - opts.push(SelectOption::new("r", "Skip and reply")); - opts.push(SelectOption::new("e", "Edit arguments")); - } - - opts + SelectOption::new("r", "Skip and reply"), + SelectOption::new("e", "Edit arguments"), + ] } /// Shows the interactive permission prompt. @@ -214,7 +208,7 @@ impl ToolPrompter { loop { let question = build_permission_question(tool_name, tool_source); - let inline_options = select_options_to_inline(&self.permission_options()); + let inline_options = select_options_to_inline(&Self::permission_options()); let mut writer = self.printer.prompt_writer(); @@ -295,21 +289,17 @@ impl ToolPrompter { } } - /// Opens an editor for the user to modify tool arguments. + /// Edits the tool arguments inline before running. /// - /// If no editor is configured, falls back to Ask mode. /// If the user empties the content, falls back to Ask mode. - /// If JSON parsing fails, prompts to retry or fail. + /// Invalid JSON re-prompts the inline editor with the error in the prompt + /// line. fn prompt_edit( &self, tool_name: &str, tool_source: &ToolSource, arguments: Value, ) -> Result { - let Some(_) = &self.editor else { - return self.prompt_ask(tool_name, tool_source, arguments); - }; - match self.try_edit_arguments(&arguments)? { EditResult::Edited(edited) => Ok(PermissionResult::Run { arguments: edited, @@ -323,98 +313,109 @@ impl ToolPrompter { } } - /// Attempts to edit arguments in an editor. + /// Edit JSON arguments inline, re-prompting on parse errors. /// - /// Returns the edit result without any async recursion. + /// The buffer is seeded with the pretty-printed arguments. + /// `Ctrl+X` escapes to the configured editor; an emptied buffer abandons + /// the edit (fall back to Ask), and a cancel abandons it as cancelled. fn try_edit_arguments(&self, arguments: &Value) -> Result { - let Some(editor) = &self.editor else { - return Err(Error::Editor("No editor configured".to_string())); - }; - - let mut json = serde_json::to_string_pretty(arguments) + let mut text = serde_json::to_string_pretty(arguments) .map_err(|e| Error::Editor(format!("Failed to serialize arguments: {e}")))?; + let mut message = "Edit arguments".to_owned(); loop { - json = editor - .edit(&json) - .map_err(|error| Error::Editor(error.to_string()))?; - - if json.trim().is_empty() { - return Ok(EditResult::Emptied); - } - - match serde_json::from_str::(&json) { - Ok(edited) => return Ok(EditResult::Edited(edited)), - Err(e) => { - let mut writer = self.printer.prompt_writer(); - drop(writeln!(writer, "JSON parsing error: {e}")); - - let options = vec![ - InlineOption::new('y', "Open editor to fix arguments"), - InlineOption::new('n', "Cancel edit"), - ]; - - let retry = self - .prompt_backend - .inline_select("Re-open editor?", options, Some('n'), &mut writer) - .unwrap_or('n'); - - if retry == 'n' { - return Ok(EditResult::Cancelled); + match self.inline_edit(&message, &text)? { + InlineEditResult::Cancelled => return Ok(EditResult::Cancelled), + InlineEditResult::Submitted(edited) => { + if edited.trim().is_empty() { + return Ok(EditResult::Emptied); + } + match serde_json::from_str::(&edited) { + Ok(value) => return Ok(EditResult::Edited(value)), + Err(e) => { + // Re-seed with the user's text and surface the error + // in the prompt line — no process re-spawn. + text = edited; + message = format!("Invalid JSON: {e}"); + } } } } } } - /// Opens an editor for the user to provide reasoning for skipping tool - /// execution. + /// Collect a free-text reason for skipping a tool. /// - /// The editor is pre-populated with a placeholder prompt. - /// If the user empties the content or leaves only the placeholder, a - /// default reason is returned. + /// The buffer is seeded with `placeholder`. + /// Submitting it unchanged, an empty buffer, or a cancel all yield `None` + /// (skip with no reason). fn edit_text(&self, placeholder: &str) -> Result, Error> { - let Some(editor) = &self.editor else { - return Err(Error::Editor("No editor configured".to_string())); - }; - - let content = editor - .edit(placeholder) - .map_err(|error| Error::Editor(error.to_string()))?; - - let trimmed = content.trim(); - if trimmed.is_empty() || trimmed == placeholder { - Ok(None) - } else { - Ok(Some(content)) + match self.inline_edit("Reason for skipping (optional)", placeholder)? { + InlineEditResult::Cancelled => Ok(None), + InlineEditResult::Submitted(content) => { + let trimmed = content.trim(); + if trimmed.is_empty() || trimmed == placeholder { + Ok(None) + } else { + Ok(Some(content)) + } + } } } - /// Opens an editor for the user to modify tool result before delivery to - /// LLM. + /// Edit a tool result inline before delivery to the LLM. /// /// # Returns /// - /// - `Some(edited_result)` if user edited and saved - /// - `None` if user emptied content (caller should fall back to Ask) - /// - /// # Errors - /// - /// Returns an error if no editor is configured or the editor fails. + /// - `Some(edited_result)` if the user submitted non-empty text. + /// - `None` if the user emptied the content or cancelled (caller falls back + /// to Ask). pub fn edit_result(&self, result: &str) -> Result, Error> { - let Some(editor) = &self.editor else { - return Err(Error::Editor("No editor configured".to_string())); - }; - - let content = editor - .edit(result) - .map_err(|error| Error::Editor(error.to_string()))?; - - if content.trim().is_empty() { - return Ok(None); + match self.inline_edit("Edit result before delivery", result)? { + InlineEditResult::Cancelled => Ok(None), + InlineEditResult::Submitted(content) => { + if content.trim().is_empty() { + Ok(None) + } else { + Ok(Some(content)) + } + } } + } - Ok(Some(content)) + /// Run the inline reply widget seeded with `seed`, handling the `Ctrl+X` + /// editor escape internally (re-seeding the widget with the editor's + /// output). + /// + /// Returns when the user submits or cancels. + /// With no editor configured the escape is a no-op and the widget stays + /// open. + fn inline_edit(&self, message: &str, seed: &str) -> Result { + let mut text = seed.to_owned(); + loop { + let output = self.printer.owned_prompt_writer(); + match self + .prompt_backend + .inline_reply(message, &text, self.edit_mode, output) + .map_err(|error| Error::Editor(error.to_string()))? + { + ReplyOutcome::Submit(content) => return Ok(InlineEditResult::Submitted(content)), + ReplyOutcome::Cancelled => return Ok(InlineEditResult::Cancelled), + ReplyOutcome::OpenEditor { current_text } => { + text = current_text; + if let Some(editor) = &self.editor { + match editor + .edit_text(&text) + .map_err(|error| Error::Editor(error.to_string()))? + { + (EditOutcome::Saved, edited) => text = edited, + // Editor cancelled: keep the buffer and re-prompt. + (EditOutcome::Cancelled, _) => {} + } + } + } + } + } } /// Prompts the user for confirmation before delivering tool result to LLM. @@ -428,18 +429,13 @@ impl ToolPrompter { let question = format!("Deliver {} result to assistant?", tool_name.yellow().bold()); - let options = if self.editor.is_some() { - vec![ - InlineOption::new('y', "Deliver result"), - InlineOption::new('n', "Skip delivery"), - InlineOption::new('e', "Edit result first"), - ] - } else { - vec![ - InlineOption::new('y', "Deliver result"), - InlineOption::new('n', "Skip delivery"), - ] - }; + // "Edit result first" uses the inline widget (any tty); it no longer + // requires a configured editor. + let options = vec![ + InlineOption::new('y', "Deliver result"), + InlineOption::new('n', "Skip delivery"), + InlineOption::new('e', "Edit result first"), + ]; match self .prompt_backend @@ -457,11 +453,6 @@ impl ToolPrompter { } } - /// Returns whether an editor is configured. - pub fn has_editor(&self) -> bool { - self.editor.is_some() - } - /// Prompts the user for a tool-specific question. /// /// This handles different question types: diff --git a/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs b/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs index d8f299e9..4d414fcb 100644 --- a/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs +++ b/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs @@ -1,761 +1,410 @@ +use std::sync::Arc; + use jp_editor::MockEditorBackend; -use jp_inquire::prompt::MockPromptBackend; +use jp_inquire::{ReplyOutcome, prompt::MockPromptBackend}; use jp_printer::OutputFormat; +use serde_json::json; use super::*; -/// Creates a test prompter with a mock editor backend (real prompt backend). -fn prompter_with_mock_editor(mock: MockEditorBackend) -> ToolPrompter { - let (printer, _, _) = Printer::memory(OutputFormat::TextPretty); - ToolPrompter::with_editor_backend(Arc::new(printer), mock) +fn printer() -> Arc { + let (printer, _out, _err) = Printer::memory(OutputFormat::TextPretty); + Arc::new(printer) } -/// Creates a test prompter without an editor. -fn prompter_without_editor() -> ToolPrompter { - let (printer, _, _) = Printer::memory(OutputFormat::TextPretty); - ToolPrompter::with_backends(Arc::new(printer), None, Arc::new(MockPromptBackend::new())) +/// Prompter with a mock prompt backend and no editor. +/// +/// The inline widget (the prompt backend), not the editor, drives edits now; +/// the editor is only the `Ctrl+X` escape. +fn prompter(prompt: MockPromptBackend) -> ToolPrompter { + ToolPrompter::with_backends(printer(), None, Arc::new(prompt)) } -/// Creates a test prompter with both mock editor and mock prompt backends. -fn prompter_with_mocks(editor: MockEditorBackend, prompt: MockPromptBackend) -> ToolPrompter { - let (printer, _, _) = Printer::memory(OutputFormat::TextPretty); - ToolPrompter::with_backends(Arc::new(printer), Some(Arc::new(editor)), Arc::new(prompt)) +/// Prompter with a mock prompt backend and a mock editor for the `Ctrl+X` +/// escape. +fn prompter_with_editor(prompt: MockPromptBackend, editor: MockEditorBackend) -> ToolPrompter { + ToolPrompter::with_backends(printer(), Some(Arc::new(editor)), Arc::new(prompt)) } -/// Creates a test prompter with mock prompt backend but no editor. -fn prompter_with_mock_prompt(prompt: MockPromptBackend) -> ToolPrompter { - let (printer, _, _) = Printer::memory(OutputFormat::TextPretty); - ToolPrompter::with_backends(Arc::new(printer), None, Arc::new(prompt)) +fn make_permission_info(run_mode: RunMode, arguments: Value) -> PermissionInfo { + PermissionInfo { + tool_id: "call_123".to_string(), + tool_name: "test_tool".to_string(), + tool_source: ToolSource::Builtin { tool: None }, + run_mode, + arguments, + } } #[test] -fn test_permission_result_variants() { +fn permission_result_variants() { let run = PermissionResult::Run { - arguments: serde_json::json!({"key": "value"}), + arguments: json!({"key": "value"}), persist: false, }; assert!(matches!(run, PermissionResult::Run { .. })); let skip = PermissionResult::Skip { - reason: None, - persist: false, - }; - assert!(matches!(skip, PermissionResult::Skip { .. })); - - let skip_with_reason = PermissionResult::Skip { reason: Some("User cancelled".to_string()), persist: false, }; - assert!(matches!(skip_with_reason, PermissionResult::Skip { + assert!(matches!(skip, PermissionResult::Skip { reason: Some(_), .. })); } -#[test] -fn test_mock_editor_returns_configured_response() { - let mock = MockEditorBackend::always("edited content"); - let result = mock.edit("original").unwrap(); - assert_eq!(result, "edited content"); -} - -#[test] -fn test_mock_editor_returns_responses_in_sequence() { - let mock = MockEditorBackend::with_responses(["first", "second", "third"]); - - assert_eq!(mock.edit("").unwrap(), "first"); - assert_eq!(mock.edit("").unwrap(), "second"); - assert_eq!(mock.edit("").unwrap(), "third"); - // Exhausted, returns empty - assert_eq!(mock.edit("").unwrap(), ""); -} - -#[test] -fn test_mock_editor_empty() { - let mock = MockEditorBackend::empty(); - let result = mock.edit("some content").unwrap(); - assert_eq!(result, ""); -} - -#[test] -fn test_mock_editor_json() { - let value = serde_json::json!({"key": "value"}); - let mock = MockEditorBackend::json(&value); - let result = mock.edit("").unwrap(); - let parsed: Value = serde_json::from_str(&result).unwrap(); - assert_eq!(parsed, value); -} +// --- Argument editing (`try_edit_arguments`) ------------------------------ #[test] -fn test_edit_arguments_returns_modified_json() { - let original = serde_json::json!({"key": "original"}); - let modified = serde_json::json!({"key": "modified"}); - - let mock = MockEditorBackend::json(&modified); - let prompter = prompter_with_mock_editor(mock); +fn edit_arguments_returns_modified_json() { + let modified = json!({"key": "modified"}); + let prompt = MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit( + serde_json::to_string(&modified).unwrap(), + )]); - let result = prompter.try_edit_arguments(&original).unwrap(); + let result = prompter(prompt) + .try_edit_arguments(&json!({"key": "original"})) + .unwrap(); match result { EditResult::Edited(v) => assert_eq!(v, modified), - other => panic!("Expected Edited, got {other:?}"), + other => panic!("expected Edited, got {other:?}"), } } #[test] -fn test_edit_arguments_empty_returns_emptied() { - let original = serde_json::json!({"key": "value"}); - - let mock = MockEditorBackend::empty(); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.try_edit_arguments(&original).unwrap(); - +fn edit_arguments_empty_returns_emptied() { + let prompt = + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit(String::new())]); + let result = prompter(prompt) + .try_edit_arguments(&json!({"key": "value"})) + .unwrap(); assert!(matches!(result, EditResult::Emptied)); } #[test] -fn test_edit_arguments_whitespace_only_returns_emptied() { - let original = serde_json::json!({"key": "value"}); - - let mock = MockEditorBackend::always(" \n\t "); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.try_edit_arguments(&original).unwrap(); - - assert!(matches!(result, EditResult::Emptied)); +fn edit_arguments_cancel_returns_cancelled() { + let prompt = MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Cancelled]); + let result = prompter(prompt) + .try_edit_arguments(&json!({"key": "value"})) + .unwrap(); + assert!(matches!(result, EditResult::Cancelled)); } #[test] -fn test_edit_arguments_no_editor_returns_error() { - let prompter = prompter_without_editor(); - let original = serde_json::json!({"key": "value"}); - - let result = prompter.try_edit_arguments(&original); +fn edit_arguments_invalid_json_reprompts_then_accepts() { + // Invalid JSON re-seeds the inline prompt (no retry menu); a second submit + // with valid JSON succeeds. + let valid = json!({"key": "fixed"}); + let prompt = MockPromptBackend::new().with_reply_outcomes([ + ReplyOutcome::Submit("{ not json }".into()), + ReplyOutcome::Submit(serde_json::to_string(&valid).unwrap()), + ]); - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("No editor configured") - ); + let result = prompter(prompt).try_edit_arguments(&json!({})).unwrap(); + match result { + EditResult::Edited(v) => assert_eq!(v, valid), + other => panic!("expected Edited, got {other:?}"), + } } #[test] -#[allow(clippy::approx_constant)] -fn test_edit_arguments_preserves_complex_json() { - let complex = serde_json::json!({ - "string": "value", - "number": 42, - "float": 3.1415, - "boolean": true, - "null": null, - "array": [1, 2, 3], - "nested": { - "deep": { - "value": "found" - } - } - }); - - let mock = MockEditorBackend::json(&complex); - let prompter = prompter_with_mock_editor(mock); +fn edit_arguments_invalid_json_then_cancel() { + let prompt = MockPromptBackend::new().with_reply_outcomes([ + ReplyOutcome::Submit("{ not json }".into()), + ReplyOutcome::Cancelled, + ]); + let result = prompter(prompt).try_edit_arguments(&json!({})).unwrap(); + assert!(matches!(result, EditResult::Cancelled)); +} - let result = prompter.try_edit_arguments(&serde_json::json!({})).unwrap(); +#[test] +fn edit_arguments_open_editor_re_seeds_then_submits() { + // Ctrl+X opens the editor; its output is re-seeded and submitted inline. + let modified = json!({"key": "from editor"}); + let prompt = MockPromptBackend::new().with_reply_outcomes([ + ReplyOutcome::OpenEditor { + current_text: "{}".into(), + }, + ReplyOutcome::Submit(serde_json::to_string(&modified).unwrap()), + ]); + let editor = MockEditorBackend::always(serde_json::to_string(&modified).unwrap()); + let result = prompter_with_editor(prompt, editor) + .try_edit_arguments(&json!({})) + .unwrap(); match result { - EditResult::Edited(v) => assert_eq!(v, complex), - other => panic!("Expected Edited, got {other:?}"), + EditResult::Edited(v) => assert_eq!(v, modified), + other => panic!("expected Edited, got {other:?}"), } } -#[test] -fn test_edit_result_returns_modified_content() { - let mock = MockEditorBackend::always("edited result"); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("original result").unwrap(); +// --- Result editing (`edit_result`) -------------------------------------- +#[test] +fn edit_result_returns_modified_content() { + let prompt = MockPromptBackend::new() + .with_reply_outcomes([ReplyOutcome::Submit("edited result".into())]); + let result = prompter(prompt).edit_result("original").unwrap(); assert_eq!(result, Some("edited result".to_string())); } #[test] -fn test_edit_result_empty_returns_none() { - let mock = MockEditorBackend::empty(); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("original result").unwrap(); - +fn edit_result_empty_returns_none() { + let prompt = + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit(String::new())]); + let result = prompter(prompt).edit_result("original").unwrap(); assert_eq!(result, None); } #[test] -fn test_edit_result_whitespace_only_returns_none() { - let mock = MockEditorBackend::always(" \n\t "); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("original result").unwrap(); - +fn edit_result_cancel_returns_none() { + let prompt = MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Cancelled]); + let result = prompter(prompt).edit_result("original").unwrap(); assert_eq!(result, None); } #[test] -fn test_edit_result_no_editor_returns_error() { - let prompter = prompter_without_editor(); - - let result = prompter.edit_result("some result"); - - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("No editor configured") - ); -} - -#[test] -fn test_edit_result_preserves_multiline_content() { +fn edit_result_preserves_multiline_content() { let multiline = "line 1\nline 2\nline 3"; - let mock = MockEditorBackend::always(multiline); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("original").unwrap(); - + let prompt = + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit(multiline.into())]); + let result = prompter(prompt).edit_result("original").unwrap(); assert_eq!(result, Some(multiline.to_string())); } -#[test] -fn test_has_editor_with_editor() { - let mock = MockEditorBackend::empty(); - let prompter = prompter_with_mock_editor(mock); +// --- Skip reasoning (`edit_text`) ---------------------------------------- - assert!(prompter.has_editor()); +#[test] +fn skip_reasoning_returns_text() { + let prompt = MockPromptBackend::new() + .with_reply_outcomes([ReplyOutcome::Submit("not needed here".into())]); + let result = prompter(prompt).edit_text("placeholder").unwrap(); + assert_eq!(result, Some("not needed here".to_string())); } #[test] -fn test_has_editor_without_editor() { - let prompter = prompter_without_editor(); +fn skip_reasoning_empty_returns_none() { + let prompt = + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit(String::new())]); + let result = prompter(prompt).edit_text("placeholder").unwrap(); + assert_eq!(result, None); +} - assert!(!prompter.has_editor()); +#[test] +fn skip_reasoning_unchanged_placeholder_returns_none() { + let placeholder = "_Provide reasoning_"; + let prompt = + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit(placeholder.into())]); + let result = prompter(prompt).edit_text(placeholder).unwrap(); + assert_eq!(result, None); } -/// Creates a `PermissionInfo` for testing. -fn make_permission_info(run_mode: RunMode, arguments: Value) -> PermissionInfo { - PermissionInfo { - tool_id: "call_123".to_string(), - tool_name: "test_tool".to_string(), - tool_source: ToolSource::Builtin { tool: None }, - run_mode, - arguments, - } +#[test] +fn skip_reasoning_cancel_returns_none() { + let prompt = MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Cancelled]); + let result = prompter(prompt).edit_text("placeholder").unwrap(); + assert_eq!(result, None); } -#[tokio::test] -async fn test_prompt_permission_unattended_returns_original_args() { - let prompter = prompter_without_editor(); - let original = serde_json::json!({"key": "original"}); +// --- Permission prompts (`prompt_permission`) ---------------------------- +#[test] +fn permission_unattended_returns_original_args() { + let original = json!({"key": "original"}); let info = make_permission_info(RunMode::Unattended, original.clone()); - let result = prompter.prompt_permission(&info).unwrap(); - + let result = prompter(MockPromptBackend::new()) + .prompt_permission(&info) + .unwrap(); match result { PermissionResult::Run { arguments, .. } => assert_eq!(arguments, original), - other @ PermissionResult::Skip { .. } => panic!("Expected Run, got {other:?}"), + other @ PermissionResult::Skip { .. } => panic!("expected Run, got {other:?}"), } } -#[tokio::test] -async fn test_prompt_permission_skip_returns_skip() { - let prompter = prompter_without_editor(); - - let info = make_permission_info(RunMode::Skip, serde_json::json!({})); - let result = prompter.prompt_permission(&info).unwrap(); - +#[test] +fn permission_skip_returns_skip() { + let info = make_permission_info(RunMode::Skip, json!({})); + let result = prompter(MockPromptBackend::new()) + .prompt_permission(&info) + .unwrap(); assert!(matches!(result, PermissionResult::Skip { reason: None, .. })); } -#[tokio::test] -async fn test_prompt_permission_edit_returns_modified_args() { - let original = serde_json::json!({"key": "original"}); - let modified = serde_json::json!({"key": "modified", "extra": true}); - - let mock = MockEditorBackend::json(&modified); - let prompter = prompter_with_mock_editor(mock); - - let info = make_permission_info(RunMode::Edit, original); - let result = prompter.prompt_permission(&info).unwrap(); +#[test] +fn permission_edit_returns_modified_args() { + let modified = json!({"key": "modified", "extra": true}); + let prompt = MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit( + serde_json::to_string(&modified).unwrap(), + )]); + let info = make_permission_info(RunMode::Edit, json!({"key": "original"})); + let result = prompter(prompt).prompt_permission(&info).unwrap(); match result { PermissionResult::Run { arguments, .. } => assert_eq!(arguments, modified), - other @ PermissionResult::Skip { .. } => { - panic!("Expected Run with modified args, got {other:?}") - } - } -} - -#[tokio::test] -async fn test_prompt_permission_edit_can_add_new_fields() { - let original = serde_json::json!({"existing": "value"}); - let modified = serde_json::json!({ - "existing": "value", - "new_field": "added", - "another": 42 - }); - - let mock = MockEditorBackend::json(&modified); - let prompter = prompter_with_mock_editor(mock); - - let info = make_permission_info(RunMode::Edit, original); - let result = prompter.prompt_permission(&info).unwrap(); - - match result { - PermissionResult::Run { arguments, .. } => { - assert_eq!(arguments["existing"], "value"); - assert_eq!(arguments["new_field"], "added"); - assert_eq!(arguments["another"], 42); - } - other @ PermissionResult::Skip { .. } => panic!("Expected Run, got {other:?}"), - } -} - -#[tokio::test] -async fn test_prompt_permission_edit_can_remove_fields() { - let original = serde_json::json!({"keep": "this", "remove": "that"}); - let modified = serde_json::json!({"keep": "this"}); - - let mock = MockEditorBackend::json(&modified); - let prompter = prompter_with_mock_editor(mock); - - let info = make_permission_info(RunMode::Edit, original); - let result = prompter.prompt_permission(&info).unwrap(); - - match result { - PermissionResult::Run { arguments, .. } => { - assert_eq!(arguments, modified); - assert!(arguments.get("remove").is_none()); - } - other @ PermissionResult::Skip { .. } => panic!("Expected Run, got {other:?}"), + other @ PermissionResult::Skip { .. } => panic!("expected Run, got {other:?}"), } } -#[tokio::test] -async fn test_prompt_permission_edit_can_change_types() { - let original = serde_json::json!({"value": "string"}); - let modified = serde_json::json!({"value": 123}); - - let mock = MockEditorBackend::json(&modified); - let prompter = prompter_with_mock_editor(mock); - - let info = make_permission_info(RunMode::Edit, original); - let result = prompter.prompt_permission(&info).unwrap(); - - match result { - PermissionResult::Run { arguments, .. } => { - assert_eq!(arguments["value"], 123); - } - other @ PermissionResult::Skip { .. } => panic!("Expected Run, got {other:?}"), - } -} - -#[test] -fn test_edit_result_modifies_tool_output() { - let original_result = "Original tool output:\n- item 1\n- item 2"; - let edited_result = "Edited output:\n- modified item"; - - let mock = MockEditorBackend::always(edited_result); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result(original_result).unwrap(); - - assert_eq!(result, Some(edited_result.to_string())); -} - -#[test] -fn test_edit_result_can_completely_replace_content() { - let original = "This is the original content from the tool"; - let replacement = "Completely different content"; - - let mock = MockEditorBackend::always(replacement); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result(original).unwrap(); - - assert_eq!(result, Some(replacement.to_string())); -} - #[test] -fn test_edit_result_empty_signals_fallback() { - // When user empties the content, edit_result returns None - // This signals the caller to fall back to Ask mode - let mock = MockEditorBackend::empty(); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("some content").unwrap(); - - assert!( - result.is_none(), - "Empty content should return None to signal fallback" - ); -} - -#[test] -fn test_edit_result_preserves_special_characters() { - let content_with_special = "Result with special chars: <>&\"'\nAnd unicode: 你好 🎉"; - - let mock = MockEditorBackend::always(content_with_special); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("original").unwrap(); - - assert_eq!(result, Some(content_with_special.to_string())); -} - -#[test] -fn test_edit_result_preserves_json_in_result() { - // Tool results often contain JSON - make sure it's preserved - let json_result = r#"{"status": "success", "data": [1, 2, 3]}"#; - - let mock = MockEditorBackend::always(json_result); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_result("{}").unwrap(); - - assert_eq!(result, Some(json_result.to_string())); - // Verify it's valid JSON - let parsed: serde_json::Value = serde_json::from_str(result.as_ref().unwrap()).unwrap(); - assert_eq!(parsed["status"], "success"); -} - -#[tokio::test] -async fn test_run_mode_edit_empty_falls_back_to_ask_then_approves() { - // Flow: Edit → empty → Ask → 'y' → Run with original args - let original = serde_json::json!({"key": "original"}); - - let editor = MockEditorBackend::empty(); +fn ask_then_run() { + let original = json!({"key": "original"}); let prompt = MockPromptBackend::new().with_inline_responses(['y']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Edit, original.clone()); - let result = prompter.prompt_permission(&info).unwrap(); - - match result { - PermissionResult::Run { arguments, .. } => assert_eq!(arguments, original), - PermissionResult::Skip { .. } => panic!("Expected Run, got Skip"), - } -} - -#[tokio::test] -async fn test_run_mode_edit_empty_falls_back_to_ask_then_skips() { - // Flow: Edit → empty → Ask → 'n' → Skip - let original = serde_json::json!({"key": "original"}); - - let editor = MockEditorBackend::empty(); - let prompt = MockPromptBackend::new().with_inline_responses(['n']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Edit, original); - let result = prompter.prompt_permission(&info).unwrap(); - - assert!(matches!(result, PermissionResult::Skip { - reason: None, - .. - })); -} - -#[tokio::test] -async fn test_run_mode_ask_approves() { - // Flow: Ask → 'y' → Run with original args - let original = serde_json::json!({"key": "original"}); - - let prompt = MockPromptBackend::new().with_inline_responses(['y']); - let prompter = prompter_with_mock_prompt(prompt); - let info = make_permission_info(RunMode::Ask, original.clone()); - let result = prompter.prompt_permission(&info).unwrap(); + let result = prompter(prompt).prompt_permission(&info).unwrap(); match result { PermissionResult::Run { arguments, .. } => assert_eq!(arguments, original), - PermissionResult::Skip { .. } => panic!("Expected Run, got Skip"), + other @ PermissionResult::Skip { .. } => panic!("expected Run, got {other:?}"), } } -#[tokio::test] -async fn test_run_mode_ask_skips() { - // Flow: Ask → 'n' → Skip +#[test] +fn ask_then_skip() { let prompt = MockPromptBackend::new().with_inline_responses(['n']); - let prompter = prompter_with_mock_prompt(prompt); - - let info = make_permission_info(RunMode::Ask, serde_json::json!({})); - let result = prompter.prompt_permission(&info).unwrap(); - + let info = make_permission_info(RunMode::Ask, json!({})); + let result = prompter(prompt).prompt_permission(&info).unwrap(); assert!(matches!(result, PermissionResult::Skip { reason: None, .. })); } -#[tokio::test] -async fn test_run_mode_ask_edit_option_modifies_args() { - // Flow: Ask → 'e' → Edit (valid JSON) → Run with modified args - let original = serde_json::json!({"key": "original"}); - let modified = serde_json::json!({"key": "modified"}); - - let editor = MockEditorBackend::json(&modified); - let prompt = MockPromptBackend::new().with_inline_responses(['e']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Ask, original); - let result = prompter.prompt_permission(&info).unwrap(); +#[test] +fn ask_edit_modifies_args() { + // Ask -> 'e' -> inline edit -> Run with modified args. + let modified = json!({"key": "modified"}); + let prompt = MockPromptBackend::new() + .with_inline_responses(['e']) + .with_reply_outcomes([ReplyOutcome::Submit( + serde_json::to_string(&modified).unwrap(), + )]); + let info = make_permission_info(RunMode::Ask, json!({"key": "original"})); + let result = prompter(prompt).prompt_permission(&info).unwrap(); match result { PermissionResult::Run { arguments, .. } => assert_eq!(arguments, modified), - PermissionResult::Skip { .. } => panic!("Expected Run with modified args, got Skip"), + other @ PermissionResult::Skip { .. } => panic!("expected Run, got {other:?}"), } } -#[tokio::test] -async fn test_run_mode_ask_edit_empty_loops_back_then_approves() { - // Flow: Ask → 'e' → empty → Ask → 'y' → Run with original - let original = serde_json::json!({"key": "original"}); - - let editor = MockEditorBackend::empty(); - // First 'e' to edit, then 'y' to approve after empty - let prompt = MockPromptBackend::new().with_inline_responses(['e', 'y']); - let prompter = prompter_with_mocks(editor, prompt); - +#[test] +fn ask_edit_empty_loops_back_then_approves() { + // Ask -> 'e' -> empty (back to Ask) -> 'y' -> Run with original args. + let original = json!({"key": "original"}); + let prompt = MockPromptBackend::new() + .with_inline_responses(['e', 'y']) + .with_reply_outcomes([ReplyOutcome::Submit(String::new())]); let info = make_permission_info(RunMode::Ask, original.clone()); - let result = prompter.prompt_permission(&info).unwrap(); + let result = prompter(prompt).prompt_permission(&info).unwrap(); match result { PermissionResult::Run { arguments, .. } => assert_eq!(arguments, original), - PermissionResult::Skip { .. } => panic!("Expected Run, got Skip"), + other @ PermissionResult::Skip { .. } => panic!("expected Run, got {other:?}"), } } -#[tokio::test] -async fn test_run_mode_ask_edit_invalid_json_retry_then_valid() { - // Flow: Ask → 'e' → invalid JSON → 'y' (retry) → valid JSON → Run - let modified = serde_json::json!({"key": "modified"}); - - // First edit returns invalid JSON, second returns valid - let editor = MockEditorBackend::with_responses([ - "{ invalid json }".to_string(), - serde_json::to_string_pretty(&modified).unwrap(), - ]); - // 'e' to edit, 'y' to retry after invalid JSON - let prompt = MockPromptBackend::new().with_inline_responses(['e', 'y']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Ask, serde_json::json!({})); - let result = prompter.prompt_permission(&info).unwrap(); +#[test] +fn ask_skip_with_reasoning() { + // Ask -> 'r' -> reason -> Skip with that reason. + let prompt = MockPromptBackend::new() + .with_inline_responses(['r']) + .with_reply_outcomes([ReplyOutcome::Submit("not applicable".into())]); + let info = make_permission_info(RunMode::Ask, json!({})); + let result = prompter(prompt).prompt_permission(&info).unwrap(); match result { - PermissionResult::Run { arguments, .. } => assert_eq!(arguments, modified), - PermissionResult::Skip { .. } => panic!("Expected Run, got Skip"), + PermissionResult::Skip { reason, .. } => { + assert_eq!(reason, Some("not applicable".to_string())); + } + other @ PermissionResult::Run { .. } => panic!("expected Skip, got {other:?}"), } } -#[tokio::test] -async fn test_run_mode_ask_edit_invalid_json_cancel() { - // Flow: Ask → 'e' → invalid JSON → 'n' (cancel) → Skip - let editor = MockEditorBackend::invalid_json(); - // 'e' to edit, 'n' to cancel after invalid JSON - let prompt = MockPromptBackend::new().with_inline_responses(['e', 'n']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Ask, serde_json::json!({})); - let result = prompter.prompt_permission(&info).unwrap(); +#[test] +fn ask_skip_with_empty_reasoning() { + // Ask -> 'r' -> empty -> Skip with no reason. + let prompt = MockPromptBackend::new() + .with_inline_responses(['r']) + .with_reply_outcomes([ReplyOutcome::Submit(String::new())]); + let info = make_permission_info(RunMode::Ask, json!({})); + let result = prompter(prompt).prompt_permission(&info).unwrap(); match result { - PermissionResult::Skip { reason, .. } => { - assert_eq!(reason, Some("Edit cancelled".to_string())); - } - PermissionResult::Run { .. } => panic!("Expected Skip, got Run"), + PermissionResult::Skip { reason, .. } => assert_eq!(reason, None), + other @ PermissionResult::Run { .. } => panic!("expected Skip, got {other:?}"), } } +// --- Result delivery confirmation (`prompt_result_confirmation`) ---------- + #[test] -fn test_result_confirmation_approves() { +fn result_confirmation_approves() { let prompt = MockPromptBackend::new().with_inline_responses(['y']); - let prompter = prompter_with_mock_prompt(prompt); - - let result = prompter.prompt_result_confirmation("test_tool").unwrap(); - - assert!(result, "Expected confirmation to return true"); + assert!( + prompter(prompt) + .prompt_result_confirmation("test_tool") + .unwrap() + ); } #[test] -fn test_result_confirmation_skips() { +fn result_confirmation_skips() { let prompt = MockPromptBackend::new().with_inline_responses(['n']); - let prompter = prompter_with_mock_prompt(prompt); - - let result = prompter.prompt_result_confirmation("test_tool").unwrap(); - - assert!(!result, "Expected skip to return false"); + assert!( + !prompter(prompt) + .prompt_result_confirmation("test_tool") + .unwrap() + ); } #[test] -fn test_result_confirmation_edit_requested() { - // When user presses 'e', prompt_result_confirmation returns an error - // signaling that the caller should handle editing - let editor = MockEditorBackend::empty(); // Need editor for 'e' option +fn result_confirmation_edit_requested() { + // 'e' is always offered now (un-gated); it signals edit via an error. let prompt = MockPromptBackend::new().with_inline_responses(['e']); - let prompter = prompter_with_mocks(editor, prompt); - - let result = prompter.prompt_result_confirmation("test_tool"); - + let result = prompter(prompt).prompt_result_confirmation("test_tool"); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("edit_requested")); } #[test] -fn test_result_confirmation_cancelled_returns_false() { - // When prompt is cancelled (no response), returns false - let prompt = MockPromptBackend::new(); // No responses = cancelled - let prompter = prompter_with_mock_prompt(prompt); - - let result = prompter.prompt_result_confirmation("test_tool").unwrap(); - - assert!(!result, "Expected cancelled to return false"); +fn result_confirmation_cancelled_returns_false() { + let prompt = MockPromptBackend::new(); + assert!( + !prompter(prompt) + .prompt_result_confirmation("test_tool") + .unwrap() + ); } +// --- Tool questions (`prompt_question`) ---------------------------------- + #[test] -fn test_prompt_question_boolean_uses_inline_select() { +fn question_boolean_uses_inline_select() { let prompt = MockPromptBackend::new().with_inline_responses(['y']); - let prompter = prompter_with_mock_prompt(prompt); - let question = jp_tool::Question::boolean("q1", "Proceed?"); - - let result = prompter.prompt_question(&question).unwrap(); + let result = prompter(prompt).prompt_question(&question).unwrap(); assert_eq!(result.answer, Value::Bool(true)); assert_eq!(result.persist_level, jp_tool::PersistLevel::None); } #[test] -fn test_prompt_question_boolean_default() { - // With default=true (y), entering nothing (simulated by Mock defaulting logic?) - // MockPromptBackend::inline_select returns the first response. - // If we want to simulate "default used", we can't easily with current MockPromptBackend - // unless we interpret a specific char as "default used"? - // But `inline_select` implementation in MockPromptBackend returns what's in queue. - // So we just verify `default` is passed to backend. - // However, MockPromptBackend stores responses, it doesn't inspect args passed to it. - // To verify default is passed, we might need a spy or just rely on manual verification/Terminal backend correctness. - // Since MockPromptBackend doesn't capture calls, we can only verify the return value logic. - - let prompt = MockPromptBackend::new().with_inline_responses(['N']); - let prompter = prompter_with_mock_prompt(prompt); - - let question = jp_tool::Question::boolean("q1", "Proceed?").with_default(Value::Bool(false)); - - let result = prompter.prompt_question(&question).unwrap(); - assert_eq!(result.answer, Value::Bool(false)); - assert_eq!(result.persist_level, jp_tool::PersistLevel::Turn); -} - -#[test] -fn test_prompt_question_text_uses_backend() { +fn question_text_uses_backend() { let prompt = MockPromptBackend::new().with_text_responses(["user input"]); - let prompter = prompter_with_mock_prompt(prompt); - let question = jp_tool::Question::text("q2", "Input:"); - - let result = prompter.prompt_question(&question).unwrap(); + let result = prompter(prompt).prompt_question(&question).unwrap(); assert_eq!(result.answer, Value::String("user input".to_string())); } #[test] -fn test_prompt_question_select_uses_backend() { +fn question_select_uses_backend() { let prompt = MockPromptBackend::new().with_select_responses(["Option B"]); - let prompter = prompter_with_mock_prompt(prompt); - let question = jp_tool::Question::select("q3", "Choose:") .with_options(vec!["Option A".to_string(), "Option B".to_string()]); - - let result = prompter.prompt_question(&question).unwrap(); + let result = prompter(prompt).prompt_question(&question).unwrap(); assert_eq!(result.answer, Value::String("Option B".to_string())); } - -#[test] -fn test_skip_reasoning_returns_editor_content() { - let mock = MockEditorBackend::always("This tool is not needed for this task."); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_text("").unwrap().unwrap(); - assert_eq!(result, "This tool is not needed for this task."); -} - -#[test] -fn test_skip_reasoning_empty_returns_default() { - let mock = MockEditorBackend::empty(); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_text("").unwrap(); - assert_eq!(result, None); -} - -#[test] -fn test_skip_reasoning_placeholder_unchanged_returns_default() { - let placeholder = "_Provide reasoning for skipping tool execution_"; - let mock = MockEditorBackend::always(placeholder); - let prompter = prompter_with_mock_editor(mock); - - let result = prompter.edit_text(placeholder).unwrap(); - assert_eq!(result, None); -} - -#[test] -fn test_skip_reasoning_no_editor_returns_error() { - let prompter = prompter_without_editor(); - - let result = prompter.edit_text(""); - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("No editor configured") - ); -} - -#[tokio::test] -async fn test_run_mode_ask_skip_with_reasoning() { - // Flow: Ask → 'r' (skip and reply) → editor → Skip with reason - let editor = MockEditorBackend::always("Not applicable for this commit."); - let prompt = MockPromptBackend::new().with_inline_responses(['r']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Ask, serde_json::json!({})); - let result = prompter.prompt_permission(&info).unwrap(); - - match result { - PermissionResult::Skip { reason, .. } => { - assert_eq!(reason, Some("Not applicable for this commit.".to_string())); - } - PermissionResult::Run { .. } => panic!("Expected Skip, got Run"), - } -} - -#[tokio::test] -async fn test_run_mode_ask_skip_with_reasoning_empty_editor() { - // Flow: Ask → 'r' (skip and reply) → empty editor → Skip with no reason - // edit_text returns None for empty content, which becomes the skip reason. - let editor = MockEditorBackend::empty(); - let prompt = MockPromptBackend::new().with_inline_responses(['r']); - let prompter = prompter_with_mocks(editor, prompt); - - let info = make_permission_info(RunMode::Ask, serde_json::json!({})); - let result = prompter.prompt_permission(&info).unwrap(); - - match result { - PermissionResult::Skip { reason, .. } => { - assert_eq!(reason, None); - } - PermissionResult::Run { .. } => panic!("Expected Skip, got Run"), - } -} diff --git a/crates/jp_cli/src/cmd/query/turn_loop.rs b/crates/jp_cli/src/cmd/query/turn_loop.rs index 8787b4fa..c3a4595f 100644 --- a/crates/jp_cli/src/cmd/query/turn_loop.rs +++ b/crates/jp_cli/src/cmd/query/turn_loop.rs @@ -45,7 +45,7 @@ use tracing::{debug, info, warn}; use super::{ build_sections, build_thread, - interrupt::{LoopAction, handle_llm_event, handle_streaming_signal}, + interrupt::{LoopAction, handle_llm_event, handle_streaming_signal, reply_edit_mode}, stream::{StreamRetryState, handle_stream_error}, tool::{ PendingEntry, PendingTools, ToolCallDecision, ToolCallState, ToolCoordinator, ToolPrompter, @@ -57,6 +57,7 @@ use super::{ }; use crate::{ cmd::query::tool::coordinator::ExecutionResult, + editor::build_editor_backend, error::Error, render::metadata::set_rendered_arguments, signals::{SignalRx, SignalTo}, @@ -216,8 +217,9 @@ pub(super) async fn run_turn_loop( // executing (tool question prompts) phases. let prompter = Arc::new(ToolPrompter::with_prompt_backend( printer.clone(), - cfg.editor.path(), + build_editor_backend(&cfg.editor), prompt_backend.clone(), + reply_edit_mode(cfg.editor.inline.edit_mode), )); loop { @@ -360,6 +362,8 @@ pub(super) async fn run_turn_loop( stream, &printer, prompt_backend.as_ref(), + build_editor_backend(&cfg.editor), + reply_edit_mode(cfg.editor.inline.edit_mode), &cfg.interrupt.streaming, !llm_alive, ) @@ -557,8 +561,9 @@ pub(super) async fn run_turn_loop( let unavailable = tool_coordinator.prepare(restart_calls); let restart_prompter = ToolPrompter::with_prompt_backend( printer.clone(), - cfg.editor.path(), + build_editor_backend(&cfg.editor), prompt_backend.clone(), + reply_edit_mode(cfg.editor.inline.edit_mode), ); let (executors, skipped) = tool_coordinator .run_permission_phase( @@ -635,6 +640,8 @@ pub(super) async fn run_turn_loop( &mut turn_state, &printer, prompt_backend.as_ref(), + build_editor_backend(&cfg.editor), + reply_edit_mode(cfg.editor.inline.edit_mode), Arc::clone(&inquiry_backend), &conv, mcp_client, diff --git a/crates/jp_cli/src/editor.rs b/crates/jp_cli/src/editor.rs index b4f5b44b..13584adb 100644 --- a/crates/jp_cli/src/editor.rs +++ b/crates/jp_cli/src/editor.rs @@ -3,59 +3,43 @@ mod parser; use std::{ fs::{self, OpenOptions}, io::{Read as _, Write as _}, - str::FromStr, + sync::Arc, }; use camino::{Utf8Path, Utf8PathBuf}; use chrono::{FixedOffset, Local}; -use duct::Expression; use jp_config::{ - AppConfig, PartialAppConfig, ToPartial as _, model::parameters::PartialReasoningConfig, + AppConfig, PartialAppConfig, ToPartial as _, editor::EditorConfig, + model::parameters::PartialReasoningConfig, }; use jp_conversation::{ ConversationStream, event::{ChatResponse, EventKind}, }; +use jp_editor::{EditOutcome, EditRequest, EditorBackend, TerminalEditorBackend}; use crate::{ editor::parser::QueryDocument, error::{Error, Result}, }; -/// The name of the file used to store the current query message. -pub(crate) const QUERY_FILENAME: &str = "QUERY_MESSAGE.md"; - -/// How to edit the query. -#[derive(Debug, Clone, PartialEq, Default)] -pub(crate) enum Editor { - /// Use whatever editor is configured. - #[default] - Default, - - /// Use the given command. - Command(String), - - /// Do not edit the query. - Disabled, +/// Build a terminal editor backend from the resolved editor configuration. +/// +/// Returns `None` when no editor command resolves: neither `editor.cmd` is set +/// nor does a configured editor environment variable point at an installed +/// binary. +pub(crate) fn build_editor_backend(config: &EditorConfig) -> Option> { + config + .command() + .map(|cmd| Arc::new(TerminalEditorBackend::new(cmd)) as Arc) } -impl FromStr for Editor { - type Err = Error; - - fn from_str(s: &str) -> Result { - match s { - "true" => Ok(Self::Default), - "false" => Ok(Self::Disabled), - s => Ok(Self::Command(s.to_owned())), - } - } -} +/// The name of the file used to store the current query message. +pub(crate) const QUERY_FILENAME: &str = "QUERY_MESSAGE.md"; /// Options for opening an editor. #[derive(Debug)] pub(crate) struct Options { - cmd: Expression, - /// The working directory to use. cwd: Option, @@ -67,9 +51,8 @@ pub(crate) struct Options { } impl Options { - pub(crate) fn new(cmd: Expression) -> Self { + pub(crate) fn new() -> Self { Self { - cmd, cwd: None, content: None, force_write: false, @@ -148,10 +131,16 @@ impl Drop for RevertFileGuard { /// If the file exists, it will be opened, but the content will not be modified /// (in other words, `content` is ignored). /// -/// When the editor is closed, the contents are returned. -pub(crate) fn open(path: Utf8PathBuf, options: Options) -> Result<(String, RevertFileGuard)> { +/// When the editor is closed, the interaction outcome and the file's contents +/// are returned. +/// On [`EditOutcome::Cancelled`] the contents reflect whatever the editor left +/// on disk; the caller decides how to treat a cancellation. +pub(crate) fn open( + path: Utf8PathBuf, + options: Options, + editor: &dyn EditorBackend, +) -> Result<(EditOutcome, String, RevertFileGuard)> { let Options { - cmd, cwd, content, force_write, @@ -185,32 +174,17 @@ pub(crate) fn open(path: Utf8PathBuf, options: Options) -> Result<(String, Rever file.write_all(current_content.as_bytes())?; } - // Open the editor - let output = cmd - .before_spawn({ - let path = path.clone(); - move |cmd| { - cmd.arg(path.clone()); - - if let Some(cwd) = &cwd { - cmd.current_dir(cwd); - } - - Ok(()) - } + let outcome = editor + .edit_file(EditRequest { + paths: std::slice::from_ref(&path), + cwd: cwd.as_deref(), }) - .unchecked() - .run()?; - - let status = output.status; - if !status.success() { - return Err(Error::Editor(format!("Editor exited with error: {status}"))); - } + .map_err(|error| Error::Editor(error.to_string()))?; // Read the edited content let content = fs::read_to_string(path)?; - Ok((content, guard)) + Ok((outcome, content, guard)) } /// Open an editor for the user to input or edit text using a file in the @@ -220,7 +194,7 @@ pub(crate) fn edit_query( conversation_root: &Utf8Path, stream: &ConversationStream, query: &str, - cmd: Expression, + editor: &dyn EditorBackend, config_error: Option<&str>, ) -> Result<(String, PartialAppConfig)> { let query_file_path = conversation_root.join(QUERY_FILENAME); @@ -243,12 +217,18 @@ pub(crate) fn edit_query( let history_value = build_history_text(stream); doc.meta.history.value = &history_value; - let options = Options::new(cmd.clone()) + let options = Options::new() .with_cwd(conversation_root) .with_content(doc) .with_force_write(true); - let (content, mut guard) = open(query_file_path.clone(), options)?; + let (outcome, content, mut guard) = open(query_file_path.clone(), options, editor)?; + + // A cancelled editor (non-zero exit) sends nothing: return an empty query so + // the caller skips it, and let the guard revert `QUERY_MESSAGE.md`. + if outcome == EditOutcome::Cancelled { + return Ok((String::new(), PartialAppConfig::empty())); + } let doc = QueryDocument::try_from(content.as_str()).unwrap_or_default(); let mut partial = PartialAppConfig::empty(); @@ -257,7 +237,7 @@ pub(crate) fn edit_query( Ok(v) => partial = v, Err(error) => { let error = error.to_string(); - return edit_query(config, conversation_root, stream, "", cmd, Some(&error)); + return edit_query(config, conversation_root, stream, "", editor, Some(&error)); } } } diff --git a/crates/jp_config/src/editor.rs b/crates/jp_config/src/editor.rs index 4f1bda7e..34f2a93d 100644 --- a/crates/jp_config/src/editor.rs +++ b/crates/jp_config/src/editor.rs @@ -2,9 +2,9 @@ use std::env; -use camino::Utf8PathBuf; use duct::Expression; -use schematic::Config; +use schematic::{Config, ConfigEnum}; +use serde::{Deserialize, Serialize}; use crate::{ assignment::{AssignKeyValue, AssignResult, KvAssignment, missing_key}, @@ -45,6 +45,43 @@ pub struct EditorConfig { merge = schematic::merge::append_vec, )] pub envs: Vec, + + /// Settings for the inline reply widget. + /// + /// Lives at `editor.inline.*`. + #[setting(nested)] + pub inline: InlineEditorConfig, +} + +/// Inline reply widget configuration. +/// +/// Settings for the inline editor JP shows for short replies (for example after +/// `Ctrl+C` during a query). +/// Independent of which external editor `cmd`/`envs` opens. +#[derive(Debug, Clone, PartialEq, Default, Config)] +#[config(rename_all = "snake_case")] +pub struct InlineEditorConfig { + /// Editing style of the inline reply buffer. + /// + /// - `emacs`: Emacs-style keybindings (default). + /// - `vi`: Vi-style modal editing (insert/normal modes). + /// + /// Controls only the inline buffer's editing style; it is independent of + /// which external editor opens when you escape to `$EDITOR`. + #[setting(default)] + pub edit_mode: InlineEditMode, +} + +/// Editing style for the inline reply widget. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize, ConfigEnum)] +#[serde(rename_all = "snake_case")] +pub enum InlineEditMode { + /// Emacs-style keybindings. + #[default] + Emacs, + + /// Vi-style modal editing (insert/normal modes). + Vi, } impl AssignKeyValue for PartialEditorConfig { @@ -53,6 +90,7 @@ impl AssignKeyValue for PartialEditorConfig { "" => kv.try_merge_object(self)?, "cmd" => self.cmd = kv.try_some_string()?, _ if kv.p("envs") => kv.try_some_vec_of_strings(&mut self.envs)?, + _ if kv.p("inline") => self.inline.assign(kv)?, _ => return missing_key(&kv), } @@ -65,6 +103,7 @@ impl PartialConfigDelta for PartialEditorConfig { Self { cmd: delta_opt(self.cmd.as_ref(), next.cmd), envs: delta_opt_vec(self.envs.as_ref(), next.envs), + inline: self.inline.delta(next.inline), } } } @@ -74,6 +113,7 @@ impl FillDefaults for PartialEditorConfig { Self { cmd: self.cmd.or(defaults.cmd), envs: self.envs.or(defaults.envs), + inline: self.inline.fill_from(defaults.inline), } } } @@ -85,6 +125,45 @@ impl ToPartial for EditorConfig { Self::Partial { cmd: partial_opts(self.cmd.as_ref(), defaults.cmd), envs: partial_opt(&self.envs, defaults.envs), + inline: self.inline.to_partial(), + } + } +} + +impl AssignKeyValue for PartialInlineEditorConfig { + fn assign(&mut self, kv: KvAssignment) -> AssignResult { + match kv.key_string().as_str() { + "" => kv.try_merge_object(self)?, + "edit_mode" => self.edit_mode = kv.try_some_from_str()?, + _ => return missing_key(&kv), + } + + Ok(()) + } +} + +impl PartialConfigDelta for PartialInlineEditorConfig { + fn delta(&self, next: Self) -> Self { + Self { + edit_mode: delta_opt(self.edit_mode.as_ref(), next.edit_mode), + } + } +} + +impl FillDefaults for PartialInlineEditorConfig { + fn fill_from(self, defaults: Self) -> Self { + Self { + edit_mode: self.edit_mode.or(defaults.edit_mode), + } + } +} + +impl ToPartial for InlineEditorConfig { + fn to_partial(&self) -> Self::Partial { + let defaults = Self::Partial::default(); + + Self::Partial { + edit_mode: partial_opt(&self.edit_mode, defaults.edit_mode), } } } @@ -116,22 +195,6 @@ impl EditorConfig { }) }) } - - /// Return the path to the editor, if any. - /// - /// For env-var fallback, the first shlex token is taken as the binary; any - /// additional arguments are dropped (use [`Self::command`] when the caller - /// can invoke the program with arguments). - #[must_use] - pub fn path(&self) -> Option { - self.cmd.as_ref().map(Utf8PathBuf::from).or_else(|| { - self.envs.iter().find_map(|v| { - let value = env::var(v).ok()?; - let program = shlex::split(&value)?.into_iter().next()?; - which::which(&program).ok().and_then(|p| p.try_into().ok()) - }) - }) - } } #[cfg(test)] diff --git a/crates/jp_config/src/editor_tests.rs b/crates/jp_config/src/editor_tests.rs index 24eccc40..096e9647 100644 --- a/crates/jp_config/src/editor_tests.rs +++ b/crates/jp_config/src/editor_tests.rs @@ -1,8 +1,7 @@ -use serial_test::serial; use test_log::test; use super::*; -use crate::{assignment::KvAssignment, util::EnvVarGuard}; +use crate::assignment::KvAssignment; #[test] fn test_editor_config_cmd() { @@ -52,43 +51,3 @@ fn test_editor_config_envs() { ]) ); } - -#[test(serial(env_vars))] -fn test_editor_config_path() { - let mut p = EditorConfig { - cmd: Some("vim".into()), - envs: vec![], - }; - - assert_eq!(p.path(), Some(Utf8PathBuf::from("vim"))); - - p.cmd = Some("subl -w".into()); - assert_eq!(p.path(), Some(Utf8PathBuf::from("subl -w"))); - - p.cmd = Some("/usr/bin/vim".into()); - assert_eq!(p.path(), Some(Utf8PathBuf::from("/usr/bin/vim"))); - - p.cmd = None; - p.envs = vec![]; - assert_eq!(p.path(), None); - - let _env = EnvVarGuard::set("JP_EDITOR1", "vi"); - p.envs = vec!["JP_EDITOR1".into()]; - assert!(p.path().unwrap().to_string().ends_with("/bin/vi")); - - let _env = EnvVarGuard::set("JP_EDITOR2", "doesnotexist"); - p.envs = vec!["JP_EDITOR2".into()]; - assert_eq!(p.path(), None); - - // Env-var values are shlex-split; the first token is the binary, args - // are preserved by `command()` (verified separately) and dropped by - // `path()`. The binary must still be on PATH. - let _env = EnvVarGuard::set("JP_EDITOR3", "vi --readonly"); - p.envs = vec!["JP_EDITOR3".into()]; - assert!(p.path().unwrap().to_string().ends_with("/bin/vi")); - - // Unbalanced quoting causes the env var to be skipped. - let _env = EnvVarGuard::set("JP_EDITOR4", "vi 'unterminated"); - p.envs = vec!["JP_EDITOR4".into()]; - assert_eq!(p.path(), None); -} diff --git a/crates/jp_config/src/interrupt.rs b/crates/jp_config/src/interrupt.rs index 903def73..ac45a6ea 100644 --- a/crates/jp_config/src/interrupt.rs +++ b/crates/jp_config/src/interrupt.rs @@ -51,6 +51,17 @@ pub struct StreamingInterruptConfig { /// so far is kept as context. #[setting(default)] pub action: StreamingInterruptAction, + + /// Open the editor directly for a reply, skipping the inline widget. + /// + /// Defaults to `false`. + /// When `true`, the `reply` path opens the configured editor immediately + /// instead of the inline reply prompt; a non-empty saved result is sent, an + /// empty or cancelled result returns to the menu. + /// Falls back to the inline widget when no editor is configured, and has no + /// effect in non-interactive (no-tty) mode. + #[setting(default = false)] + pub reply_in_editor: bool, } /// Ctrl-C behavior while tools are executing. @@ -66,6 +77,17 @@ pub struct ToolInterruptConfig { /// assistant in their place. #[setting(default)] pub action: ToolInterruptAction, + + /// Open the editor directly for a reply, skipping the inline widget. + /// + /// Defaults to `false`. + /// When `true`, the `stop_reply` path opens the configured editor + /// immediately instead of the inline reply prompt; a non-empty saved result + /// is sent, an empty result falls through to the canned rejection notice. + /// Falls back to the inline widget when no editor is configured, and has no + /// effect in non-interactive (no-tty) mode. + #[setting(default = false)] + pub reply_in_editor: bool, } /// What Ctrl-C does while the assistant is generating content. @@ -130,6 +152,7 @@ impl AssignKeyValue for PartialStreamingInterruptConfig { match kv.key_string().as_str() { "" => kv.try_merge_object(self)?, "action" => self.action = kv.try_some_from_str()?, + "reply_in_editor" => self.reply_in_editor = kv.try_some_bool()?, _ => return missing_key(&kv), } @@ -142,6 +165,7 @@ impl AssignKeyValue for PartialToolInterruptConfig { match kv.key_string().as_str() { "" => kv.try_merge_object(self)?, "action" => self.action = kv.try_some_from_str()?, + "reply_in_editor" => self.reply_in_editor = kv.try_some_bool()?, _ => return missing_key(&kv), } @@ -162,6 +186,7 @@ impl PartialConfigDelta for PartialStreamingInterruptConfig { fn delta(&self, next: Self) -> Self { Self { action: delta_opt(self.action.as_ref(), next.action), + reply_in_editor: delta_opt(self.reply_in_editor.as_ref(), next.reply_in_editor), } } } @@ -170,6 +195,7 @@ impl PartialConfigDelta for PartialToolInterruptConfig { fn delta(&self, next: Self) -> Self { Self { action: delta_opt(self.action.as_ref(), next.action), + reply_in_editor: delta_opt(self.reply_in_editor.as_ref(), next.reply_in_editor), } } } @@ -187,6 +213,7 @@ impl FillDefaults for PartialStreamingInterruptConfig { fn fill_from(self, defaults: Self) -> Self { Self { action: self.action.or(defaults.action), + reply_in_editor: self.reply_in_editor.or(defaults.reply_in_editor), } } } @@ -195,6 +222,7 @@ impl FillDefaults for PartialToolInterruptConfig { fn fill_from(self, defaults: Self) -> Self { Self { action: self.action.or(defaults.action), + reply_in_editor: self.reply_in_editor.or(defaults.reply_in_editor), } } } @@ -214,6 +242,7 @@ impl ToPartial for StreamingInterruptConfig { Self::Partial { action: partial_opt(&self.action, defaults.action), + reply_in_editor: partial_opt(&self.reply_in_editor, defaults.reply_in_editor), } } } @@ -224,6 +253,7 @@ impl ToPartial for ToolInterruptConfig { Self::Partial { action: partial_opt(&self.action, defaults.action), + reply_in_editor: partial_opt(&self.reply_in_editor, defaults.reply_in_editor), } } } diff --git a/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap b/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap index f1540ccd..99417c32 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap @@ -62,9 +62,12 @@ expression: "AppConfig::fields()" "plugins.command", "plugins.shutdown_timeout_secs", "interrupt.tool_call.action", + "interrupt.tool_call.reply_in_editor", "interrupt.streaming.action", + "interrupt.streaming.reply_in_editor", "editor.cmd", "editor.envs", + "editor.inline.edit_mode", "conversation.attachments", "conversation.default_id", "conversation.start_local", diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap index e9b8ac71..64664ac4 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap @@ -144,14 +144,19 @@ PartialAppConfig { interrupt: PartialInterruptConfig { streaming: PartialStreamingInterruptConfig { action: None, + reply_in_editor: None, }, tool_call: PartialToolInterruptConfig { action: None, + reply_in_editor: None, }, }, editor: PartialEditorConfig { cmd: None, envs: None, + inline: PartialInlineEditorConfig { + edit_mode: None, + }, }, template: PartialTemplateConfig { values: {}, diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap index 524ad710..0716b4df 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap @@ -284,9 +284,15 @@ Ok( interrupt: PartialInterruptConfig { streaming: PartialStreamingInterruptConfig { action: None, + reply_in_editor: Some( + false, + ), }, tool_call: PartialToolInterruptConfig { action: None, + reply_in_editor: Some( + false, + ), }, }, editor: PartialEditorConfig { @@ -298,6 +304,9 @@ Ok( "EDITOR", ], ), + inline: PartialInlineEditorConfig { + edit_mode: None, + }, }, template: PartialTemplateConfig { values: {}, diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap index 8885788f..eed9de1c 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap @@ -144,14 +144,19 @@ PartialAppConfig { interrupt: PartialInterruptConfig { streaming: PartialStreamingInterruptConfig { action: None, + reply_in_editor: None, }, tool_call: PartialToolInterruptConfig { action: None, + reply_in_editor: None, }, }, editor: PartialEditorConfig { cmd: None, envs: None, + inline: PartialInlineEditorConfig { + edit_mode: None, + }, }, template: PartialTemplateConfig { values: {}, diff --git a/crates/jp_editor/Cargo.toml b/crates/jp_editor/Cargo.toml index 7fd50d7b..714880c9 100644 --- a/crates/jp_editor/Cargo.toml +++ b/crates/jp_editor/Cargo.toml @@ -14,9 +14,11 @@ version.workspace = true [dependencies] camino = { workspace = true } -open-editor = { workspace = true } +camino-tempfile = { workspace = true } +duct = { workspace = true } serde = { workspace = true } serde_json = { workspace = true, features = ["std"] } +thiserror = { workspace = true } [lints] workspace = true diff --git a/crates/jp_editor/src/lib.rs b/crates/jp_editor/src/lib.rs index c183cbac..add5ef1d 100644 --- a/crates/jp_editor/src/lib.rs +++ b/crates/jp_editor/src/lib.rs @@ -1,34 +1,150 @@ -use std::sync::Mutex; +//! Editor invocation backends. +//! +//! [`EditorBackend`] is the frontend seam for running the user's configured +//! editor. +//! It offers two shapes of edit: [`edit_text`] for ephemeral +//! string-in/string-out editing, and [`edit_file`] for opening the editor on +//! caller-owned paths. +//! +//! [`TerminalEditorBackend`] spawns the editor as a local process from a +//! [`duct::Expression`], so it honours every flag the user attached to their +//! editor command. +//! [`MockEditorBackend`] scripts edited text for tests without spawning +//! anything. +//! +//! [`edit_file`]: EditorBackend::edit_file +//! [`edit_text`]: EditorBackend::edit_text -use camino::Utf8PathBuf; -use open_editor::{Editor, EditorCallBuilder, errors::OpenEditorError}; +use std::{fs, sync::Mutex}; + +use camino::{Utf8Path, Utf8PathBuf}; +use camino_tempfile::NamedUtf8TempFile; +use duct::Expression; use serde::Serialize; -/// Backend for opening an editor to modify text. +/// Backend for invoking the user's configured editor. /// -/// This trait abstracts the editor interaction, allowing tests to mock the -/// editor without actually opening an external process. +/// Each frontend (terminal, web, native, mock) provides one implementation. pub trait EditorBackend: Send + Sync { - /// Opens an editor with the given content and returns the modified content. - fn edit(&self, content: &str) -> Result; + /// Edit `content` in the editor and return the edited text. + /// + /// On [`EditOutcome::Cancelled`] the returned string is meaningless and + /// callers should ignore it. + fn edit_text(&self, content: &str) -> Result<(EditOutcome, String), EditorError>; + + /// Open the editor on the requested path(s) and block until it exits. + /// + /// The edited content is read back from disk by the caller. + fn edit_file(&self, req: EditRequest<'_>) -> Result; } -/// Terminal editor implementation using the `open-editor` crate. +/// Frontend-agnostic request data for [`EditorBackend::edit_file`]. +pub struct EditRequest<'a> { + /// The path(s) to open in the editor. + 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 of an editor session. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EditOutcome { + /// The user saved and closed (terminal editor exited zero). + Saved, + + /// The user aborted (terminal editor exited non-zero). + Cancelled, +} + +/// An error from invoking the editor. +#[derive(Debug, thiserror::Error)] +pub enum EditorError { + /// The editor process could not be spawned (e.g. the binary was not found). + #[error("failed to spawn editor")] + Spawn(#[source] std::io::Error), + + /// Reading or writing the file being edited failed. + #[error("editor file I/O failed")] + Io(#[source] std::io::Error), +} + +/// Terminal editor backend: spawns the editor as a local process. +/// +/// The path(s) being edited are appended as trailing arguments to the +/// configured command, preserving any flags the user attached (e.g. +/// `code --wait`). pub struct TerminalEditorBackend { - pub path: Utf8PathBuf, + cmd: Expression, +} + +impl TerminalEditorBackend { + /// Create a backend that runs `cmd`, appending the edited path(s) as + /// trailing arguments. + #[must_use] + pub fn new(cmd: Expression) -> Self { + Self { cmd } + } + + /// Spawn the editor on `paths`, mapping the exit status to an + /// [`EditOutcome`]. + fn run( + &self, + paths: &[Utf8PathBuf], + cwd: Option<&Utf8Path>, + ) -> Result { + let args: Vec = paths.iter().map(|p| p.as_str().to_owned()).collect(); + let cwd = cwd.map(ToOwned::to_owned); + + let output = self + .cmd + .clone() + .before_spawn(move |cmd| { + for arg in &args { + cmd.arg(arg); + } + if let Some(cwd) = &cwd { + cmd.current_dir(cwd); + } + Ok(()) + }) + .unchecked() + .run() + .map_err(EditorError::Spawn)?; + + Ok(if output.status.success() { + EditOutcome::Saved + } else { + EditOutcome::Cancelled + }) + } } impl EditorBackend for TerminalEditorBackend { - fn edit(&self, content: &str) -> Result { - EditorCallBuilder::new() - .with_editor(Editor::from_bin_path(self.path.as_std_path().into())) - .edit_string(content) + fn edit_text(&self, content: &str) -> Result<(EditOutcome, String), EditorError> { + let tmp = NamedUtf8TempFile::new().map_err(EditorError::Io)?; + let path = tmp.path().to_owned(); + fs::write(&path, content).map_err(EditorError::Io)?; + + let outcome = self.run(std::slice::from_ref(&path), None)?; + let edited = fs::read_to_string(&path).map_err(EditorError::Io)?; + + Ok((outcome, edited)) + } + + fn edit_file(&self, req: EditRequest<'_>) -> Result { + self.run(req.paths, req.cwd) } } /// Mock editor backend for testing. /// -/// Returns pre-configured responses without opening an actual editor. +/// Scripts the text returned by [`edit_text`] without spawning a process. +/// Every interaction reports [`EditOutcome::Saved`]. +/// +/// [`edit_text`]: EditorBackend::edit_text pub struct MockEditorBackend { responses: Mutex>, } @@ -36,7 +152,7 @@ pub struct MockEditorBackend { impl MockEditorBackend { /// Creates a mock that returns the given responses in sequence. /// - /// Each call to `edit()` consumes one response. + /// Each call to `edit_text` consumes one response. /// If all responses are exhausted, subsequent calls return an empty string. #[must_use] pub fn with_responses(responses: impl IntoIterator>) -> Self { @@ -74,14 +190,20 @@ impl MockEditorBackend { } impl EditorBackend for MockEditorBackend { - fn edit(&self, _content: &str) -> Result { + fn edit_text(&self, _content: &str) -> Result<(EditOutcome, String), EditorError> { let mut responses = self.responses.lock().unwrap(); - if responses.is_empty() { + let text = if responses.is_empty() { // If no more responses, return empty (simulates user clearing - // content) - Ok(String::new()) + // content). + String::new() } else { - Ok(responses.remove(0)) - } + responses.remove(0) + }; + + Ok((EditOutcome::Saved, text)) + } + + fn edit_file(&self, _req: EditRequest<'_>) -> Result { + Ok(EditOutcome::Saved) } } diff --git a/crates/jp_editor/tests/terminal.rs b/crates/jp_editor/tests/terminal.rs new file mode 100644 index 00000000..a853731d --- /dev/null +++ b/crates/jp_editor/tests/terminal.rs @@ -0,0 +1,57 @@ +//! Integration tests for [`TerminalEditorBackend`]. +//! +//! A fake "editor" is spawned via `sh -c` so the tests can assert that the +//! edited path is passed as an argument (the arg-preserving contract) and that +//! the editor's exit status maps to the right [`EditOutcome`], without a real +//! editor or a tty. +#![cfg(unix)] + +use camino_tempfile::NamedUtf8TempFile; +use jp_editor::{EditOutcome, EditRequest, EditorBackend, TerminalEditorBackend}; + +/// `sh -c