From 8d623c0b3a471d6be59343536dde91362215ea6b Mon Sep 17 00:00:00 2001 From: daniel Date: Wed, 21 Oct 2026 18:21:01 +0100 Subject: [PATCH 1/6] feat(tui): add ACP user question forms --- README.md | 4 +- docs/user/compose-and-local-tools.md | 10 +- docs/user/tui-and-sessions.md | 13 + src/protocols/acp.rs | 1 + src/protocols/acp/v2.rs | 48 ++ src/runtime.rs | 12 + src/runtime/tests.rs | 16 +- src/tools/ask_user.rs | 529 +++++++++++++ src/tools/ask_user_tests.rs | 588 ++++++++++++++ src/tools/mod.rs | 1 + src/tui/app.rs | 47 +- src/tui/app/recovery.rs | 2 + src/tui/elicitation.rs | 1057 ++++++++++++++++++++++++++ src/tui/elicitation_requests.rs | 652 ++++++++++++++++ src/tui/mod.rs | 26 +- src/tui/ui.rs | 9 +- 16 files changed, 3005 insertions(+), 10 deletions(-) create mode 100644 src/tools/ask_user.rs create mode 100644 src/tools/ask_user_tests.rs create mode 100644 src/tui/elicitation.rs create mode 100644 src/tui/elicitation_requests.rs diff --git a/README.md b/README.md index 917e5f7d..ca7084de 100644 --- a/README.md +++ b/README.md @@ -30,11 +30,11 @@ kit init && kit auth login openai && kit tui ## Why Kit -Most agent harnesses expose many tools and require one model round trip for each call. Kit exposes one tool: `compose`. The `compose` argument is a short program in [Runlet](https://github.com/danielkov/runlet). In one round trip, the program can read files, run tests, apply edits, retry failed commands, delegate work to subagents, and return structured data. +Most agent harnesses expose many tools and require one model round trip for each call. Kit exposes `compose` for tool orchestration, plus `ask_user` for interactive decisions when the connected client supports forms. The `compose` argument is a short program in [Runlet](https://github.com/danielkov/runlet). In one round trip, the program can read files, run tests, apply edits, retry failed commands, delegate work to subagents, and return structured data. Fewer model round trips repeat less context and complete more work in each request. In size-matched production tasks, Kit used approximately half the input tokens and active time per hand-written line of Codex CLI or Claude Code ([comparison](#how-it-compares)). -- **One tool, unlimited composition.** The model receives one tool: `compose`. A `compose` program can call `shell`, `edit`, `subagent`, `prompt`, `fork`, `tool_search`, `tool_schema`, `tool`, `auth`, `skill`, `a2a`, and `docs`. Independent calls run concurrently. Use data dependencies or `after` blocks to control execution order. Use `boundary retry N` and `fail()` to handle errors in the program. +- **One tool for orchestration, unlimited composition.** The model uses `compose` to orchestrate work. A `compose` program can call `shell`, `edit`, `subagent`, `prompt`, `fork`, `tool_search`, `tool_schema`, `tool`, `auth`, `skill`, `a2a`, and `docs`. Independent calls run concurrently. Use data dependencies or `after` blocks to control execution order. Use `boundary retry N` and `fail()` to handle errors in the program. - **Reusable subagents.** A subagent is a reusable value. You can continue, fork, inspect, and close a subagent. You can require JSON that matches a schema. You can also use Claude Code, Codex, Cursor, or Kit as the subagent harness over ACP. - **Open protocols.** Kit supports ACP v1 and v2 over stdio, HTTP/SSE, and WebSocket. Kit supports A2A v1 in both directions. It also supports MCP, Agent Skills, and Agent Plugin packages. - **Long sessions.** Kit synchronizes each append-only JSONL transcript item to disk before it accepts the item. It compacts context automatically at 80% of the context window. You can resume a session from `tui`, `prompt`, or any ACP client. Kit also uses crash-safe locks. It retries eligible `openai-subscription` failures for up to 24 hours. diff --git a/docs/user/compose-and-local-tools.md b/docs/user/compose-and-local-tools.md index 154bcecf..c70abd10 100644 --- a/docs/user/compose-and-local-tools.md +++ b/docs/user/compose-and-local-tools.md @@ -1,6 +1,14 @@ # Compose, Runlet, and Local Tools -Kit exposes one model-visible tool, `compose`. A compose call contains a Runlet program that can invoke Kit's hidden tools, including `shell`, `edit`, `a2a`, subagents, Agent Skills, bundled documentation, and MCP meta-tools. Users normally ask Kit to do work through an installed-binary command such as `kit tui --root /path/to/project` or `kit prompt --root /path/to/project "Run the smallest relevant test"`; the agent writes the Runlet program. Use `kit --help` and `kit --help` for the exhaustive CLI reference. +Kit exposes `compose` for tool orchestration and, when the connected client supports forms, `ask_user` for interactive decisions. A compose call contains a Runlet program that can invoke Kit's hidden tools, including `shell`, `edit`, `a2a`, subagents, Agent Skills, bundled documentation, and MCP meta-tools. Users normally ask Kit to do work through an installed-binary command such as `kit tui --root /path/to/project` or `kit prompt --root /path/to/project "Run the smallest relevant test"`; the agent writes the Runlet program. Use `kit --help` and `kit --help` for the exhaustive CLI reference. + +## Ask for a user decision + +When an ACP v2 client advertises form elicitation, Kit also exposes `ask_user` as a standalone tool. It is not a Runlet function and cannot run in the background. Use it for decisions that need explicit user input, not credentials or secrets. + +A request contains a `message` and 1–8 `questions`. Each question has a stable `id`, `title`, `prompt`, and 1–32 `options` with stable IDs and display labels. Set `multiple: true` for multiple selections and `allow_custom: true` to offer a separate text field. Both flags default to false. The complete form request is limited to 128 KiB; shorten descriptions or split the batch if the tool reports that the form is too large. + +Accepted answers are keyed by question ID, with selected option IDs in `selected` and optional text in `custom`. A declined or dismissed request returns `action: "decline"` or `action: "cancel"` without answers; do not treat either as a selection. The bundled TUI supports [paged forms and answer review](tui-and-sessions.md#answer-agent-questions). Clients without form support do not receive this tool; use ordinary chat questions instead. ## How a compose Runlet works diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index c7e0997a..51a1c9a4 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -133,6 +133,19 @@ Press `Command+B` to detach the newest running foreground top-level compose call At an idle, non-empty editor, `Ctrl+C` clears the prompt instead of unexpectedly discarding it and quitting in one step; press it again with the empty editor to quit. +## Answer agent questions + +When an agent needs a decision, Kit can show a form with single-choice or multiple-choice options and optional custom-answer fields. The form identifies the requesting agent and explains what it needs. Answers stay local to the form until you submit the review page. + +- Use `Up` / `Down` to highlight an option and `Space` to select it. For multiple-choice fields, `Space` toggles each option independently. +- Type into text or numeric fields. Use `Left` / `Right` to choose false / true for a boolean field, or `Space` to toggle it. +- Press `Enter` to advance and `Shift+Tab` to go back without losing answers. Optional custom answers appear on a separate page. +- On the review page, press `Enter` to submit all answers together, or `Shift+Tab` to revise them. +- Press `Delete` to clear the current field and `PageUp` / `PageDown` to scroll. +- Press `Esc` to dismiss the request, or `Ctrl+D` to explicitly decline. Neither action submits selections. + +Do not enter passwords, API keys, or other credentials in these forms. Kit supports session-scoped ACP form elicitation, not URL elicitation. Unsupported form constraints are rejected rather than silently ignored. If your ACP client does not support forms, ask and answer in ordinary chat instead. + ## Read the transcript The transcript keeps a three-column gutter: `›` marks your prompts, `⋮` reasoning, a spinner, `✓`, or `✗` each compose program, `⇅` a compaction, and `↩` a background result landing after its turn ended. Prose has an empty gutter, so the left edge alone shows the turn's shape. diff --git a/src/protocols/acp.rs b/src/protocols/acp.rs index 6df6fd36..0583364f 100644 --- a/src/protocols/acp.rs +++ b/src/protocols/acp.rs @@ -1810,6 +1810,7 @@ impl Server { ) .with_hook_messages(self.runtime.hooks()); let context = AcpDriverContext { + ask_user: None, cwd, additional_directories, integration: Arc::new(observer), diff --git a/src/protocols/acp/v2.rs b/src/protocols/acp/v2.rs index dff86cde..d758ba69 100644 --- a/src/protocols/acp/v2.rs +++ b/src/protocols/acp/v2.rs @@ -728,6 +728,8 @@ struct PendingSessionPublication { } struct Server { + // Connection-local negotiated snapshot; only initialize writes it. + form_elicitation: AtomicBool, runtime: Arc, integration: Arc, registry: SessionRegistry, @@ -754,6 +756,7 @@ impl Server { fn new(runtime: Arc, registry: SessionRegistry) -> Self { Self { + form_elicitation: AtomicBool::new(false), runtime, integration: Arc::new(AcpIntegration::default()), registry, @@ -870,6 +873,14 @@ impl Server { &self, request: wire::InitializeRequest, ) -> Result { + self.form_elicitation.store( + request + .capabilities + .elicitation + .as_ref() + .is_some_and(|value| value.form.is_some()), + Ordering::Release, + ); // This handler only speaks v2. Initialization must return our supported // version even when the requested version is unsupported; the client // then disconnects if it cannot speak v2. It does not switch wire schemas. @@ -1065,6 +1076,9 @@ impl Server { .subscribe(session_id.to_string()) .map_err(AcpRuntimeError::Loop)?; let cancellation = CancellationController::new(); + let ask_user = self.form_elicitation.load(Ordering::Acquire).then(|| { + crate::tools::ask_user::AskUserTool::new(connection.clone(), session_id.clone()) + }); let sink = ResponseReplacementSink::new(ConnectionSink(connection)); let activity = native_activity(session_id.clone(), sink.clone()); let binding = @@ -1083,6 +1097,7 @@ impl Server { ) .with_hook_messages(self.runtime.hooks()); let context = AcpDriverContext { + ask_user, cwd, additional_directories, integration: Arc::new(observer), @@ -6706,6 +6721,39 @@ mod tests { .initialize(terminal_auth_initialize_request()) .unwrap(); + assert!(!server.form_elicitation.load(Ordering::Acquire)); + for (capabilities, expected) in [ + ( + wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new() + .form(wire::ElicitationFormCapabilities::new()), + ), + true, + ), + ( + wire::ClientCapabilities::new().elicitation(wire::ElicitationCapabilities::new()), + false, + ), + ( + wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new() + .url(wire::ElicitationUrlCapabilities::new()), + ), + false, + ), + (wire::ClientCapabilities::new(), false), + ] { + server + .initialize( + wire::InitializeRequest::new( + wire::ProtocolVersion::V2, + wire::Implementation::new("questions-client", "0"), + ) + .capabilities(capabilities), + ) + .unwrap(); + assert_eq!(server.form_elicitation.load(Ordering::Acquire), expected); + } assert_eq!(response.protocol_version, wire::ProtocolVersion::V2); assert_eq!(response.auth_methods.len(), 4); assert!(matches!( diff --git a/src/runtime.rs b/src/runtime.rs index 4a7ba3b5..f43d347a 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -323,6 +323,7 @@ pub(crate) struct AcpForkState { } pub(crate) struct AcpDriverContext { + pub ask_user: Option, pub cwd: PathBuf, pub additional_directories: Vec, pub integration: Arc, @@ -1977,6 +1978,16 @@ impl Runtime { crate::ahp::PromptRecord::Session(opened.observer.clone()), )); } + if let Some(ask_user) = context.ask_user { + // User interaction remains foreground-only, outside backgroundable compose. + let questions = agentkit_tools_core::ToolRegistry::new().with(Observed::new(ask_user)); + let questions = if let Some(hooks) = &self.hooks { + crate::ahp::hooked_tools(questions, hooks) + } else { + questions + }; + builder = builder.add_tool_source(questions); + } let driver = builder .observer(context.integration.as_ref().clone()) .observer(crate::fatal::RetryLog) @@ -3311,6 +3322,7 @@ mod additional_directory_tests { let driver = runtime .start_acp_driver_with_mcp( AcpDriverContext { + ask_user: None, cwd: root.path().to_path_buf(), additional_directories: vec![], integration: Arc::new(Observer), diff --git a/src/runtime/tests.rs b/src/runtime/tests.rs index 327e8883..b1be39cd 100644 --- a/src/runtime/tests.rs +++ b/src/runtime/tests.rs @@ -262,6 +262,7 @@ mod reasoning_effort_persistence { let driver = runtime .start_acp_driver_with_mcp( AcpDriverContext { + ask_user: None, cwd: runtime.root().to_path_buf(), additional_directories: vec![], integration: Arc::new(Observer), @@ -1424,7 +1425,20 @@ async fn compose_can_load_a_skill_added_after_its_spec_is_frozen() { fn compose_is_the_only_visible_tool_and_documents_mcp_meta_tools() { let root = tempfile::tempdir().unwrap(); let runtime = Runtime::new(root.path(), "gpt-5.4").unwrap(); - let specs = runtime.compose(0).specs(); + let compose = runtime.compose(0); + let specs = compose.specs(); + assert!( + compose + .backgroundable + .hidden_tools + .get(&ToolName::new("ask_user")) + .is_none(), + "foreground user questions must never be available through compose" + ); + assert!( + !specs.iter().any(|spec| spec.name.0 == "ask_user"), + "ordinary non-ACP runtime catalogs must not advertise user questions" + ); assert_eq!(specs.len(), 1); assert_eq!(specs[0].name.0, "compose"); assert_eq!( diff --git a/src/tools/ask_user.rs b/src/tools/ask_user.rs new file mode 100644 index 00000000..9a370999 --- /dev/null +++ b/src/tools/ask_user.rs @@ -0,0 +1,529 @@ +//! Connection-bound, foreground-only structured user questions. +use std::{collections::BTreeSet, sync::Arc}; + +use agent_client_protocol::{Client, V2ConnectionTo, schema::v2 as wire}; +use agentkit_core::{ToolOutput, ToolResultPart}; +use agentkit_tools_core::{ + Tool, ToolAnnotations, ToolContext, ToolError, ToolName, ToolRequest, ToolResult, ToolSpec, +}; +use async_trait::async_trait; +use futures_util::future::{Either, select}; +use serde::Deserialize; +use serde_json::Value; + +#[cfg(test)] +#[allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::disallowed_methods, + clippy::disallowed_macros +)] +#[path = "ask_user_tests.rs"] +mod tests; + +const MAX_FORM_REQUEST_BYTES: usize = 128 * 1024; + +fn object(fields: [(&str, Value); N]) -> Value { + Value::Object( + fields + .into_iter() + .map(|(key, value)| (key.into(), value)) + .collect(), + ) +} + +fn strings(values: Vec) -> Value { + Value::Array(values.into_iter().map(Value::from).collect()) +} + +fn text_schema(min: usize, max: usize) -> Value { + object([ + ("type", Value::from("string")), + ("minLength", Value::from(min)), + ("maxLength", Value::from(max)), + ]) +} + +#[derive(Clone)] +pub(crate) struct AskUserTool { + spec: ToolSpec, + connection: V2ConnectionTo, + session: wire::SessionId, + // This lock owns no mutable data. The permit serializes dialogs and their + // requires_action/running pair. Cancellation drops the permit; no repair or + // poison recovery is needed. No other lock is acquired by this owner. + admission: Arc>, +} + +impl AskUserTool { + pub(crate) fn new(connection: V2ConnectionTo, session: wire::SessionId) -> Self { + let option = object([ + ("type", Value::from("object")), + ( + "properties", + object([ + ("id", text_schema(1, 64)), + ("label", text_schema(1, 200)), + ("description", text_schema(0, 1000)), + ]), + ), + ("required", strings(vec!["id".into(), "label".into()])), + ("additionalProperties", Value::from(false)), + ]); + let question = object([ + ("type", Value::from("object")), + ( + "properties", + object([ + ( + "id", + object([ + ("type", Value::from("string")), + ("pattern", Value::from("^[A-Za-z][A-Za-z0-9_-]{0,63}$")), + ]), + ), + ("title", text_schema(1, 120)), + ("prompt", text_schema(1, 2000)), + ( + "options", + object([ + ("type", Value::from("array")), + ("minItems", Value::from(1)), + ("maxItems", Value::from(32)), + ("items", option), + ]), + ), + ("multiple", object([("type", Value::from("boolean"))])), + ("allow_custom", object([("type", Value::from("boolean"))])), + ]), + ), + ( + "required", + strings(vec![ + "id".into(), + "title".into(), + "prompt".into(), + "options".into(), + ]), + ), + ("additionalProperties", Value::from(false)), + ]); + let input_schema = object([ + ("type", Value::from("object")), + ( + "properties", + object([ + ("message", text_schema(1, 2000)), + ( + "questions", + object([ + ("type", Value::from("array")), + ("minItems", Value::from(1)), + ("maxItems", Value::from(8)), + ("items", question), + ]), + ), + ]), + ), + ( + "required", + strings(vec!["message".into(), "questions".into()]), + ), + ("additionalProperties", Value::from(false)), + ]); + let answer = object([ + ("type", Value::from("object")), + ( + "properties", + object([ + ( + "selected", + object([ + ("type", Value::from("array")), + ("items", object([("type", Value::from("string"))])), + ]), + ), + ("custom", object([("type", Value::from("string"))])), + ]), + ), + ("required", strings(vec!["selected".into()])), + ("additionalProperties", Value::from(false)), + ]); + let output_schema = object([ + ("type", Value::from("object")), + ( + "properties", + object([ + ( + "action", + object([ + ("type", Value::from("string")), + ( + "enum", + strings(vec!["accept".into(), "decline".into(), "cancel".into()]), + ), + ]), + ), + ( + "answers", + object([ + ("type", Value::from("object")), + ("additionalProperties", answer), + ]), + ), + ]), + ), + ("required", strings(vec!["action".into()])), + ("additionalProperties", Value::from(false)), + ]); + Self { + spec: ToolSpec::new(ToolName::new("ask_user"), + "Ask the user to choose options or provide a custom answer. Use for decisions that need user input; do not request passwords, API keys, credentials, or other secrets. Questions are presented together and submitted after review. The complete serialized form request must fit within 128 KiB (UTF-8); shorten descriptions or split large batches. Decline and cancel are not answers; do not invent selections. This foreground tool is separate from compose and cannot run in the background.", + input_schema) + .with_output_schema(output_schema) + .with_annotations(ToolAnnotations::new()), + connection, session, admission: Arc::new(tokio::sync::Mutex::new(())), + } + } +} + +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct Input { + message: String, + questions: Vec, +} +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct Question { + id: String, + title: String, + prompt: String, + options: Vec, + #[serde(default)] + multiple: bool, + #[serde(default)] + allow_custom: bool, +} +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct OptionValue { + id: String, + label: String, + #[serde(default)] + description: String, +} + +fn invalid(message: impl Into) -> ToolError { + ToolError::InvalidInput(message.into()) +} +fn bounded(value: &str, min: usize, max: usize) -> bool { + (min..=max).contains(&value.chars().count()) && (min == 0 || !value.trim().is_empty()) +} + +impl Input { + fn validate(&self) -> Result<(), ToolError> { + if !bounded(&self.message, 1, 2000) || !(1..=8).contains(&self.questions.len()) { + return Err(invalid( + "message must be 1–2000 characters and questions must contain 1–8 entries", + )); + } + let mut ids = BTreeSet::new(); + for q in &self.questions { + let valid_id = bounded(&q.id, 1, 64) + && q.id.starts_with(|c: char| c.is_ascii_alphabetic()) + && q.id + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-'); + if !valid_id + || !ids.insert(&q.id) + || !bounded(&q.title, 1, 120) + || !bounded(&q.prompt, 1, 2000) + || !(1..=32).contains(&q.options.len()) + { + return Err(invalid( + "questions require unique identifier IDs, bounded title/prompt, and 1–32 options", + )); + } + let mut options = BTreeSet::new(); + for option in &q.options { + if !bounded(&option.id, 1, 64) + || !options.insert(&option.id) + || !bounded(&option.label, 1, 200) + || !bounded(&option.description, 0, 1000) + { + return Err(invalid( + "options require unique IDs and bounded labels/descriptions", + )); + } + } + } + Ok(()) + } + + fn schema(&self) -> Result { + self.validate()?; + let mut properties = serde_json::Map::new(); + let mut required = Vec::new(); + let mut order = Vec::new(); + let mut require_answer = Vec::new(); + for (index, q) in self.questions.iter().enumerate() { + // Generated wire names cannot collide with model-provided IDs. + let key = format!("question_{index}"); + let choices = Value::Array( + q.options + .iter() + .map(|o| { + object([ + ("const", Value::from(o.id.clone())), + ("title", Value::from(o.label.clone())), + ("description", Value::from(o.description.clone())), + ]) + }) + .collect(), + ); + let field = if q.multiple { + object([ + ("type", Value::from("array")), + ("title", Value::from(q.title.clone())), + ("description", Value::from(q.prompt.clone())), + ("items", object([("anyOf", choices)])), + ("minItems", Value::from(if q.allow_custom { 0 } else { 1 })), + ("maxItems", Value::from(q.options.len())), + ]) + } else { + object([ + ("type", Value::from("string")), + ("title", Value::from(q.title.clone())), + ("description", Value::from(q.prompt.clone())), + ("oneOf", choices), + ]) + }; + properties.insert(key.clone(), field); + if !q.allow_custom { + required.push(key.clone()); + } + order.push(key.clone()); + if q.allow_custom { + let custom = format!("question_{index}_custom"); + properties.insert( + custom.clone(), + object([ + ("type", Value::from("string")), + ("title", Value::from(format!("{} — custom answer", q.title))), + ( + "description", + Value::from( + "Optional custom answer. Provide a selection or a custom answer.", + ), + ), + ("maxLength", Value::from(4000)), + ]), + ); + require_answer.push(object([ + ("selection", Value::from(key)), + ("custom", Value::from(custom.clone())), + ])); + order.push(custom); + } + } + serde_json::from_value(object([ + ("type", Value::from("object")), + ("properties", Value::Object(properties)), + ("required", strings(required)), + ( + "_meta", + object([( + "kit/questions", + object([ + ("order", strings(order)), + ("require_answer", Value::Array(require_answer)), + ]), + )]), + ), + ])) + .map_err(|e| invalid(e.to_string())) + } + + fn request( + &self, + session: wire::SessionId, + call: wire::ToolCallId, + ) -> Result { + let scope = wire::ElicitationSessionScope::new(session).tool_call_id(call); + let request = wire::CreateElicitationRequest::new( + wire::ElicitationFormMode::new(scope, self.schema()?), + self.message.clone(), + ); + let bytes = serde_json::to_vec(&request).map_err(|error| invalid(error.to_string()))?; + if bytes.len() > MAX_FORM_REQUEST_BYTES { + return Err(invalid( + "The complete question form exceeds 128 KiB of serialized UTF-8. Shorten question/option descriptions or split the questions into smaller batches.", + )); + } + Ok(request) + } + + fn answer(&self, response: wire::CreateElicitationResponse) -> Result { + let failure = + || ToolError::ExecutionFailed("client returned an invalid question response".into()); + let accepted = match response.action { + wire::ElicitationAction::Accept(value) => value, + wire::ElicitationAction::Decline => { + return Ok(object([("action", Value::from("decline"))])); + } + wire::ElicitationAction::Cancel => { + return Ok(object([("action", Value::from("cancel"))])); + } + _ => return Err(failure()), + }; + let content = accepted.content.ok_or_else(failure)?; + let mut allowed = BTreeSet::new(); + let mut answers = serde_json::Map::new(); + for (index, q) in self.questions.iter().enumerate() { + let key = format!("question_{index}"); + allowed.insert(key.clone()); + let selected = match content.get(&key) { + Some(wire::ElicitationContentValue::String(value)) if !q.multiple => { + vec![value.clone()] + } + Some(wire::ElicitationContentValue::StringArray(values)) + if q.multiple && values.len() <= q.options.len() => + { + values.clone() + } + None if q.allow_custom => Vec::new(), + _ => return Err(failure()), + }; + let unique: BTreeSet<_> = selected.iter().collect(); + if unique.len() != selected.len() + || selected + .iter() + .any(|id| !q.options.iter().any(|option| &option.id == id)) + { + return Err(failure()); + } + let custom_key = format!("question_{index}_custom"); + let custom = if q.allow_custom { + allowed.insert(custom_key.clone()); + match content.get(&custom_key) { + Some(wire::ElicitationContentValue::String(value)) + if bounded(value, 0, 4000) => + { + Some(value) + } + None => None, + _ => return Err(failure()), + } + } else { + None + }; + if selected.is_empty() && custom.is_none_or(|value| value.trim().is_empty()) { + return Err(failure()); + } + let mut answer = serde_json::Map::from_iter([("selected".into(), strings(selected))]); + if let Some(custom) = custom { + answer.insert("custom".into(), Value::from(custom.clone())); + } + answers.insert(q.id.clone(), Value::Object(answer)); + } + if content.keys().any(|key| !allowed.contains(key)) { + return Err(failure()); + } + Ok(object([ + ("action", Value::from("accept")), + ("answers", Value::Object(answers)), + ])) + } +} + +// Created only after the request is admitted. On ordinary completion, transport +// failure, unwind, or future cancellation the state is restored before releasing +// admission. The actor owns the eventual idle transition. This tool is not in the +// backgroundable compose registry, so it cannot outlive foreground settlement. +struct Waiting<'a>(&'a AskUserTool); +impl Drop for Waiting<'_> { + fn drop(&mut self) { + let _ = self + .0 + .state(wire::StateUpdate::Running(wire::RunningStateUpdate::new())); + } +} +impl AskUserTool { + fn state(&self, state: wire::StateUpdate) -> Result<(), ToolError> { + self.connection + .send_notification(wire::UpdateSessionNotification::new( + self.session.clone(), + wire::SessionUpdate::StateUpdate(state), + )) + .map_err(|error| ToolError::ExecutionFailed(error.to_string())) + } +} + +#[async_trait] +impl Tool for AskUserTool { + fn spec(&self) -> &ToolSpec { + &self.spec + } + async fn invoke( + &self, + request: ToolRequest, + context: &mut ToolContext<'_>, + ) -> Result { + if request.session_id.0.as_str() != self.session.0.as_ref() { + return Err(ToolError::ExecutionFailed( + "question belongs to a different session".into(), + )); + } + let input: Input = + serde_json::from_value(request.input).map_err(|e| invalid(e.to_string()))?; + let elicitation = input.request( + self.session.clone(), + wire::ToolCallId::new(request.call_id.to_string()), + )?; + let cancellation = context.cancellation.clone(); + let interrupted = async { + match &cancellation { + Some(cancel) => cancel.cancelled().await, + None => std::future::pending().await, + } + }; + let operation = async { + let _admission = self.admission.lock().await; + if cancellation + .as_ref() + .is_some_and(|cancel| cancel.is_cancelled()) + { + return Err(ToolError::Cancelled); + } + self.state(wire::StateUpdate::RequiresAction( + wire::RequiresActionStateUpdate::new(), + ))?; + let _waiting = Waiting(self); + // Dropping block_task drops SentRequest: the SDK sends the targeted + // $/cancel_request, never a blanket cancellation of session dialogs. + let response = self + .connection + .send_request(elicitation) + .block_task() + .await + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + if cancellation + .as_ref() + .is_some_and(|cancel| cancel.is_cancelled()) + { + return Err(ToolError::Cancelled); + } + input.answer(response) + }; + // Poll cancellation first; drop the losing operation before returning. + match select(std::pin::pin!(interrupted), std::pin::pin!(operation)).await { + Either::Left(((), _)) => Err(ToolError::Cancelled), + Either::Right((value, _)) => Ok(ToolResult::new(ToolResultPart::success( + request.call_id, + ToolOutput::structured(value?), + ))), + } + } +} diff --git a/src/tools/ask_user_tests.rs b/src/tools/ask_user_tests.rs new file mode 100644 index 00000000..8256ceac --- /dev/null +++ b/src/tools/ask_user_tests.rs @@ -0,0 +1,588 @@ +use super::*; +use agentkit_core::{CancellationController, MetadataMap, SessionId, TurnId}; +use agentkit_tools_core::{AllowAllPermissions, OwnedToolContext}; +use serde_json::json; + +fn example() -> Value { + json!({"message":"Choose the implementation.","questions":[ + {"id":"scope","title":"Scope","prompt":"Which parts?","multiple":true,"allow_custom":true,"options":[{"id":"tui","label":"TUI"},{"id":"runtime","label":"Runtime"}]}, + {"id":"strategy","title":"Strategy","prompt":"How?","options":[{"id":"small","label":"Small patch"},{"id":"full","label":"Full implementation"}]} + ]}) +} +fn input() -> Input { + serde_json::from_value(example()).unwrap() +} +fn response(value: Value) -> wire::CreateElicitationResponse { + serde_json::from_value(value).unwrap() +} + +#[test] +fn form_preserves_order_ids_labels_and_custom_fields() { + let schema = serde_json::to_value(input().schema().unwrap()).unwrap(); + assert_eq!( + schema["_meta"]["kit/questions"]["order"], + json!(["question_0", "question_0_custom", "question_1"]) + ); + assert_eq!(schema["required"], json!(["question_1"])); + assert_eq!( + schema["properties"]["question_0"]["items"]["anyOf"][0]["const"], + "tui" + ); + assert_eq!( + schema["properties"]["question_1"]["oneOf"][0]["title"], + "Small patch" + ); +} + +#[test] +fn accepted_answers_are_keyed_by_stable_ids_and_custom_is_separate() { + let result = input().answer(response(json!({"action":"accept","content":{"question_0":["runtime","tui"],"question_0_custom":"Also docs, please","question_1":"small"}}))).unwrap(); + assert_eq!( + result, + json!({"action":"accept","answers":{"scope":{"selected":["runtime","tui"],"custom":"Also docs, please"},"strategy":{"selected":["small"]}}}) + ); + assert!(input().answer(response(json!({"action":"accept","content":{"question_0_custom":"Different approach","question_1":"small"}}))).is_ok()); +} + +#[test] +fn rejects_missing_unknown_duplicate_or_wrong_typed_answers() { + for content in [ + json!({"question_0":[],"question_1":"small"}), + json!({"question_0":["tui","tui"],"question_1":"small"}), + json!({"question_0":["missing"],"question_1":"small"}), + json!({"question_0":"tui","question_1":"small"}), + json!({"question_0":["tui"],"question_1":["small"]}), + json!({"question_0":["tui"]}), + json!({"question_0":["tui"],"question_1":"small","extra":"ignored?"}), + json!({"question_0":["tui"],"question_1":"small","question_1_custom":"not allowed"}), + ] { + assert!( + input() + .answer(response(json!({"action":"accept","content":content}))) + .is_err(), + "{content}" + ); + } + assert!( + input() + .answer(response(json!({"action":"accept"}))) + .is_err() + ); + for action in ["cancel", "decline"] { + assert_eq!( + input().answer(response(json!({"action":action}))).unwrap(), + json!({"action":action}) + ); + } +} + +#[test] +fn rejects_duplicate_ids_and_bounded_input_violations() { + for value in [ + { + let mut v = example(); + v["questions"][1]["id"] = json!("scope"); + v + }, + { + let mut v = example(); + v["questions"][0]["options"][1]["id"] = json!("tui"); + v + }, + { + let mut v = example(); + v["questions"][0]["id"] = json!("invalid.id"); + v + }, + { + let mut v = example(); + v["questions"][0]["options"] = json!([]); + v + }, + { + let mut v = example(); + v["message"] = json!("x".repeat(2001)); + v + }, + { + let mut v = example(); + v["questions"] = json!([]); + v + }, + ] { + assert!( + serde_json::from_value::(value) + .unwrap() + .schema() + .is_err() + ); + } +} + +fn context(controller: &CancellationController) -> OwnedToolContext { + OwnedToolContext { + session_id: SessionId::new("session"), + turn_id: TurnId::new("turn"), + metadata: MetadataMap::new(), + permissions: Arc::new(AllowAllPermissions), + resources: Arc::new(()), + cancellation: Some(controller.handle().checkpoint()), + execution_scope: None, + approved_request: None, + } +} + +#[tokio::test] +async fn real_acp_request_returns_answer_and_restores_running() { + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let (states, mut received) = tokio::sync::mpsc::unbounded_channel(); + let client = agent_client_protocol::Client.v2() + .on_receive_notification(async move |notification: wire::UpdateSessionNotification, _cx| { + if let wire::SessionUpdate::StateUpdate(state) = notification.update { states.send(state).unwrap(); } + Ok(()) + }, agent_client_protocol::on_receive_notification!()) + .on_receive_request(async move |request: wire::CreateElicitationRequest, responder, _cx| { + assert_eq!(request.message,"Choose the implementation."); + let wire::ElicitationMode::Form(form) = request.mode else { panic!("expected form") }; + let wire::ElicitationScope::Session(scope) = form.scope else { panic!("expected session") }; + assert_eq!(scope.session_id.to_string(),"session"); + assert_eq!(scope.tool_call_id.unwrap().to_string(),"call"); + responder.respond(response(json!({"action":"accept","content":{"question_0":["tui"],"question_1":"small"}}))) + }, agent_client_protocol::on_receive_request!()); + let (ready, initialized) = tokio::sync::oneshot::channel(); + let task = tokio::spawn(async move { + client + .connect_with(client_transport, async move |connection| { + connection + .send_request( + wire::InitializeRequest::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test", "0"), + ) + .capabilities( + wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new() + .form(wire::ElicitationFormCapabilities::new()), + ), + ), + ) + .block_task() + .await?; + let _ = ready.send(()); + std::future::pending::>().await + }) + .await + }); + agent_client_protocol::Agent + .v2() + .on_receive_request( + async move |_request: wire::InitializeRequest, responder, _cx| { + responder.respond(wire::InitializeResponse::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test-agent", "0"), + )) + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_with(agent_transport, async move |connection| { + initialized.await.unwrap(); + let tool = AskUserTool::new(connection, wire::SessionId::new("session")); + let cancelled_controller = CancellationController::new(); + let cancelled_context = context(&cancelled_controller); + cancelled_controller.interrupt(); + assert!(matches!( + tool.invoke( + ToolRequest::new( + "already-cancelled", + "ask_user", + example(), + "session", + "turn" + ), + &mut cancelled_context.borrowed(), + ) + .await, + Err(ToolError::Cancelled) + )); + let controller = CancellationController::new(); + let context = context(&controller); + let wrong_session = tool + .invoke( + ToolRequest::new("wrong", "ask_user", example(), "another-session", "turn"), + &mut context.borrowed(), + ) + .await; + assert!(matches!(wrong_session, Err(ToolError::ExecutionFailed(_)))); + let result = tool + .invoke( + ToolRequest::new("call", "ask_user", example(), "session", "turn"), + &mut context.borrowed(), + ) + .await + .unwrap(); + assert!(!result.result.is_error); + assert!(matches!( + received.recv().await, + Some(wire::StateUpdate::RequiresAction(_)) + )); + assert!(matches!( + received.recv().await, + Some(wire::StateUpdate::Running(_)) + )); + Ok::<_, agent_client_protocol::Error>(()) + }) + .await + .unwrap(); + task.abort(); +} + +#[tokio::test] +async fn turn_cancellation_cancels_individual_acp_request() { + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let controller = CancellationController::new(); + let interrupt = controller.clone(); + let (cancelled, cancellation_seen) = tokio::sync::oneshot::channel(); + let cancelled = Arc::new(tokio::sync::Mutex::new(Some(cancelled))); + let (states, mut received_states) = tokio::sync::mpsc::unbounded_channel(); + let client = agent_client_protocol::Client + .v2() + .on_receive_notification( + async move |notification: wire::UpdateSessionNotification, _cx| { + if let wire::SessionUpdate::StateUpdate(state) = notification.update { + states.send(state).unwrap(); + } + Ok(()) + }, + agent_client_protocol::on_receive_notification!(), + ) + .on_receive_request( + async move |_request: wire::CreateElicitationRequest, responder, cx| { + let cancellation = responder.cancellation(); + let cancelled = cancelled.clone(); + interrupt.interrupt(); + cx.spawn(async move { + cancellation.cancelled().await; + if let Some(sender) = cancelled.lock().await.take() { + let _ = sender.send(()); + } + responder.respond(response(json!({"action":"cancel"}))) + })?; + Ok(()) + }, + agent_client_protocol::on_receive_request!(), + ); + let (ready, initialized) = tokio::sync::oneshot::channel(); + let task = tokio::spawn(async move { + client + .connect_with(client_transport, async move |connection| { + connection + .send_request( + wire::InitializeRequest::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test", "0"), + ) + .capabilities( + wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new() + .form(wire::ElicitationFormCapabilities::new()), + ), + ), + ) + .block_task() + .await?; + let _ = ready.send(()); + std::future::pending::>().await + }) + .await + }); + tokio::time::timeout( + std::time::Duration::from_secs(5), + agent_client_protocol::Agent + .v2() + .on_receive_request( + async move |_request: wire::InitializeRequest, responder, _cx| { + responder.respond(wire::InitializeResponse::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test-agent", "0"), + )) + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_with(agent_transport, async move |connection| { + initialized.await.unwrap(); + let tool = AskUserTool::new(connection, wire::SessionId::new("session")); + let context = context(&controller); + let result = tool + .invoke( + ToolRequest::new("call", "ask_user", example(), "session", "turn"), + &mut context.borrowed(), + ) + .await; + assert!(matches!(result, Err(ToolError::Cancelled))); + cancellation_seen.await.unwrap(); + assert!(matches!( + received_states.recv().await, + Some(wire::StateUpdate::RequiresAction(_)) + )); + assert!(matches!( + received_states.recv().await, + Some(wire::StateUpdate::Running(_)) + )); + Ok::<_, agent_client_protocol::Error>(()) + }), + ) + .await + .unwrap() + .unwrap(); + task.abort(); +} + +#[tokio::test] +async fn cancelled_waiter_does_not_publish_and_admission_is_reusable() { + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let (requests, mut received_requests) = tokio::sync::mpsc::unbounded_channel(); + let (states, mut received_states) = tokio::sync::mpsc::unbounded_channel(); + let client = agent_client_protocol::Client + .v2() + .on_receive_notification( + async move |notification: wire::UpdateSessionNotification, _cx| { + if let wire::SessionUpdate::StateUpdate(state) = notification.update { + states.send(state).unwrap(); + } + Ok(()) + }, + agent_client_protocol::on_receive_notification!(), + ) + .on_receive_request( + async move |_request: wire::CreateElicitationRequest, responder, _cx| { + requests.send(responder).unwrap(); + Ok(()) + }, + agent_client_protocol::on_receive_request!(), + ); + let (ready, initialized) = tokio::sync::oneshot::channel(); + let client_task = tokio::spawn(async move { + client + .connect_with(client_transport, async move |connection| { + connection + .send_request( + wire::InitializeRequest::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test", "0"), + ) + .capabilities( + wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new() + .form(wire::ElicitationFormCapabilities::new()), + ), + ), + ) + .block_task() + .await?; + let _ = ready.send(()); + std::future::pending::>().await + }) + .await + }); + tokio::time::timeout( + std::time::Duration::from_secs(5), + agent_client_protocol::Agent + .v2() + .on_receive_request( + async move |_request: wire::InitializeRequest, responder, _cx| { + responder.respond(wire::InitializeResponse::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test-agent", "0"), + )) + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_with(agent_transport, async move |connection| { + initialized.await.unwrap(); + let tool = AskUserTool::new(connection, wire::SessionId::new("session")); + let clone = tool.clone(); + let first_controller = CancellationController::new(); + let first_context = context(&first_controller); + let mut first_context = first_context.borrowed(); + let first = tool.invoke( + ToolRequest::new("first", "ask_user", example(), "session", "turn"), + &mut first_context, + ); + tokio::pin!(first); + assert!(futures_util::poll!(&mut first).is_pending()); + let first_request = received_requests.recv().await.unwrap(); + assert!(matches!( + received_states.recv().await, + Some(wire::StateUpdate::RequiresAction(_)) + )); + + let second_controller = CancellationController::new(); + let second_context = context(&second_controller); + let mut second_context = second_context.borrowed(); + let second = clone.invoke( + ToolRequest::new("second", "ask_user", example(), "session", "turn"), + &mut second_context, + ); + tokio::pin!(second); + assert!(futures_util::poll!(&mut second).is_pending()); + second_controller.interrupt(); + assert!(matches!(second.await, Err(ToolError::Cancelled))); + first_request.respond(response(json!({"action":"decline"})))?; + assert!(first.await.is_ok()); + assert!(matches!( + received_states.recv().await, + Some(wire::StateUpdate::Running(_)) + )); + + // The cancelled waiter neither steals admission nor leaves a stale RPC. + let third_controller = CancellationController::new(); + let third_context = context(&third_controller); + let mut third_context = third_context.borrowed(); + let (third, ()) = tokio::join!( + tool.invoke( + ToolRequest::new("third", "ask_user", example(), "session", "turn"), + &mut third_context + ), + async { + let request = received_requests.recv().await.unwrap(); + request + .respond(response(json!({"action":"decline"}))) + .unwrap(); + } + ); + assert!(third.is_ok()); + assert!(matches!( + received_states.recv().await, + Some(wire::StateUpdate::RequiresAction(_)) + )); + assert!(matches!( + received_states.recv().await, + Some(wire::StateUpdate::Running(_)) + )); + Ok::<_, agent_client_protocol::Error>(()) + }), + ) + .await + .unwrap() + .unwrap(); + client_task.abort(); +} + +#[tokio::test] +async fn cancelled_turn_does_not_accept_a_racing_response() { + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let controller = CancellationController::new(); + let interrupt = controller.clone(); + let client = agent_client_protocol::Client.v2() + .on_receive_notification(async move |_notification:wire::UpdateSessionNotification,_cx| Ok(()),agent_client_protocol::on_receive_notification!()) + .on_receive_request(async move |_request:wire::CreateElicitationRequest,responder,_cx| { + interrupt.interrupt(); + responder.respond(response(json!({"action":"accept","content":{"question_0":["tui"],"question_1":"small"}}))) + },agent_client_protocol::on_receive_request!()); + let (ready, initialized) = tokio::sync::oneshot::channel(); + let task = tokio::spawn(async move { + client + .connect_with(client_transport, async move |connection| { + connection + .send_request( + wire::InitializeRequest::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test", "0"), + ) + .capabilities( + wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new() + .form(wire::ElicitationFormCapabilities::new()), + ), + ), + ) + .block_task() + .await?; + let _ = ready.send(()); + std::future::pending::>().await + }) + .await + }); + tokio::time::timeout( + std::time::Duration::from_secs(5), + agent_client_protocol::Agent + .v2() + .on_receive_request( + async move |_request: wire::InitializeRequest, responder, _cx| { + responder.respond(wire::InitializeResponse::new( + agentkit_acp::v2::wire::ProtocolVersion::V2, + wire::Implementation::new("questions-test-agent", "0"), + )) + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_with(agent_transport, async move |connection| { + initialized.await.unwrap(); + let tool = AskUserTool::new(connection, wire::SessionId::new("session")); + let context = context(&controller); + let result = tool + .invoke( + ToolRequest::new("call", "ask_user", example(), "session", "turn"), + &mut context.borrowed(), + ) + .await; + assert!(matches!(result, Err(ToolError::Cancelled))); + Ok::<_, agent_client_protocol::Error>(()) + }), + ) + .await + .unwrap() + .unwrap(); + task.abort(); +} + +#[test] +fn custom_answer_requirements_are_advertised_for_client_validation() { + let schema = serde_json::to_value(input().schema().unwrap()).unwrap(); + assert_eq!( + schema["_meta"]["kit/questions"]["require_answer"], + json!([ + {"selection":"question_0", "custom":"question_0_custom"} + ]) + ); + for (selection, custom) in [(json!([]), json!("")), (json!([]), json!(" "))] { + assert!( + input() + .answer(response(json!({"action":"accept", "content":{ + "question_0":selection, "question_0_custom":custom, "question_1":"small" + }}))) + .is_err() + ); + } +} + +#[test] +fn aggregate_request_limit_counts_serialized_utf8_not_characters() { + let make = |character: &str| -> Input { + serde_json::from_value(json!({"message":"Choose options", "questions":(0..8).map(|q| json!({ + "id":format!("q_{q}"),"title":"Scope","prompt":"Which parts?", + "options":(0..32).map(|i| json!({"id":format!("option_{i}"),"label":"Option","description":character.repeat(300)})).collect::>() + })).collect::>()})).unwrap() + }; + let ascii = make("a"); + let unicode = make("🦀"); + assert!(ascii.validate().is_ok()); + assert!( + unicode.validate().is_ok(), + "all per-field character limits are satisfied" + ); + let accepted = ascii + .request( + wire::SessionId::new("session"), + wire::ToolCallId::new("call"), + ) + .unwrap(); + assert!(serde_json::to_vec(&accepted).unwrap().len() <= MAX_FORM_REQUEST_BYTES); + let rejected = unicode.request( + wire::SessionId::new("session"), + wire::ToolCallId::new("call"), + ); + assert!( + matches!(rejected, Err(ToolError::InvalidInput(message)) if message.contains("128 KiB") && message.contains("split")) + ); +} diff --git a/src/tools/mod.rs b/src/tools/mod.rs index b9a82920..98d7d75c 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -1,5 +1,6 @@ mod a2a; mod artifact; +pub(crate) mod ask_user; mod docs; mod edit; pub(crate) mod mcp; diff --git a/src/tui/app.rs b/src/tui/app.rs index c4f0eb8c..ab7fe0a9 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -49,6 +49,7 @@ pub use focus::ChildView; /// Everything the client learns from the agent or its own runtime channel. #[derive(Debug)] pub enum Update { + Elicitation(super::elicitation_requests::Pending), GatewayRecovery(crate::tui::recovery::Event), ChildSteerFinished { id: String, @@ -849,6 +850,7 @@ pub struct App { pub model: String, pub model_choices: Vec, pub model_dialog: Option, + pub(super) elicitations: VecDeque, pub model_switch: Option, next_model_switch: u64, pub reasoning_effort: String, @@ -1151,6 +1153,7 @@ impl App { model, model_choices: Vec::new(), model_dialog: None, + elicitations: VecDeque::new(), model_switch: None, next_model_switch: 0, reasoning_effort: "default".into(), @@ -1476,6 +1479,7 @@ impl App { /// This does not mean that every poll needs a terminal frame. pub fn needs_redraw_tick(&self) -> bool { (!self.runtime_status_unavailable && self.runtime_last_frame.is_some()) + || !self.elicitations.is_empty() || self.toast.is_some() || self.has_tick_animation() } @@ -1497,6 +1501,10 @@ impl App { /// Runs periodic maintenance and reports whether a new frame is needed. pub fn tick(&mut self) -> bool { + let pending_count = self.elicitations.len(); + self.elicitations + .retain(|pending| !pending.reply.is_closed()); + let forms_changed = self.elicitations.len() != pending_count; let animated = self.has_tick_animation(); let unavailable = self.runtime_status_unavailable; let toast = self.toast.is_some(); @@ -1504,6 +1512,7 @@ impl App { self.runtime_tick_at(Instant::now()); self.tick_at(crate::events::now_millis()); animated + || forms_changed || unavailable != self.runtime_status_unavailable || toast != self.toast.is_some() || agents != self.agents.len() @@ -2254,6 +2263,13 @@ impl App { fn apply_observed(&mut self, update: Update) { let at = self.observed_at(); match update { + Update::Elicitation(pending) => { + self.elicitations + .retain(|pending| !pending.reply.is_closed()); + if !pending.reply.is_closed() { + self.elicitations.push_back(pending); + } + } Update::GatewayRecovery(event) => self.apply_recovery(event, Vec::new()), Update::ChildSteerFinished { id, @@ -2626,6 +2642,7 @@ impl App { _ => {} }, Update::ProcessExited(error) => { + self.elicitations.clear(); // Confirmed process exit can retire known roster rows. A mere // diagnostic gap cannot claim those terminal outcomes. self.invalidate_runtime_status(); @@ -3122,6 +3139,7 @@ impl App { /// history and diagnostics remain useful, while transcript-derived state /// starts empty. pub fn start_session(&mut self, session_id: String) { + self.elicitations.clear(); self.model_switch = None; self.cancel_steer_edit(); self.selected_steer = None; @@ -3469,7 +3487,8 @@ impl App { } pub(super) fn clipboard_route(&self) -> ClipboardRoute { - if self.child_focus.is_some() + if !self.elicitations.is_empty() + || self.child_focus.is_some() || self.model_switch.is_some() || self.model_dialog.is_some() || self.effort_dialog.is_some() @@ -3496,6 +3515,10 @@ impl App { } pub fn paste(&mut self, text: &str) { + if let Some(pending) = self.elicitations.front_mut() { + pending.form.paste(text); + return; + } if self.child_focus.is_some() { self.paste_child(text); return; @@ -4211,6 +4234,25 @@ impl App { if key.kind != KeyEventKind::Press { return Action::None; } + if self + .elicitations + .front() + .is_some_and(|pending| pending.reply.is_closed()) + { + self.elicitations + .retain(|pending| !pending.reply.is_closed()); + return Action::Redraw; + } + if let Some(pending) = self.elicitations.front_mut() { + if let Some(response) = pending.form.key(key) + && let Some(pending) = self.elicitations.pop_front() + { + let _ = pending.reply.send(response); + } + // End the input burst before exposing a new page, queued form or + // the composer. Buffered Enter must not act on an unseen target. + return Action::Redraw; + } if key.code == KeyCode::Char('c') && key.modifiers == KeyModifiers::SUPER { self.command_copy_observed = true; } @@ -4786,6 +4828,9 @@ impl App { } pub fn handle_mouse(&mut self, mouse: MouseEvent) -> Action { + if !self.elicitations.is_empty() { + return Action::None; + } if let Some(action) = self.handle_focus_mouse(mouse) { return action; } diff --git a/src/tui/app/recovery.rs b/src/tui/app/recovery.rs index bed89d8a..c491c8e2 100644 --- a/src/tui/app/recovery.rs +++ b/src/tui/app/recovery.rs @@ -14,6 +14,7 @@ pub(super) struct Candidate { impl App { fn recovery_failed(&mut self, message: &str) { + self.elicitations.clear(); self.gateway_candidate = None; self.gateway_blocked = true; self.gateway_status = Some(format!( @@ -59,6 +60,7 @@ impl App { }); self.gateway_epoch = marker.epoch; self.gateway_blocked = true; + self.elicitations.clear(); self.model_switch = None; self.model_dialog = None; self.effort_dialog = None; diff --git a/src/tui/elicitation.rs b/src/tui/elicitation.rs new file mode 100644 index 00000000..f64f1d46 --- /dev/null +++ b/src/tui/elicitation.rs @@ -0,0 +1,1057 @@ +//! Bounded, local-only ACP forms. No answer is sent before the review page. +use agent_client_protocol::schema::v2 as wire; +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; +use ratatui::{ + Frame, + layout::{Constraint, Layout}, + widgets::{Block, Borders, Clear, Paragraph, Wrap}, +}; +use serde_json::{Map, Value}; + +const MAX_TEXT: usize = 4096; + +#[derive(Debug)] +pub(super) struct Form { + message: String, + fields: Vec, + require_answer: Vec, + page: usize, + scroll: u16, + error: Option, +} + +#[derive(Debug)] +struct AnswerRequirement { + selection: usize, + custom: usize, +} + +#[derive(Debug)] +struct Field { + name: String, + schema: Value, + required: bool, + kind: String, + options: Vec, + cursor: usize, + value: Value, + text: String, +} + +#[derive(Debug)] +struct Choice { + value: String, + label: String, + description: String, +} + +// Keep wire values intact; strip terminal and directional controls only at display. +fn display(text: &str) -> String { + text.chars().filter(|c| (!c.is_control() || matches!(c, '\n' | '\t')) && !matches!(c, '\u{061c}' | '\u{200e}' | '\u{200f}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}')).collect() +} + +fn annotation(v: &Value, key: &str) -> String { + v.get(key) + .and_then(Value::as_str) + .unwrap_or_default() + .to_owned() +} + +fn keys(v: &Value, allowed: &[&str]) -> Result<(), String> { + let object = v.as_object().ok_or("Expected a schema object")?; + for key in object.keys() { + if !allowed.contains(&key.as_str()) { + return Err(format!("Unsupported schema keyword: {key}")); + } + } + Ok(()) +} + +fn bounded(v: &Value) -> Result<(), String> { + match v { + Value::String(s) if s.chars().count() > MAX_TEXT => { + return Err("Text exceeds 4096 characters".into()); + } + Value::Array(a) => { + for v in a { + bounded(v)?; + } + } + Value::Object(o) => { + for (k, v) in o { + if k.chars().count() > MAX_TEXT { + return Err("Field name too long".into()); + } + bounded(v)?; + } + } + _ => {} + } + Ok(()) +} + +impl Field { + fn new(name: String, schema: Value, required: bool) -> Result { + let kind = annotation(&schema, "type"); + let mut allowed = vec!["type", "title", "description", "default", "_meta"]; + match kind.as_str() { + "string" => allowed.extend(["minLength", "maxLength", "enum", "oneOf"]), + "number" | "integer" => allowed.extend(["minimum", "maximum"]), + "boolean" => {} + "array" => allowed.extend(["items", "minItems", "maxItems"]), + _ => return Err(format!("Unsupported field type: {kind}")), + } + keys(&schema, &allowed)?; + for key in ["title", "description"] { + if !schema[key].is_null() && !schema[key].is_string() { + return Err(format!("Expected string {key}")); + } + } + for key in ["minLength", "maxLength", "minItems", "maxItems"] { + if !schema[key].is_null() && schema[key].as_u64().is_none() { + return Err(format!("Expected nonnegative integer {key}")); + } + } + for key in ["minimum", "maximum"] { + if !schema[key].is_null() + && (schema[key].as_f64().is_none() + || (kind == "integer" && schema[key].as_i64().is_none())) + { + return Err(format!("Invalid numeric bound {key}")); + } + } + let default = &schema["default"]; + if !default.is_null() + && !match kind.as_str() { + "string" => default.is_string(), + "number" => default.is_number(), + "integer" => default.as_i64().is_some(), + "boolean" => default.is_boolean(), + "array" => default.is_array(), + _ => false, + } + { + return Err("Default has wrong type".into()); + } + for (min, max) in [ + ("minLength", "maxLength"), + ("minItems", "maxItems"), + ("minimum", "maximum"), + ] { + if let (Some(a), Some(b)) = (schema[min].as_f64(), schema[max].as_f64()) + && a > b + { + return Err(format!("Invalid bounds for {name}")); + } + } + if kind == "integer" + && let (Some(min), Some(max)) = (schema["minimum"].as_i64(), schema["maximum"].as_i64()) + && min > max + { + return Err("Invalid integer bounds".into()); + } + if schema["minLength"] + .as_u64() + .is_some_and(|n| n > MAX_TEXT as u64) + { + return Err("Minimum length exceeds form limit".into()); + } + let choices = if kind == "array" { + &schema["items"] + } else { + &schema + }; + if kind == "array" { + keys(choices, &["type", "enum", "anyOf", "_meta"])?; + if choices["type"] != "string" + && !(choices["type"].is_null() && choices["anyOf"].is_array()) + { + return Err("Only string multiselect items are supported".into()); + } + } + let titled_key = if kind == "array" { "anyOf" } else { "oneOf" }; + for key in ["enum", titled_key] { + if !choices[key].is_null() && !choices[key].is_array() { + return Err(format!("Expected option array {key}")); + } + } + if !choices["enum"].is_null() && !choices[titled_key].is_null() { + return Err("Ambiguous enum schema".into()); + } + let mut options = Vec::new(); + if let Some(values) = choices["enum"].as_array() { + for value in values { + let value = value.as_str().ok_or("Expected string option")?.to_owned(); + options.push(Choice { + label: value.clone(), + value, + description: String::new(), + }); + } + } else if let Some(values) = choices[titled_key].as_array() { + for value in values { + keys(value, &["const", "title", "description", "_meta"])?; + options.push(Choice { + value: value["const"] + .as_str() + .ok_or("Expected string constant")? + .to_owned(), + label: value["title"] + .as_str() + .ok_or("Expected option title")? + .to_owned(), + description: annotation(value, "description"), + }); + } + } + if options.len() > 64 + || ((kind == "array" || !choices["enum"].is_null() || !choices[titled_key].is_null()) + && options.is_empty()) + { + return Err("Enums require 1–64 options".into()); + } + for (i, option) in options.iter().enumerate() { + if options[..i].iter().any(|o| o.value == option.value) { + return Err("Duplicate option value".into()); + } + } + if schema["minItems"] + .as_u64() + .is_some_and(|n| n > options.len() as u64) + { + return Err("Minimum selections exceeds available options".into()); + } + let value = schema.get("default").cloned().unwrap_or(Value::Null); + let text = match &value { + Value::String(s) => s.clone(), + Value::Number(n) => n.to_string(), + _ => String::new(), + }; + let cursor = options.iter().position(|o| value == o.value).unwrap_or(0); + let field = Self { + name, + schema, + required, + kind, + options, + cursor, + value, + text, + }; + if !field.value.is_null() { + field.validate()?; + } + Ok(field) + } + + fn answer(&self) -> Result { + if self.options.is_empty() { + match self.kind.as_str() { + "string" => { + return Ok( + if self.text.is_empty() && !self.required && self.value.is_null() { + Value::Null + } else { + Value::String(self.text.clone()) + }, + ); + } + "number" | "integer" => { + if self.text.is_empty() { + return Ok(Value::Null); + } + let value: Value = + serde_json::from_str(&self.text).map_err(|_| "Enter a valid number")?; + if !value.is_number() || (self.kind == "integer" && value.as_i64().is_none()) { + return Err("Enter a valid number of the requested type".into()); + } + return Ok(value); + } + _ => {} + } + } + Ok(self.value.clone()) + } + + fn validate(&self) -> Result { + let value = self.answer()?; + if value.is_null() { + return if self.required { + Err("An answer is required".into()) + } else { + Ok(value) + }; + } + let (size, min, max) = match self.kind.as_str() { + "string" => { + let s = value.as_str().ok_or("Expected text")?; + if !self.options.is_empty() && !self.options.iter().any(|o| o.value == s) { + return Err("Select a listed option".into()); + } + (Some(s.chars().count() as u64), "minLength", "maxLength") + } + "array" => { + let a = value.as_array().ok_or("Expected selections")?; + for (i, v) in a.iter().enumerate() { + if !self + .options + .iter() + .any(|o| v == &Value::String(o.value.clone())) + || a[..i].contains(v) + { + return Err("Invalid or duplicate selection".into()); + } + } + (Some(a.len() as u64), "minItems", "maxItems") + } + "boolean" => { + if !value.is_boolean() { + return Err("Choose yes or no".into()); + } + (None, "", "") + } + "integer" => { + let n = value.as_i64().ok_or("Expected an integer")?; + if self.schema["minimum"].as_i64().is_some_and(|min| n < min) + || self.schema["maximum"].as_i64().is_some_and(|max| n > max) + { + return Err("Integer is outside the allowed range".into()); + } + (None, "", "") + } + _ => { + let n = value.as_f64().ok_or("Expected a number")?; + if self.schema["minimum"].as_f64().is_some_and(|min| n < min) + || self.schema["maximum"].as_f64().is_some_and(|max| n > max) + { + return Err("Number is outside the allowed range".into()); + } + (None, "", "") + } + }; + if let Some(size) = size + && (self.schema[min].as_u64().is_some_and(|n| size < n) + || self.schema[max].as_u64().is_some_and(|n| size > n)) + { + return Err("Answer does not meet the length or selection limits".into()); + } + Ok(value) + } + + fn selected(&self, option: &Choice) -> bool { + self.value == option.value + || self + .value + .as_array() + .is_some_and(|a| a.contains(&Value::String(option.value.clone()))) + } + + fn toggle(&mut self) { + if self.kind == "boolean" { + self.value = Value::Bool(!self.value.as_bool().unwrap_or(false)); + } else if let Some(option) = self.options.get(self.cursor) { + let value = Value::String(option.value.clone()); + if self.kind == "array" { + let mut values = self.value.as_array().cloned().unwrap_or_default(); + if let Some(index) = values.iter().position(|v| *v == value) { + values.remove(index); + } else { + values.push(value); + } + self.value = Value::Array(values); + } else { + self.value = value; + } + } + } +} + +fn answer_requirements(schema: &Value, fields: &[Field]) -> Result, String> { + let Some(meta) = schema.get("_meta").filter(|meta| !meta.is_null()) else { + return Ok(Vec::new()); + }; + let meta = meta.as_object().ok_or("Expected metadata object")?; + let Some(questions) = meta.get("kit/questions") else { + return Ok(Vec::new()); + }; + let questions = questions + .as_object() + .ok_or("Expected question metadata object")?; + let Some(groups) = questions.get("require_answer") else { + return Ok(Vec::new()); + }; + let groups = groups + .as_array() + .ok_or("Expected answer requirement array")?; + if groups.len() > fields.len() { + return Err("Too many answer requirements".into()); + } + let mut requirements: Vec = Vec::new(); + for group in groups { + keys(group, &["selection", "custom"])?; + let selection = group["selection"] + .as_str() + .ok_or("Expected selection field name")?; + let custom = group["custom"] + .as_str() + .ok_or("Expected custom field name")?; + let selection = fields + .iter() + .position(|field| field.name == selection) + .ok_or("Unknown selection field")?; + let custom = fields + .iter() + .position(|field| field.name == custom) + .ok_or("Unknown custom field")?; + if fields[selection].options.is_empty() + || fields[custom].kind != "string" + || !fields[custom].options.is_empty() + || requirements + .iter() + .any(|rule| rule.selection == selection || rule.custom == custom) + { + return Err("Invalid or repeated answer requirement".into()); + } + requirements.push(AnswerRequirement { selection, custom }); + } + Ok(requirements) +} + +impl Form { + pub(super) fn with_origin(mut self, origin: &str) -> Self { + let origin: String = origin.chars().take(MAX_TEXT).collect(); + self.message = format!("Request from {origin}\n\n{}", self.message); + self + } + + #[cfg(test)] + pub(super) fn new(request: wire::CreateElicitationRequest) -> Result { + Self::from_value(serde_json::to_value(request).map_err(|e| e.to_string())?) + } + + pub(super) fn from_value(request: Value) -> Result { + let encoded = serde_json::to_vec(&request).map_err(|e| e.to_string())?; + if encoded.len() > 128 * 1024 { + return Err("Form exceeds 128 KiB".into()); + } + bounded(&request)?; + if !request["message"].is_string() { + return Err("Expected request message".into()); + } + if request["mode"] != "form" { + return Err("Only form elicitation is supported".into()); + } + let schema = &request["requestedSchema"]; + keys( + schema, + &[ + "type", + "title", + "description", + "properties", + "required", + "_meta", + ], + )?; + if schema["type"] != "object" { + return Err("Expected object schema".into()); + } + let properties = schema["properties"] + .as_object() + .ok_or("Expected properties")?; + if properties.len() > 32 { + return Err("Form exceeds 32 fields".into()); + } + if !schema["required"].is_null() && !schema["required"].is_array() { + return Err("Expected required array".into()); + } + for key in ["title", "description"] { + if !schema[key].is_null() && !schema[key].is_string() { + return Err(format!("Expected string {key}")); + } + } + let required = schema["required"].as_array().cloned().unwrap_or_default(); + for name in &required { + if !name.as_str().is_some_and(|n| properties.contains_key(n)) { + return Err("Unknown required field".into()); + } + } + let mut names = Vec::new(); + let order = &schema["_meta"]["kit/questions"]["order"]; + if !order.is_null() && !order.is_array() { + return Err("Expected question order array".into()); + } + if let Some(order) = order.as_array() { + for name in order { + let name = name.as_str().ok_or("Invalid question order")?; + if !properties.contains_key(name) || names.contains(&name.to_owned()) { + return Err("Invalid question order".into()); + } + names.push(name.to_owned()); + } + } + for name in properties.keys() { + if !names.contains(name) { + names.push(name.clone()); + } + } + let fields = names + .into_iter() + .map(|name| { + let required = required.contains(&Value::String(name.clone())); + Field::new(name.clone(), properties[&name].clone(), required) + }) + .collect::, _>>()?; + let require_answer = answer_requirements(schema, &fields)?; + Ok(Self { + message: [ + annotation(&request, "message"), + annotation(schema, "title"), + annotation(schema, "description"), + ] + .into_iter() + .filter(|s| !s.is_empty()) + .collect::>() + .join("\n"), + fields, + require_answer, + page: 0, + scroll: 0, + error: None, + }) + } + + pub(super) fn paste(&mut self, text: &str) { + if let Some(field) = self.fields.get_mut(self.page) + && field.options.is_empty() + && matches!(field.kind.as_str(), "string" | "number" | "integer") + { + let remaining = MAX_TEXT.saturating_sub(field.text.chars().count()); + field + .text + .extend(text.chars().filter(|c| !c.is_control()).take(remaining)); + if field.kind == "string" { + field.value = Value::String(field.text.clone()); + } + self.error = None; + } + } + + pub(super) fn key(&mut self, key: KeyEvent) -> Option { + if key.kind == KeyEventKind::Release + || (key.kind == KeyEventKind::Repeat && key.code == KeyCode::Enter) + { + return None; + } + if key.code == KeyCode::Esc { + return Some(wire::CreateElicitationResponse::new( + wire::ElicitationAction::Cancel, + )); + } + if key.code == KeyCode::Char('d') && key.modifiers.contains(KeyModifiers::CONTROL) { + return Some(wire::CreateElicitationResponse::new( + wire::ElicitationAction::Decline, + )); + } + match key.code { + KeyCode::BackTab => { + self.page = self.page.saturating_sub(1); + self.scroll = 0; + self.error = None; + } + KeyCode::PageDown => self.scroll = self.scroll.saturating_add(5), + KeyCode::PageUp => self.scroll = self.scroll.saturating_sub(5), + KeyCode::Enter => { + if self.page == self.fields.len() { + let mut content = Map::new(); + for (index, field) in self.fields.iter().enumerate() { + match field.validate() { + Ok(value) => { + if !value.is_null() { + content.insert(field.name.clone(), value); + } + } + Err(error) => { + self.page = index; + self.error = Some(error); + return None; + } + } + } + for requirement in &self.require_answer { + let selected = match content.get(&self.fields[requirement.selection].name) { + Some(Value::String(_)) => true, + Some(Value::Array(values)) => !values.is_empty(), + _ => false, + }; + let custom = content + .get(&self.fields[requirement.custom].name) + .and_then(Value::as_str) + .is_some_and(|value| !value.trim().is_empty()); + if !selected && !custom { + self.page = requirement.custom; + self.scroll = 0; + self.error = + Some("Select an option or enter a non-blank custom answer.".into()); + return None; + } + } + return serde_json::from_value(Value::Object(Map::from_iter([ + ("action".into(), Value::String("accept".into())), + ("content".into(), Value::Object(content)), + ]))) + .ok(); + } + match self.fields[self.page].validate() { + Ok(_) => { + self.page += 1; + self.scroll = 0; + self.error = None; + } + Err(error) => self.error = Some(error), + } + } + _ => { + if let Some(field) = self.fields.get_mut(self.page) { + match key.code { + KeyCode::Up => { + field.cursor = field.cursor.saturating_sub(1); + self.scroll = 0; + } + KeyCode::Down => { + field.cursor = + (field.cursor + 1).min(field.options.len().saturating_sub(1)); + self.scroll = 0; + } + KeyCode::Left if field.kind == "boolean" => { + field.value = Value::Bool(false) + } + KeyCode::Right if field.kind == "boolean" => { + field.value = Value::Bool(true) + } + KeyCode::Char(' ') + if !field.options.is_empty() || field.kind == "boolean" => + { + field.toggle() + } + KeyCode::Delete => { + field.value = Value::Null; + field.text.clear(); + } + KeyCode::Backspace => { + field.text.pop(); + } + KeyCode::Char(c) + if !key + .modifiers + .intersects(KeyModifiers::CONTROL | KeyModifiers::ALT) => + { + self.paste(&c.to_string()) + } + _ => {} + } + } + } + } + None + } + + pub(super) fn render(&mut self, frame: &mut Frame) { + let area = frame.area(); + frame.render_widget(Clear, area); + let block = Block::default() + .title("Agent request") + .borders(Borders::ALL) + .border_style(super::theme::accent()); + let inner = block.inner(area); + frame.render_widget(block, area); + let parts = Layout::vertical([Constraint::Min(1), Constraint::Length(3)]).split(inner); + let mut text = format!("{}\n\n", self.message); + if let Some(field) = self.fields.get(self.page) { + let title = annotation(&field.schema, "title"); + text.push_str(&format!( + "Question {}/{} — {} {}\n{}\n", + self.page + 1, + self.fields.len(), + if title.is_empty() { + &field.name + } else { + &title + }, + if field.required { + "(required)" + } else { + "(optional)" + }, + annotation(&field.schema, "description") + )); + for key in [ + "minLength", + "maxLength", + "minimum", + "maximum", + "minItems", + "maxItems", + ] { + if !field.schema[key].is_null() { + text.push_str(&format!("{key}: {} ", field.schema[key])); + } + } + text.push('\n'); + if !field.options.is_empty() { + let option = &field.options[field.cursor]; + text.push_str(&format!( + "Highlighted: {}\n{}\n", + option.label, option.description + )); + let regions = + Layout::vertical([Constraint::Percentage(60), Constraint::Percentage(40)]) + .split(parts[0]); + render_text(frame, regions[0], text, &mut self.scroll); + let height = usize::from(regions[1].height).max(1); + let start = field.cursor.saturating_sub(height - 1); + let lines = field + .options + .iter() + .enumerate() + .skip(start) + .take(height) + .map(|(i, o)| { + format!( + "{} [{}] {}", + if i == field.cursor { ">" } else { " " }, + if field.selected(o) { "x" } else { " " }, + o.label + ) + }) + .collect::>() + .join("\n"); + frame.render_widget(Paragraph::new(display(&lines)), regions[1]); + } else { + text.push_str(&format!( + "\nAnswer: {}", + if field.kind == "boolean" { + match field.value.as_bool() { + Some(true) => "Yes", + Some(false) => "No", + None => "Not answered (← No / → Yes)", + } + } else { + &field.text + } + )); + render_text(frame, parts[0], text, &mut self.scroll); + } + } else { + text.push_str("Review answers — BackTab to modify; Enter to submit\n\n"); + for field in &self.fields { + let value = field.answer().unwrap_or(Value::Null); + let display = if value.is_null() { + "(omitted)".into() + } else if !field.options.is_empty() { + field + .options + .iter() + .filter(|o| field.selected(o)) + .map(|o| o.label.clone()) + .collect::>() + .join(", ") + } else { + value.to_string() + }; + text.push_str(&format!("{}: {}\n", field.name, display)); + } + render_text(frame, parts[0], text, &mut self.scroll); + } + frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • BackTab: back • Space: select • Delete: clear\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); + } +} + +fn render_text(frame: &mut Frame, area: ratatui::layout::Rect, text: String, scroll: &mut u16) { + use unicode_segmentation::UnicodeSegmentation; + use unicode_width::UnicodeWidthStr; + + // Hard-wrap graphemes ourselves: ratatui's rendered line count is unstable. + // This also provides an exact scroll bound without enabling extra features. + let text = display(&text).replace('\t', " "); + let width = usize::from(area.width).max(1); + let mut lines = Vec::new(); + let mut line = String::new(); + let mut column = 0; + for grapheme in text.graphemes(true) { + if grapheme == "\n" { + lines.push(std::mem::take(&mut line)); + column = 0; + } else { + let size = grapheme.width(); + if column > 0 && column + size > width { + lines.push(std::mem::take(&mut line)); + column = 0; + } + line.push_str(grapheme); + column += size; + } + } + lines.push(line); + let max = lines.len().saturating_sub(usize::from(area.height)); + *scroll = (*scroll).min(max.min(u16::MAX as usize) as u16); + let paragraph = Paragraph::new(lines.join("\n")); + frame.render_widget(paragraph.scroll((*scroll, 0)), area); +} + +#[cfg(test)] +#[allow(clippy::disallowed_macros, clippy::disallowed_methods)] +mod tests { + use super::*; + use serde_json::json; + fn form(properties: Value, required: Value) -> Result { + Form::new(serde_json::from_value(json!({"mode":"form", "scope":"session", "sessionId":"test", "message":"Please answer", "requestedSchema":{"type":"object", "properties":properties, "required":required}})).unwrap()) + } + fn press(form: &mut Form, code: KeyCode) -> Option { + form.key(KeyEvent::new(code, KeyModifiers::NONE)) + } + #[test] + fn review_is_required_and_edits_are_preserved() { + let mut f = form( + json!({"name":{"type":"string", "minLength":2}}), + json!(["name"]), + ) + .unwrap(); + assert!(press(&mut f, KeyCode::Enter).is_none()); + assert_eq!(f.page, 0); + f.paste("Ada"); + assert!(press(&mut f, KeyCode::Enter).is_none()); + assert_eq!(f.page, 1); + press(&mut f, KeyCode::BackTab); + assert_eq!(f.fields[0].text, "Ada"); + press(&mut f, KeyCode::Enter); + let result = serde_json::to_value(press(&mut f, KeyCode::Enter).unwrap()).unwrap(); + assert_eq!(result["content"]["name"], "Ada"); + } + #[test] + fn native_titled_multiselect_roundtrip() { + let field = wire::MultiSelectPropertySchema::titled(vec![wire::EnumOption::new( + "stable", + "Friendly label", + )]); + let schema = wire::ElicitationSchema::new().property("choice", field, true); + let request = wire::CreateElicitationRequest::new( + wire::ElicitationFormMode::new(wire::ElicitationSessionScope::new("test"), schema), + "Choose", + ); + let encoded = serde_json::to_value(&request).unwrap(); + assert!(encoded["requestedSchema"]["properties"]["choice"]["items"]["type"].is_null()); + let decoded = serde_json::from_value(encoded).unwrap(); + let mut form = Form::new(decoded).unwrap(); + press(&mut form, KeyCode::Char(' ')); + press(&mut form, KeyCode::Enter); + let response = serde_json::to_value(press(&mut form, KeyCode::Enter).unwrap()).unwrap(); + assert_eq!(response["content"]["choice"], json!(["stable"])); + } + + #[test] + fn raw_constraints_are_not_lost_by_sdk_decoding() { + let request = json!({"mode":"form", "sessionId":"test", "message":"Answer", "requestedSchema":{"type":"object", "properties":{"x":{"type":"string"}}}}); + for restriction in [ + json!({"const":"fixed"}), + json!({"const":null}), + json!({"minLength":-1}), + json!({"default":false}), + json!({"enum":"wrong"}), + ] { + let mut raw = request.clone(); + raw["requestedSchema"]["properties"]["x"] + .as_object_mut() + .unwrap() + .extend(restriction.as_object().unwrap().clone()); + assert!(Form::from_value(raw).is_err()); + } + let mut raw = request.clone(); + raw["requestedSchema"]["type"] = json!("array"); + assert!(Form::from_value(raw).is_err()); + let mut raw = request; + raw["requestedSchema"]["additionalProperties"] = json!(false); + assert!(Form::from_value(raw).is_err()); + } + + #[test] + fn display_strips_controls_but_preserves_wire_text() { + assert_eq!( + display("safe\u{1b}[2J\u{202e}text\nline"), + "safe[2Jtext\nline" + ); + let mut form = form( + json!({"x":{"type":"string","default":"a\u{1b}b"}}), + json!([]), + ) + .unwrap(); + assert_eq!(form.fields[0].validate().unwrap(), json!("a\u{1b}b")); + press(&mut form, KeyCode::Backspace); + assert_eq!(form.fields[0].validate().unwrap(), json!("a\u{1b}")); + } + + #[test] + fn stable_multiselect_values_and_defaults() { + let mut f = form(json!({"pick":{"type":"array","items":{"type":"string","anyOf":[{"const":"a","title":"First"},{"const":"b","title":"Second"}]},"default":["b"],"minItems":1}}), json!(["pick"])).unwrap(); + press(&mut f, KeyCode::Char(' ')); + assert_eq!(f.fields[0].validate().unwrap(), json!(["b", "a"])); + press(&mut f, KeyCode::Enter); + press(&mut f, KeyCode::BackTab); + assert_eq!(f.fields[0].validate().unwrap(), json!(["b", "a"])); + } + #[test] + fn unsupported_constraints_and_invalid_defaults_fail_closed() { + assert!(form(json!({"x":{"type":"string","pattern":"a+"}}), json!([])).is_err()); + assert!(form(json!({"x":{"type":"string","format":"email"}}), json!([])).is_err()); + assert!( + form( + json!({"x":{"type":"integer","minimum":3,"default":2}}), + json!([]) + ) + .is_err() + ); + assert!(form(json!({"x":{"type":"_future"}}), json!([])).is_err()); + } + #[test] + fn answer_requirement_metadata_rejects_unsafe_references() { + let base = json!({"mode":"form","message":"Choose","requestedSchema":{"type":"object","properties":{ + "pick":{"type":"string","enum":["a"]},"custom":{"type":"string"},"other":{"type":"string","enum":["b"]} + },"_meta":{"kit/questions":{"require_answer":[]}}}}); + for requirement in [ + json!(null), + json!({}), + json!([{}]), + json!([{"selection":"missing","custom":"custom"}]), + json!([{"selection":"custom","custom":"pick"}]), + json!([{"selection":"pick","custom":"other"}]), + json!([{"selection":"pick","custom":"custom","unknown":true}]), + json!([{"selection":"pick","custom":"custom"},{"selection":"pick","custom":"custom"}]), + ] { + let mut raw = base.clone(); + raw["requestedSchema"]["_meta"]["kit/questions"]["require_answer"] = requirement; + assert!(Form::from_value(raw).is_err()); + } + let mut raw = base; + raw["requestedSchema"]["_meta"]["kit/questions"]["require_answer"] = + json!([{"selection":"pick","custom":"custom"}]); + raw["requestedSchema"]["properties"]["pick"]["default"] = json!("a"); + raw["requestedSchema"]["properties"]["custom"]["default"] = json!(" "); + let mut form = Form::from_value(raw).unwrap(); + for _ in 0..3 { + press(&mut form, KeyCode::Enter); + } + assert!(matches!( + press(&mut form, KeyCode::Enter).unwrap().action, + wire::ElicitationAction::Accept(_) + )); + } + + #[test] + fn page_up_moves_immediately_after_overscrolling_prompts_descriptions_and_review() { + fn render(form: &mut Form) -> String { + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(40, 12)).unwrap(); + terminal.draw(|frame| form.render(frame)).unwrap(); + terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect() + } + fn assert_movement(form: &mut Form) { + for _ in 0..100 { + press(form, KeyCode::PageDown); + } + let bottom = render(form); + let bottom_offset = form.scroll; + assert!(bottom_offset > 0); + press(form, KeyCode::PageUp); + let above = render(form); + assert!(form.scroll < bottom_offset); + assert_ne!(bottom, above); + } + let lines = |prefix: &str| { + (0..40) + .map(|i| format!("{prefix} line {i}\n")) + .collect::() + }; + for (message, description) in [ + (lines("prompt"), String::new()), + ("Please answer".into(), lines("description")), + ] { + let mut form = Form::from_value(json!({"mode":"form","message":message,"requestedSchema":{"type":"object","properties":{"name":{"type":"string","description":description,"default":"answer"}}}})).unwrap(); + assert_movement(&mut form); + } + let mut choice = Form::from_value(json!({"mode":"form","message":"Choose an option","requestedSchema":{"type":"object","properties":{"choice":{"type":"string","oneOf":[{"const":"a","title":"Option A","description":lines("option description")}],"default":"a"}}}})).unwrap(); + assert_movement(&mut choice); + let properties: Map = (0..20) + .map(|i| { + ( + format!("field_{i:02}"), + json!({"type":"string","default":format!("answer {i}")}), + ) + }) + .collect(); + let mut form = Form::from_value(json!({"mode":"form","message":"Review these answers","requestedSchema":{"type":"object","properties":properties}})).unwrap(); + for _ in 0..20 { + press(&mut form, KeyCode::Enter); + } + assert_eq!(form.page, form.fields.len()); + assert_movement(&mut form); + } + + #[test] + fn renders_choices_review_and_tiny_terminals() { + let mut form = form(json!({"pick":{"type":"string","oneOf":[{"const":"first","title":"First choice","description":"A useful description"}],"default":"first"}}), json!(["pick"])).unwrap(); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + terminal.draw(|frame| form.render(frame)).unwrap(); + let visible = terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect::(); + assert!(visible.contains("First choice")); + assert!(visible.contains("Question 1/1")); + press(&mut form, KeyCode::Enter); + terminal.draw(|frame| form.render(frame)).unwrap(); + let visible = terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect::(); + assert!(visible.contains("Review answers")); + for (width, height) in [(1, 1), (8, 3), (30, 10)] { + let mut tiny = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(width, height)).unwrap(); + tiny.draw(|frame| form.render(frame)).unwrap(); + } + } + + #[test] + fn bounds_optional_and_cancellation() { + let mut f = form(json!({"x":{"type":"string"}}), json!([])).unwrap(); + f.paste(&"é".repeat(5000)); + assert_eq!(f.fields[0].text.chars().count(), MAX_TEXT); + press(&mut f, KeyCode::Delete); + press(&mut f, KeyCode::Enter); + assert_eq!( + serde_json::to_value(press(&mut f, KeyCode::Enter).unwrap()).unwrap()["content"], + json!({}) + ); + assert_eq!( + serde_json::to_value(press(&mut f, KeyCode::Esc).unwrap()).unwrap()["action"], + "cancel" + ); + let decline = f + .key(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)) + .unwrap(); + assert_eq!(serde_json::to_value(decline).unwrap()["action"], "decline"); + } +} diff --git a/src/tui/elicitation_requests.rs b/src/tui/elicitation_requests.rs new file mode 100644 index 00000000..dd505217 --- /dev/null +++ b/src/tui/elicitation_requests.rs @@ -0,0 +1,652 @@ +//! Connection-local admission and ownership for interactive forms. +//! +//! The existing session-route mutex is read only here. Its writers are startup, +//! transition_route and gateway recovery; we clone a complete route while locked +//! and never await, send, or drop user-owned state under that guard. Generation +//! filtering rejects a request racing a session transition. App::start_session +//! drops already-visible forms. No answer can be redirected to a new session. +use std::sync::{Arc, Mutex, OnceLock}; + +use agent_client_protocol::{Responder, schema::v2 as wire}; +use tokio::sync::{OwnedSemaphorePermit, Semaphore, mpsc, oneshot}; + +use super::{ActiveSessionRoute, QueuedUpdate, app::Update, elicitation::Form}; + +const MAX_PENDING: usize = 8; + +// Preserve the original schema: ACP's typed schemas intentionally discard +// unknown keywords, which is not safe for a form validator. +#[derive( + Clone, Debug, serde::Serialize, serde::Deserialize, agent_client_protocol::JsonRpcRequest, +)] +#[serde(transparent)] +#[request(method = "elicitation/create", response = wire::CreateElicitationResponse)] +pub(super) struct RawRequest(pub serde_json::Value); + +#[derive(Debug)] +pub(super) struct Pending { + pub form: Form, + pub reply: oneshot::Sender, + // Admission follows queued/visible ownership, not the handler future. A + // cancelled peer cannot flood the unbounded update channel with dead forms. + _permit: OwnedSemaphorePermit, +} + +pub(super) struct Ingress { + updates: mpsc::UnboundedSender, + route: Arc>, + capacity: Arc, + pub origin: Arc>, +} + +impl Ingress { + pub fn new( + updates: mpsc::UnboundedSender, + route: Arc>, + ) -> Self { + Self { + updates, + route, + capacity: Arc::new(Semaphore::new(MAX_PENDING)), + origin: Arc::new(OnceLock::new()), + } + } + + fn prepare( + &self, + request: RawRequest, + ) -> Result< + ( + QueuedUpdate, + oneshot::Receiver, + ), + String, + > { + if request.0["mode"] != "form" { + return Err("only form elicitation is supported".into()); + } + let session = request.0["sessionId"] + .as_str() + .ok_or("form elicitation requires an active session")?; + if request.0.get("requestId").is_some() { + return Err("ambiguous elicitation scope".into()); + } + let route = self + .route + .lock() + .map_err(|_| "session routing unavailable")? + .clone(); + if route.id.is_empty() || route.id != session { + return Err("elicitation does not belong to the active session".into()); + } + let origin = self + .origin + .get() + .ok_or("agent initialization is incomplete")?; + let permit = self + .capacity + .clone() + .try_acquire_owned() + .map_err(|_| "too many pending user requests")?; + // Validate the original wire budget before adding local presentation text. + let form = Form::from_value(request.0)?.with_origin(origin); + let (reply, receiver) = oneshot::channel(); + let pending = Pending { + form, + reply, + _permit: permit, + }; + Ok(( + QueuedUpdate::for_session(route.generation, Update::Elicitation(pending)), + receiver, + )) + } + + // ACP dispatch callbacks must return immediately: awaiting user input here + // would prevent the same connection from dispatching cancellation/updates. + pub fn dispatch( + self: &Arc, + request: RawRequest, + responder: Responder, + ) -> Result<(), agent_client_protocol::Error> { + let (update, receiver) = match self.prepare(request) { + Ok(pending) => pending, + Err(message) => { + return responder.respond_with_error( + agent_client_protocol::Error::invalid_params().data(message), + ); + } + }; + let generation = update.generation; + if self.updates.send(update).is_err() { + return responder.respond(wire::CreateElicitationResponse::new( + wire::ElicitationAction::Cancel, + )); + } + let ingress = Arc::clone(self); + tokio::spawn(async move { + let _ = ingress.answer(receiver, generation, responder).await; + }); + Ok(()) + } + + async fn answer( + &self, + receiver: oneshot::Receiver, + generation: Option, + responder: Responder, + ) -> Result<(), agent_client_protocol::Error> { + let cancellation = responder.cancellation(); + let response = { + let cancelled = std::pin::pin!(cancellation.cancelled()); + match futures_util::future::select(cancelled, receiver).await { + futures_util::future::Either::Left(_) => { + wire::CreateElicitationResponse::new(wire::ElicitationAction::Cancel) + } + futures_util::future::Either::Right((response, _)) => { + response.unwrap_or_else(|_| { + wire::CreateElicitationResponse::new(wire::ElicitationAction::Cancel) + }) + } + } + }; + // The response decision is fenced against recovery/session transitions. + // The route guard is released before invoking the responder or waking + // transport work. A transition after this snapshot follows this decision. + let current = self + .route + .lock() + .is_ok_and(|route| Some(route.generation) == generation); + let response = if current { + response + } else { + wire::CreateElicitationResponse::new(wire::ElicitationAction::Cancel) + }; + responder.respond(response) + } +} + +#[cfg(test)] +#[allow(clippy::disallowed_macros, clippy::disallowed_methods)] +mod tests { + use super::*; + use crate::tui::{accept_queued_update, app::App, transition_route}; + + fn request(session: &str) -> RawRequest { + serde_json::from_value(serde_json::json!({ + "mode": "form", "sessionId": session, "message": "Choose a scope", + "requestedSchema": {"type":"object", "properties": {"scope": {"type":"string", "enum":["small","large"]}}, "required":["scope"]} + })).unwrap() + } + + fn ingress() -> Ingress { + let (sender, _) = mpsc::unbounded_channel(); + let ingress = Ingress::new( + sender, + Arc::new(Mutex::new(ActiveSessionRoute { + id: "session".into(), + generation: 1, + })), + ); + ingress.origin.set("test-agent".into()).unwrap(); + ingress + } + + fn app() -> App { + let mut app = App::new(".".into(), "test".into(), "test".into(), String::new()); + app.start_session("session".into()); + app + } + + #[test] + fn elicitation_wire_budget_excludes_local_origin_label() { + let ingress = ingress(); + let mut request = request("session"); + request.0["_meta"] = serde_json::json!({"padding": vec!["x".repeat(4096); 32]}); + let size = serde_json::to_vec(&request.0).unwrap().len(); + let excess = size - 128 * 1024; + request.0["_meta"]["padding"][31] = serde_json::Value::String("x".repeat(4096 - excess)); + assert_eq!(serde_json::to_vec(&request.0).unwrap().len(), 128 * 1024); + assert!(ingress.prepare(RawRequest(request.0.clone())).is_ok()); + request.0["_meta"]["padding"][31] = serde_json::Value::String("x".repeat(4097 - excess)); + assert!(ingress.prepare(request).is_err()); + } + + #[test] + fn elicitation_admission_is_bounded_until_queued_forms_are_dropped() { + let ingress = ingress(); + let queued: Vec<_> = (0..MAX_PENDING) + .map(|_| ingress.prepare(request("session")).unwrap()) + .collect(); + assert!(ingress.prepare(request("session")).is_err()); + let queued: Vec<_> = queued + .into_iter() + .map(|(update, receiver)| { + drop(receiver); + update + }) + .collect(); + assert!(ingress.prepare(request("session")).is_err()); + drop(queued); + assert!(ingress.prepare(request("session")).is_ok()); + } + + #[test] + fn elicitation_rejects_wrong_session_and_stale_generation() { + let ingress = ingress(); + assert!(ingress.prepare(request("other")).is_err()); + let (queued, mut receiver) = ingress.prepare(request("session")).unwrap(); + transition_route(&ingress.route, "other".into()); + assert!(accept_queued_update(&ingress.route, queued).is_none()); + assert!(matches!( + receiver.try_recv(), + Err(oneshot::error::TryRecvError::Closed) + )); + } + + #[test] + fn elicitation_session_switch_and_process_exit_close_pending_answers() { + let ingress = ingress(); + let mut app = app(); + let (queued, mut receiver) = ingress.prepare(request("session")).unwrap(); + app.apply(queued.update); + app.start_session("other".into()); + assert!(app.elicitations.is_empty()); + assert!(matches!( + receiver.try_recv(), + Err(oneshot::error::TryRecvError::Closed) + )); + let (queued, mut receiver) = ingress.prepare(request("session")).unwrap(); + app.apply(queued.update); + app.apply(Update::ProcessExited("disconnected".into())); + assert!(app.elicitations.is_empty()); + assert!(matches!( + receiver.try_recv(), + Err(oneshot::error::TryRecvError::Closed) + )); + } + + #[test] + fn elicitation_cancellation_prunes_only_closed_request() { + let ingress = ingress(); + let mut app = app(); + let (first, first_answer) = ingress.prepare(request("session")).unwrap(); + let (second, _second_answer) = ingress.prepare(request("session")).unwrap(); + app.apply(first.update); + app.apply(second.update); + drop(first_answer); + assert!(app.tick()); + assert_eq!(app.elicitations.len(), 1); + } + + #[tokio::test] + async fn elicitation_roundtrip_and_individual_cancellation_over_acp() { + use agent_client_protocol::Channel; + let exercise = async { + let (updates, mut incoming) = mpsc::unbounded_channel(); + let ingress = Ingress::new( + updates, + Arc::new(Mutex::new(ActiveSessionRoute { + id: "session".into(), + generation: 1, + })), + ); + ingress.origin.set("test-agent".into()).unwrap(); + let route = Arc::clone(&ingress.route); + let ingress = Arc::new(ingress); + let client = agent_client_protocol::Client.v2().on_receive_request( + async move |request: RawRequest, responder, _cx| { + ingress.dispatch(request, responder) + }, + agent_client_protocol::on_receive_request!(), + ); + let (client_transport, agent_transport) = Channel::duplex(); + let (ready_tx, ready_rx) = oneshot::channel(); + let (done_tx, done_rx) = oneshot::channel(); + let client_task = tokio::spawn(async move { + client + .connect_with(client_transport, async move |connection| { + connection + .send_request(wire::InitializeRequest::new( + agent_client_protocol::schema::ProtocolVersion::V2, + wire::Implementation::new("test-client", "0"), + )) + .block_task() + .await?; + let _ = ready_tx.send(()); + let _ = done_rx.await; + Ok::<_, agent_client_protocol::Error>(()) + }) + .await + }); + agent_client_protocol::Agent + .v2() + .on_receive_request( + async move |_request: wire::InitializeRequest, responder, _cx| { + responder.respond(wire::InitializeResponse::new( + agent_client_protocol::schema::ProtocolVersion::V2, + wire::Implementation::new("test-agent", "0"), + )) + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_with(agent_transport, async move |connection| { + ready_rx.await.unwrap(); + let sent = connection.send_request(request("session")); + let update = incoming.recv().await.unwrap(); + let Update::Elicitation(pending) = update.update else { + panic!("expected form") + }; + pending + .reply + .send(wire::CreateElicitationResponse::new( + wire::ElicitationAction::Decline, + )) + .unwrap(); + assert!(matches!( + sent.block_task().await?.action, + wire::ElicitationAction::Decline + )); + + let sent = connection.send_request(request("session")); + let update = incoming.recv().await.unwrap(); + let Update::Elicitation(pending) = update.update else { + panic!("expected form") + }; + sent.cancel()?; + assert!(matches!( + sent.block_task().await?.action, + wire::ElicitationAction::Cancel + )); + assert!(pending.reply.is_closed()); + + let sent = connection.send_request(request("session")); + let update = incoming.recv().await.unwrap(); + let Update::Elicitation(pending) = update.update else { + panic!("expected form") + }; + transition_route(&route, "replacement".into()); + pending + .reply + .send(wire::CreateElicitationResponse::new( + wire::ElicitationAction::Decline, + )) + .unwrap(); + assert!(matches!( + sent.block_task().await?.action, + wire::ElicitationAction::Cancel + )); + Ok::<_, agent_client_protocol::Error>(()) + }) + .await + .unwrap(); + let _ = done_tx.send(()); + client_task.await.unwrap().unwrap(); + }; + tokio::time::timeout(std::time::Duration::from_secs(10), exercise) + .await + .expect("ACP roundtrip stalled"); + } + + #[tokio::test] + async fn ask_user_tool_roundtrips_through_tui_multiselect_custom_and_review() { + use agent_client_protocol::{Channel, schema::ProtocolVersion}; + use agentkit_core::{CancellationController, MetadataMap, SessionId, ToolOutput, TurnId}; + use agentkit_tools_core::{AllowAllPermissions, OwnedToolContext, Tool, ToolRequest}; + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + use serde_json::json; + + fn screen(app: &mut App) -> String { + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 30)).unwrap(); + terminal + .draw(|frame| app.elicitations.front_mut().unwrap().form.render(frame)) + .unwrap(); + terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect() + } + fn press(app: &mut App, key: KeyCode) { + assert!(matches!( + app.handle_key(KeyEvent::new(key, KeyModifiers::NONE)), + crate::tui::app::Action::Redraw + )); + } + + let exercise = async { + let (updates, mut incoming) = mpsc::unbounded_channel(); + let ingress = Arc::new(Ingress::new( + updates, + Arc::new(Mutex::new(ActiveSessionRoute { + id: "session".into(), + generation: 1, + })), + )); + let origin = ingress.origin.clone(); + let client = agent_client_protocol::Client + .v2() + .on_receive_request( + async move |request: RawRequest, responder, _cx| { + ingress.dispatch(request, responder) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_notification( + async move |_notification: wire::UpdateSessionNotification, _cx| Ok(()), + agent_client_protocol::on_receive_notification!(), + ); + let (client_transport, agent_transport) = Channel::duplex(); + let (ready_tx, ready_rx) = oneshot::channel(); + let (done_tx, done_rx) = oneshot::channel(); + let client_task = tokio::spawn(async move { + client + .connect_with(client_transport, async move |connection| { + let initialized = connection + .send_request( + wire::InitializeRequest::new( + ProtocolVersion::V2, + wire::Implementation::new("test-tui", "0"), + ) + .capabilities( + crate::tui::client_capabilities( + &crate::tools::mcp::CredentialStorage::Memory, + ), + ), + ) + .block_task() + .await?; + origin.set(initialized.info.name).unwrap(); + let _ = ready_tx.send(()); + let _ = done_rx.await; + Ok::<_, agent_client_protocol::Error>(()) + }) + .await + }); + agent_client_protocol::Agent.v2() + .on_receive_request(async move |request: wire::InitializeRequest, responder, _cx| { + assert!(request.capabilities.elicitation.unwrap().form.is_some()); + responder.respond(wire::InitializeResponse::new(ProtocolVersion::V2, wire::Implementation::new("question-agent", "0"))) + }, agent_client_protocol::on_receive_request!()) + .connect_with(agent_transport, async move |connection| { + ready_rx.await.unwrap(); + let tool = crate::tools::ask_user::AskUserTool::new(connection, wire::SessionId::new("session")); + let controller = CancellationController::new(); + let context = OwnedToolContext { + session_id: SessionId::new("session"), turn_id: TurnId::new("turn"), metadata: MetadataMap::new(), + permissions: Arc::new(AllowAllPermissions), resources: Arc::new(()), cancellation: Some(controller.handle().checkpoint()), + execution_scope: None, approved_request: None, + }; + let arguments = json!({"message":"Choose the implementation.","questions":[ + {"id":"scope","title":"Scope","prompt":"Which parts?","multiple":true,"allow_custom":true,"options":[{"id":"tui","label":"Terminal UI"},{"id":"runtime","label":"Agent runtime"}]}, + {"id":"strategy","title":"Strategy","prompt":"How should we proceed?","options":[{"id":"small","label":"Small patch"},{"id":"full","label":"Full implementation"}]} + ]}); + // Deliberately omit both optional flags on the second question. + // The tool must default to single selection with no custom field. + for include_selections in [true, false] { + let operation = async { + tool.invoke(ToolRequest::new("call", "ask_user", arguments.clone(), "session", "turn"), &mut context.borrowed()).await.unwrap() + }; + let interaction = async { + let queued = incoming.recv().await.unwrap(); + let mut app = app(); + app.apply(queued.update); + let visible = screen(&mut app); + assert!(visible.contains("question-agent")); + assert!(visible.contains("Question 1/3")); + assert!(visible.contains("Terminal UI")); + if include_selections { + for key in [KeyCode::Char(' '), KeyCode::Down, KeyCode::Char(' ')] { press(&mut app, key); } + } + press(&mut app, KeyCode::Enter); + assert!(screen(&mut app).contains("custom answer")); + if include_selections { app.paste("Also docs, please"); } + press(&mut app, KeyCode::Enter); + let visible = screen(&mut app); + assert!(visible.contains("Question 3/3")); + assert!(visible.contains("Strategy")); + // Selecting a second option replaces the first when multiple + // is omitted, rather than returning two single-choice values. + for key in [KeyCode::Down, KeyCode::Char(' '), KeyCode::Up, KeyCode::Char(' '), KeyCode::Enter] { press(&mut app, key); } + if !include_selections { + // The tool's generated metadata requires a choice or + // non-blank custom text. Both empty and whitespace-only + // submissions must stay open and focus the custom field. + assert!(screen(&mut app).contains("Review answers")); + press(&mut app, KeyCode::Enter); + let invalid = screen(&mut app); + assert!(invalid.contains("custom answer")); + assert!(invalid.contains("non-blank")); + assert_eq!(app.elicitations.len(), 1); + app.paste(" "); + press(&mut app, KeyCode::Enter); + press(&mut app, KeyCode::Enter); + assert!(screen(&mut app).contains("Review answers")); + press(&mut app, KeyCode::Enter); + assert!(screen(&mut app).contains("non-blank")); + assert_eq!(app.elicitations.len(), 1); + press(&mut app, KeyCode::Delete); + app.paste("Also docs, please"); + press(&mut app, KeyCode::Enter); + press(&mut app, KeyCode::Enter); + } + let review = screen(&mut app); + assert!(review.contains("Review answers")); + if include_selections { assert!(review.contains("Terminal UI, Agent runtime")); } + assert!(review.contains("Also docs, please")); + assert!(review.contains("Small patch")); + assert_eq!(app.elicitations.len(), 1); + press(&mut app, KeyCode::Enter); + assert!(app.elicitations.is_empty()); + }; + let (result, ()) = tokio::join!(operation, interaction); + assert!(!result.result.is_error); + assert_eq!(result.result.output, ToolOutput::structured(json!({"action":"accept","answers":{ + "scope":{"selected":if include_selections {vec!["tui","runtime"]} else {vec![]},"custom":"Also docs, please"}, + "strategy":{"selected":["small"]} + }}))); + } + Ok::<_, agent_client_protocol::Error>(()) + }).await.unwrap(); + let _ = done_tx.send(()); + client_task.await.unwrap().unwrap(); + }; + tokio::time::timeout(std::time::Duration::from_secs(10), exercise) + .await + .expect("tool/form roundtrip stalled"); + } + + #[test] + fn elicitation_capability_is_form_only_for_all_credential_modes() { + use crate::tools::mcp::CredentialStorage; + for credentials in [CredentialStorage::Memory, CredentialStorage::Keychain] { + let capabilities = crate::tui::client_capabilities(&credentials); + let elicitation = capabilities.elicitation.unwrap(); + assert!(elicitation.form.is_some()); + assert!(elicitation.url.is_none()); + } + } + + #[test] + fn elicitation_consumes_input_without_editing_the_composer() { + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + let ingress = ingress(); + let mut app = app(); + let (queued, mut answer) = ingress.prepare(request("session")).unwrap(); + app.apply(queued.update); + app.handle_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + crate::tui::handle_paste(&mut app, "some/path.txt"); + assert_eq!(app.editor.text(), ""); + assert!(app.attachments.is_empty()); + app.handle_key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + assert!(matches!( + answer.try_recv().unwrap().action, + wire::ElicitationAction::Cancel + )); + assert!(app.elicitations.is_empty()); + } + + #[test] + fn elicitation_idle_cancelled_snapshot_does_not_cancel_user_request() { + let ingress = ingress(); + let mut app = app(); + let (queued, mut receiver) = ingress.prepare(request("session")).unwrap(); + app.apply(queued.update); + app.apply(Update::State(wire::StateUpdate::Idle( + wire::IdleStateUpdate::new().stop_reason(wire::StopReason::Cancelled), + ))); + assert_eq!(app.elicitations.len(), 1); + assert!(matches!( + receiver.try_recv(), + Err(oneshot::error::TryRecvError::Empty) + )); + } + + #[test] + fn elicitation_raw_schema_constraints_are_not_silently_discarded() { + let ingress = ingress(); + let mut input = request("session"); + input.0["requestedSchema"]["allOf"] = serde_json::json!([]); + assert!(ingress.prepare(input).is_err()); + let mut input = request("session"); + input.0["requestedSchema"]["type"] = serde_json::json!("array"); + assert!(ingress.prepare(input).is_err()); + let mut input = request("session"); + input.0["requestedSchema"]["properties"]["scope"]["not"] = + serde_json::json!({"const":"small"}); + assert!(ingress.prepare(input).is_err()); + } + + #[test] + fn elicitation_cancelled_modal_consumes_key_before_exposing_composer() { + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + let ingress = ingress(); + let mut app = app(); + app.editor.insert_str("unsent draft"); + let (queued, receiver) = ingress.prepare(request("session")).unwrap(); + app.apply(queued.update); + assert!(app.needs_redraw_tick()); + drop(receiver); + assert!(matches!( + app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + crate::tui::app::Action::Redraw + )); + assert_eq!(app.editor.text(), "unsent draft"); + assert!(app.elicitations.is_empty()); + } + + #[test] + fn elicitation_poisoned_route_fails_closed() { + let ingress = ingress(); + let route = ingress.route.clone(); + let _ = std::thread::spawn(move || { + let _guard = route.lock().unwrap(); + panic!("route writer failed"); + }) + .join(); + assert!(ingress.prepare(request("session")).is_err()); + } +} diff --git a/src/tui/mod.rs b/src/tui/mod.rs index d7f0f6bf..e0cf5620 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -11,6 +11,8 @@ mod attachment; mod background; mod command; mod editor; +mod elicitation; +mod elicitation_requests; mod hyperlinks; mod image; mod input; @@ -885,7 +887,9 @@ fn credential_storage_for_launch( } fn client_capabilities(credential_storage: &CredentialStorage) -> wire::ClientCapabilities { - let capabilities = wire::ClientCapabilities::new(); + let capabilities = wire::ClientCapabilities::new().elicitation( + wire::ElicitationCapabilities::new().form(wire::ElicitationFormCapabilities::new()), + ); if credential_storage.is_persistent() { capabilities .auth(wire::AuthCapabilities::new().terminal(wire::TerminalAuthCapabilities::new())) @@ -1645,6 +1649,11 @@ async fn run_client( Ok::<_, std::io::Error>(()) }); + let questions = Arc::new(elicitation_requests::Ingress::new( + updates_tx.clone(), + Arc::clone(&active_persisted_id), + )); + let question_origin = questions.origin.clone(); let notifications = updates_tx.clone(); let cleanup_root = root.clone(); let transition_session = Arc::clone(&active_persisted_id); @@ -1658,6 +1667,12 @@ async fn run_client( let resume_session_id = resume_session_id.clone(); agent_client_protocol::Client .v2() + .on_receive_request( + async move |request: elicitation_requests::RawRequest, responder, _cx| { + questions.dispatch(request, responder) + }, + agent_client_protocol::on_receive_request!(), + ) .on_receive_notification( async move |notification: UpdateSessionNotification, _cx| { if is_remote { @@ -1724,6 +1739,7 @@ async fn run_client( std::io::Error::other("the agent did not negotiate ACP v2"), )); } + let _ = question_origin.set(initialized.info.name.clone()); let auth_methods = if is_remote { Vec::new() } else { @@ -3484,7 +3500,8 @@ fn clipboard_paste_key(key: KeyEvent) -> bool { } fn paste_blocked(app: &App) -> bool { - app.model_switch.is_some() + !app.elicitations.is_empty() + || app.model_switch.is_some() || app.model_dialog.is_some() || app.effort_dialog.is_some() || (app.session_dialog.is_some() && !app.session_rename_active()) @@ -3552,7 +3569,7 @@ fn apply_clipboard_completion( } fn handle_paste(app: &mut App, text: &str) -> bool { - if app.child_focus.is_some() { + if !app.elicitations.is_empty() || app.child_focus.is_some() { app.paste(text); return true; } @@ -3786,6 +3803,7 @@ fn draw_frame( ui::draw(frame, app, images); // Child views render no overlays, including command completions for // literal slash drafts. Only the root view uses those controls. + let form_obscures = !app.elicitations.is_empty(); let (app, obscured) = app .child_focus .as_ref() @@ -3798,7 +3816,7 @@ fn draw_frame( rows: app.row_links.clone(), left: app.transcript_left, top: app.transcript_top, - obscured, + obscured: obscured || form_obscures, } }) } diff --git a/src/tui/ui.rs b/src/tui/ui.rs index e2e5f21f..ec3c9b4e 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -35,7 +35,8 @@ const AGENTS_WIDTH: u16 = 46; const MAX_PROMPT_ROWS: usize = 10; pub(super) fn native_links_obscured(app: &App) -> bool { - app.model_switch.is_some() + !app.elicitations.is_empty() + || app.model_switch.is_some() || app.file_picker.is_some() || app.session_dialog.is_some() || app.model_dialog.is_some() @@ -55,6 +56,12 @@ type TaggedTranscriptLine = (LinkedLine, TranscriptTag); pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { images.poll(); + app.elicitations + .retain(|pending| !pending.reply.is_closed()); + if let Some(pending) = app.elicitations.front_mut() { + pending.form.render(frame); + return; + } app.child_back_area = Rect::default(); if let Some(id) = app.child_focus.clone() && let Some(mut child) = app.child_views.remove(&id) From 60f0120e4b27bb93e03501afc4b095a8dadbe578 Mon Sep 17 00:00:00 2001 From: daniel Date: Thu, 22 Oct 2026 14:22:03 +0100 Subject: [PATCH 2/6] fix(tui): keep question forms inline with composer editing --- docs/user/tui-and-sessions.md | 6 +- src/tui/app.rs | 26 ++- src/tui/editor.rs | 2 +- src/tui/elicitation.rs | 286 +++++++++++++++++++++++++++----- src/tui/elicitation_requests.rs | 151 ++++++++++++++++- src/tui/mod.rs | 12 +- src/tui/ui.rs | 34 +++- 7 files changed, 454 insertions(+), 63 deletions(-) diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index 51a1c9a4..f05b21e2 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -135,13 +135,13 @@ At an idle, non-empty editor, `Ctrl+C` clears the prompt instead of unexpectedly ## Answer agent questions -When an agent needs a decision, Kit can show a form with single-choice or multiple-choice options and optional custom-answer fields. The form identifies the requesting agent and explains what it needs. Answers stay local to the form until you submit the review page. +When an agent needs a decision, Kit can show a form with single-choice or multiple-choice options and optional custom-answer fields. The form appears in a bounded panel below the conversation, keeping the surrounding context visible. Answers stay local to the form until you submit the review page. - Use `Up` / `Down` to highlight an option and `Space` to select it. For multiple-choice fields, `Space` toggles each option independently. -- Type into text or numeric fields. Use `Left` / `Right` to choose false / true for a boolean field, or `Space` to toggle it. +- Text and numeric fields use the composer editor: move the cursor with arrow keys, use `Alt+Left` / `Alt+Right` to move by word, and `Alt+Backspace` or `Ctrl+W` to delete the previous word. `Ctrl+U` / `Ctrl+K` delete to the start / end of the line. `Shift+Enter` inserts a newline; `Ctrl+V` / `Shift+Insert` and bracketed paste insert text at the cursor. Clipboard images are not accepted in form fields. Use `Left` / `Right` to choose false / true for a boolean field, or `Space` to toggle it. - Press `Enter` to advance and `Shift+Tab` to go back without losing answers. Optional custom answers appear on a separate page. - On the review page, press `Enter` to submit all answers together, or `Shift+Tab` to revise them. -- Press `Delete` to clear the current field and `PageUp` / `PageDown` to scroll. +- Press `Delete` to clear a choice or boolean field; in text fields it deletes forward from the cursor. Use `PageUp` / `PageDown` to scroll the question or review. - Press `Esc` to dismiss the request, or `Ctrl+D` to explicitly decline. Neither action submits selections. Do not enter passwords, API keys, or other credentials in these forms. Kit supports session-scoped ACP form elicitation, not URL elicitation. Unsupported form constraints are rejected rather than silently ignored. If your ACP client does not support forms, ask and answer in ordinary chat instead. diff --git a/src/tui/app.rs b/src/tui/app.rs index ab7fe0a9..26402f52 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -429,10 +429,17 @@ struct SteerEdit { #[derive(Clone, Debug, PartialEq, Eq)] pub(super) enum ClipboardRoute { + Elicitation { + epoch: u64, + target: super::elicitation::ClipboardTarget, + }, Blocked(u64), Composer(u64), Rename(u64), - Steer { id: String, token: u64 }, + Steer { + id: String, + token: u64, + }, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -3487,8 +3494,19 @@ impl App { } pub(super) fn clipboard_route(&self) -> ClipboardRoute { - if !self.elicitations.is_empty() - || self.child_focus.is_some() + if let Some(pending) = self.elicitations.front() { + return if pending.reply.is_closed() { + ClipboardRoute::Blocked(self.clipboard_route_epoch) + } else if let Some(target) = pending.form.clipboard_target() { + ClipboardRoute::Elicitation { + epoch: self.clipboard_route_epoch, + target, + } + } else { + ClipboardRoute::Blocked(self.clipboard_route_epoch) + }; + } + if self.child_focus.is_some() || self.model_switch.is_some() || self.model_dialog.is_some() || self.effort_dialog.is_some() @@ -3516,6 +3534,7 @@ impl App { pub fn paste(&mut self, text: &str) { if let Some(pending) = self.elicitations.front_mut() { + self.clipboard_route_epoch = self.clipboard_route_epoch.wrapping_add(1); pending.form.paste(text); return; } @@ -4244,6 +4263,7 @@ impl App { return Action::Redraw; } if let Some(pending) = self.elicitations.front_mut() { + self.clipboard_route_epoch = self.clipboard_route_epoch.wrapping_add(1); if let Some(response) = pending.form.key(key) && let Some(pending) = self.elicitations.pop_front() { diff --git a/src/tui/editor.rs b/src/tui/editor.rs index 50ca4046..3cec6f5c 100644 --- a/src/tui/editor.rs +++ b/src/tui/editor.rs @@ -7,7 +7,7 @@ use unicode_width::UnicodeWidthStr; -#[derive(Default)] +#[derive(Debug, Default)] pub struct Editor { text: String, cursor: usize, diff --git a/src/tui/elicitation.rs b/src/tui/elicitation.rs index f64f1d46..9ddea5f6 100644 --- a/src/tui/elicitation.rs +++ b/src/tui/elicitation.rs @@ -3,11 +3,14 @@ use agent_client_protocol::schema::v2 as wire; use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; use ratatui::{ Frame, - layout::{Constraint, Layout}, + layout::{Constraint, Layout, Rect}, widgets::{Block, Borders, Clear, Paragraph, Wrap}, }; use serde_json::{Map, Value}; +use super::editor::Editor; +use std::sync::Arc; + const MAX_TEXT: usize = 4096; #[derive(Debug)] @@ -18,7 +21,24 @@ pub(super) struct Form { page: usize, scroll: u16, error: Option, + editor_width: usize, + clipboard_identity: Arc<()>, +} + +// The marker is private immutable ownership, not shared mutable state. A +// clipboard operation retains it, so a replacement form can never reuse its +// identity. Page and app route epoch fence navigation and intervening edits. +#[derive(Clone, Debug)] +pub(super) struct ClipboardTarget { + identity: Arc<()>, + page: usize, } +impl PartialEq for ClipboardTarget { + fn eq(&self, other: &Self) -> bool { + self.page == other.page && Arc::ptr_eq(&self.identity, &other.identity) + } +} +impl Eq for ClipboardTarget {} #[derive(Debug)] struct AnswerRequirement { @@ -35,7 +55,7 @@ struct Field { options: Vec, cursor: usize, value: Value, - text: String, + editor: Editor, } #[derive(Debug)] @@ -91,6 +111,74 @@ fn bounded(v: &Value) -> Result<(), String> { } impl Field { + fn insert(&mut self, text: &str) { + // Normalize paste exactly like the composer before applying the form limit. + let mut normalized = Editor::default(); + normalized.insert_str(text); + let remaining = MAX_TEXT.saturating_sub(self.editor.text().chars().count()); + let text: String = normalized.text().chars().take(remaining).collect(); + if !text.is_empty() { + self.editor.insert_str(&text); + self.sync_text(); + } + } + + fn sync_text(&mut self) { + self.value = if self.editor.text().is_empty() { + Value::Null + } else if self.kind == "string" { + Value::String(self.editor.text().to_owned()) + } else { + self.value.clone() + }; + } + + fn edit(&mut self, key: KeyEvent, width: usize) -> bool { + let control = key.modifiers.contains(KeyModifiers::CONTROL); + let alt = key.modifiers.contains(KeyModifiers::ALT); + let command = key.modifiers.contains(KeyModifiers::SUPER); + let shift = key.modifiers.contains(KeyModifiers::SHIFT); + let previous = self.editor.text().to_owned(); + match key.code { + KeyCode::Enter if shift || alt || control => self.insert("\n"), + KeyCode::Char('j') if control => self.insert("\n"), + KeyCode::Tab => self.insert(" "), + KeyCode::Backspace if command => self.editor.delete_to_line_start(), + KeyCode::Backspace if alt || control => self.editor.delete_word_back(), + KeyCode::Backspace => self.editor.backspace(), + KeyCode::Delete if alt || control => self.editor.delete_word_forward(), + KeyCode::Delete => self.editor.delete_forward(), + KeyCode::Char('w') if control => self.editor.delete_word_back(), + KeyCode::Char('u') if control => self.editor.delete_to_line_start(), + KeyCode::Char('k') if control => self.editor.delete_to_line_end(), + KeyCode::Char('d') if alt => self.editor.delete_word_forward(), + KeyCode::Char('b') if alt => self.editor.move_word_left(), + KeyCode::Char('f') if alt => self.editor.move_word_right(), + KeyCode::Char('a') if control => self.editor.move_line_start(), + KeyCode::Char('e') if control => self.editor.move_line_end(), + KeyCode::Left if command || control => self.editor.move_line_start(), + KeyCode::Right if command || control => self.editor.move_line_end(), + KeyCode::Left if alt => self.editor.move_word_left(), + KeyCode::Right if alt => self.editor.move_word_right(), + KeyCode::Left => self.editor.move_left(), + KeyCode::Right => self.editor.move_right(), + KeyCode::Home => self.editor.move_line_start(), + KeyCode::End => self.editor.move_line_end(), + KeyCode::Up => { + self.editor.move_row_up(width); + } + KeyCode::Down => { + self.editor.move_row_down(width); + } + KeyCode::Char(c) if !control && !command => self.insert(&c.to_string()), + _ => return false, + } + if previous != self.editor.text() { + self.sync_text(); + } + true + } + fn new(name: String, schema: Value, required: bool) -> Result { let kind = annotation(&schema, "type"); let mut allowed = vec!["type", "title", "description", "default", "_meta"]; @@ -236,7 +324,13 @@ impl Field { options, cursor, value, - text, + editor: { + let mut editor = Editor::default(); + for c in text.chars() { + editor.insert_char(c); + } + editor + }, }; if !field.value.is_null() { field.validate()?; @@ -249,19 +343,19 @@ impl Field { match self.kind.as_str() { "string" => { return Ok( - if self.text.is_empty() && !self.required && self.value.is_null() { + if self.editor.text().is_empty() && !self.required && self.value.is_null() { Value::Null } else { - Value::String(self.text.clone()) + Value::String(self.editor.text().to_owned()) }, ); } "number" | "integer" => { - if self.text.is_empty() { + if self.editor.text().is_empty() { return Ok(Value::Null); } - let value: Value = - serde_json::from_str(&self.text).map_err(|_| "Enter a valid number")?; + let value: Value = serde_json::from_str(self.editor.text()) + .map_err(|_| "Enter a valid number")?; if !value.is_number() || (self.kind == "integer" && value.as_i64().is_none()) { return Err("Enter a valid number of the requested type".into()); } @@ -418,10 +512,17 @@ fn answer_requirements(schema: &Value, fields: &[Field]) -> Result Self { - let origin: String = origin.chars().take(MAX_TEXT).collect(); - self.message = format!("Request from {origin}\n\n{}", self.message); - self + pub(super) fn clipboard_target(&self) -> Option { + self.fields + .get(self.page) + .filter(|field| { + field.options.is_empty() + && matches!(field.kind.as_str(), "string" | "number" | "integer") + }) + .map(|_| ClipboardTarget { + identity: Arc::clone(&self.clipboard_identity), + page: self.page, + }) } #[cfg(test)] @@ -518,6 +619,8 @@ impl Form { page: 0, scroll: 0, error: None, + editor_width: 40, + clipboard_identity: Arc::new(()), }) } @@ -526,13 +629,7 @@ impl Form { && field.options.is_empty() && matches!(field.kind.as_str(), "string" | "number" | "integer") { - let remaining = MAX_TEXT.saturating_sub(field.text.chars().count()); - field - .text - .extend(text.chars().filter(|c| !c.is_control()).take(remaining)); - if field.kind == "string" { - field.value = Value::String(field.text.clone()); - } + field.insert(text); self.error = None; } } @@ -553,6 +650,14 @@ impl Form { wire::ElicitationAction::Decline, )); } + if let Some(field) = self.fields.get_mut(self.page) + && field.options.is_empty() + && matches!(field.kind.as_str(), "string" | "number" | "integer") + && field.edit(key, self.editor_width) + { + self.error = None; + return None; + } match key.code { KeyCode::BackTab => { self.page = self.page.saturating_sub(1); @@ -636,17 +741,7 @@ impl Form { } KeyCode::Delete => { field.value = Value::Null; - field.text.clear(); - } - KeyCode::Backspace => { - field.text.pop(); - } - KeyCode::Char(c) - if !key - .modifiers - .intersects(KeyModifiers::CONTROL | KeyModifiers::ALT) => - { - self.paste(&c.to_string()) + field.editor.clear(); } _ => {} } @@ -656,14 +751,18 @@ impl Form { None } + #[cfg(test)] pub(super) fn render(&mut self, frame: &mut Frame) { - let area = frame.area(); + self.render_in(frame, frame.area()); + } + + pub(super) fn render_in(&mut self, frame: &mut Frame, area: Rect) { frame.render_widget(Clear, area); let block = Block::default() - .title("Agent request") .borders(Borders::ALL) .border_style(super::theme::accent()); let inner = block.inner(area); + self.editor_width = usize::from(inner.width).max(1); frame.render_widget(block, area); let parts = Layout::vertical([Constraint::Min(1), Constraint::Length(3)]).split(inner); let mut text = format!("{}\n\n", self.message); @@ -673,11 +772,7 @@ impl Form { "Question {}/{} — {} {}\n{}\n", self.page + 1, self.fields.len(), - if title.is_empty() { - &field.name - } else { - &title - }, + if title.is_empty() { "Answer" } else { &title }, if field.required { "(required)" } else { @@ -727,6 +822,22 @@ impl Form { .collect::>() .join("\n"); frame.render_widget(Paragraph::new(display(&lines)), regions[1]); + } else if field.kind != "boolean" { + let regions = + Layout::vertical([Constraint::Min(1), Constraint::Length(3)]).split(parts[0]); + render_text(frame, regions[0], text, &mut self.scroll); + let (lines, (row, column)) = field.editor.wrapped(self.editor_width); + let first = row.saturating_sub(usize::from(regions[1].height).saturating_sub(1)); + frame.render_widget( + Paragraph::new(display(&lines.join("\n"))).scroll((first as u16, 0)), + regions[1], + ); + if regions[1].height > 0 && regions[1].width > 0 { + frame.set_cursor_position(( + regions[1].x + (column as u16).min(regions[1].width - 1), + regions[1].y + row.saturating_sub(first) as u16, + )); + } } else { text.push_str(&format!( "\nAnswer: {}", @@ -737,14 +848,14 @@ impl Form { None => "Not answered (← No / → Yes)", } } else { - &field.text + field.editor.text() } )); render_text(frame, parts[0], text, &mut self.scroll); } } else { text.push_str("Review answers — BackTab to modify; Enter to submit\n\n"); - for field in &self.fields { + for (index, field) in self.fields.iter().enumerate() { let value = field.answer().unwrap_or(Value::Null); let display = if value.is_null() { "(omitted)".into() @@ -759,11 +870,17 @@ impl Form { } else { value.to_string() }; - text.push_str(&format!("{}: {}\n", field.name, display)); + let title = annotation(&field.schema, "title"); + let label = if title.is_empty() { + format!("Answer {}", index + 1) + } else { + title + }; + text.push_str(&format!("{label}: {display}\n")); } render_text(frame, parts[0], text, &mut self.scroll); } - frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • BackTab: back • Space: select • Delete: clear\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); + frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • Shift+Tab: back • Space: select\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); } } @@ -810,6 +927,87 @@ mod tests { fn press(form: &mut Form, code: KeyCode) -> Option { form.key(KeyEvent::new(code, KeyModifiers::NONE)) } + #[test] + fn navigating_empty_and_whitespace_defaults_preserves_answers() { + for value in ["", " "] { + let mut form = form( + json!({"input":{"type":"string","default":value}}), + json!([]), + ) + .unwrap(); + for key in [KeyCode::Left, KeyCode::Right, KeyCode::Home, KeyCode::End] { + press(&mut form, key); + assert_eq!(form.fields[0].answer().unwrap(), json!(value)); + } + } + let mut form = form(json!({"input":{"type":"string"}}), json!([])).unwrap(); + form.paste(" "); + assert_eq!(form.fields[0].answer().unwrap(), json!(" ")); + } + + #[test] + fn text_fields_match_composer_editing_keys_and_cursor() { + let keys = [ + (KeyCode::Backspace, KeyModifiers::SHIFT), + (KeyCode::Backspace, KeyModifiers::ALT), + (KeyCode::Backspace, KeyModifiers::CONTROL), + (KeyCode::Backspace, KeyModifiers::SUPER), + (KeyCode::Delete, KeyModifiers::NONE), + (KeyCode::Delete, KeyModifiers::ALT), + (KeyCode::Left, KeyModifiers::NONE), + (KeyCode::Right, KeyModifiers::NONE), + (KeyCode::Left, KeyModifiers::ALT), + (KeyCode::Right, KeyModifiers::ALT), + (KeyCode::Left, KeyModifiers::CONTROL), + (KeyCode::Right, KeyModifiers::SUPER), + (KeyCode::Home, KeyModifiers::NONE), + (KeyCode::End, KeyModifiers::NONE), + (KeyCode::Char('w'), KeyModifiers::CONTROL), + (KeyCode::Char('u'), KeyModifiers::CONTROL), + (KeyCode::Char('k'), KeyModifiers::CONTROL), + (KeyCode::Char('b'), KeyModifiers::ALT), + (KeyCode::Char('f'), KeyModifiers::ALT), + (KeyCode::Char('d'), KeyModifiers::ALT), + (KeyCode::Char('a'), KeyModifiers::CONTROL), + (KeyCode::Char('e'), KeyModifiers::CONTROL), + (KeyCode::Char('j'), KeyModifiers::CONTROL), + (KeyCode::Char('界'), KeyModifiers::NONE), + (KeyCode::Enter, KeyModifiers::SHIFT), + ]; + for (code, modifiers) in keys { + let mut form = form(json!({"input":{"type":"string"}}), json!([])).unwrap(); + let mut app = + crate::tui::app::App::new(".".into(), "test".into(), "test".into(), String::new()); + form.paste("first café\nlast word"); + app.editor.insert_str("first café\nlast word"); + form.fields[0].editor.move_left(); + app.editor.move_left(); + let key = KeyEvent::new(code, modifiers); + form.key(key); + app.handle_key(key); + assert_eq!(form.fields[0].editor.text(), app.editor.text(), "{key:?}"); + assert_eq!( + form.fields[0].editor.cursor(), + app.editor.cursor(), + "{key:?}" + ); + } + } + + #[test] + fn text_paste_inserts_at_cursor_and_normalizes_like_composer() { + let mut form = form(json!({"input":{"type":"string"}}), json!([])).unwrap(); + form.paste("ab"); + press(&mut form, KeyCode::Left); + form.paste("é\r\n\t界"); + assert_eq!(form.fields[0].editor.text(), "aé\n 界b"); + press(&mut form, KeyCode::Backspace); + assert_eq!(form.fields[0].editor.text(), "aé\n b"); + press(&mut form, KeyCode::Enter); + let result = serde_json::to_value(press(&mut form, KeyCode::Enter).unwrap()).unwrap(); + assert_eq!(result["content"]["input"], "aé\n b"); + } + #[test] fn review_is_required_and_edits_are_preserved() { let mut f = form( @@ -823,7 +1021,7 @@ mod tests { assert!(press(&mut f, KeyCode::Enter).is_none()); assert_eq!(f.page, 1); press(&mut f, KeyCode::BackTab); - assert_eq!(f.fields[0].text, "Ada"); + assert_eq!(f.fields[0].editor.text(), "Ada"); press(&mut f, KeyCode::Enter); let result = serde_json::to_value(press(&mut f, KeyCode::Enter).unwrap()).unwrap(); assert_eq!(result["content"]["name"], "Ada"); @@ -1038,8 +1236,8 @@ mod tests { fn bounds_optional_and_cancellation() { let mut f = form(json!({"x":{"type":"string"}}), json!([])).unwrap(); f.paste(&"é".repeat(5000)); - assert_eq!(f.fields[0].text.chars().count(), MAX_TEXT); - press(&mut f, KeyCode::Delete); + assert_eq!(f.fields[0].editor.text().chars().count(), MAX_TEXT); + f.key(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)); press(&mut f, KeyCode::Enter); assert_eq!( serde_json::to_value(press(&mut f, KeyCode::Enter).unwrap()).unwrap()["content"], diff --git a/src/tui/elicitation_requests.rs b/src/tui/elicitation_requests.rs index dd505217..8630d60b 100644 --- a/src/tui/elicitation_requests.rs +++ b/src/tui/elicitation_requests.rs @@ -79,7 +79,7 @@ impl Ingress { if route.id.is_empty() || route.id != session { return Err("elicitation does not belong to the active session".into()); } - let origin = self + let _origin = self .origin .get() .ok_or("agent initialization is incomplete")?; @@ -89,7 +89,7 @@ impl Ingress { .try_acquire_owned() .map_err(|_| "too many pending user requests")?; // Validate the original wire budget before adding local presentation text. - let form = Form::from_value(request.0)?.with_origin(origin); + let form = Form::from_value(request.0)?; let (reply, receiver) = oneshot::channel(); let pending = Pending { form, @@ -212,6 +212,148 @@ mod tests { assert!(ingress.prepare(request).is_err()); } + #[test] + fn elicitation_clipboard_shortcuts_are_text_only_and_fenced_to_field() { + use crate::tui::app::{Action, ClipboardRoute}; + use crate::tui::{ClipboardResult, apply_clipboard_completion, handle_inner}; + use crossterm::event::{Event, KeyCode, KeyEvent, KeyModifiers}; + let ingress = ingress(); + let mut raw = request("session"); + raw.0["requestedSchema"] = serde_json::json!({"type":"object","properties":{ + "first":{"type":"string"},"second":{"type":"string"} + }}); + let (queued, _receiver) = ingress.prepare(raw.clone()).unwrap(); + let mut app = app(); + app.editor.insert_str("parked draft"); + app.apply(queued.update); + for key in [ + KeyEvent::new(KeyCode::Char('v'), KeyModifiers::CONTROL), + KeyEvent::new(KeyCode::Insert, KeyModifiers::SHIFT), + ] { + let Action::ReadClipboard(route, _) = handle_inner(&mut app, Event::Key(key)) else { + panic!("expected clipboard request"); + }; + assert!(matches!(route, ClipboardRoute::Elicitation { .. })); + assert_eq!( + apply_clipboard_completion( + &mut app, + &ingress.route, + 1, + route, + ClipboardResult::Text("pasted ".into()) + ), + Some(true) + ); + } + let stale = app.clipboard_route(); + app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!( + apply_clipboard_completion( + &mut app, + &ingress.route, + 1, + stale, + ClipboardResult::Text("wrong field".into()) + ), + None + ); + let stale = app.clipboard_route(); + app.handle_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + assert_eq!( + apply_clipboard_completion( + &mut app, + &ingress.route, + 1, + stale, + ClipboardResult::Text("after edit".into()) + ), + None + ); + let old_form = app.clipboard_route(); + app.handle_key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + let (queued, _next_receiver) = ingress.prepare(raw).unwrap(); + app.apply(queued.update); + assert_eq!( + apply_clipboard_completion( + &mut app, + &ingress.route, + 1, + old_form, + ClipboardResult::Text("wrong form".into()) + ), + None + ); + assert_eq!(app.editor.text(), "parked draft"); + } + + #[test] + fn elicitation_panel_keeps_conversation_visible_without_generated_chrome() { + let ingress = ingress(); + let mut request = request("session"); + request.0["requestedSchema"] = serde_json::json!({"type":"object","properties":{ + "question_0":{"type":"string","title":"Scope","default":"Small patch"}, + "question_1":{"type":"string","default":"Notes"} + }}); + let (queued, _receiver) = ingress.prepare(request).unwrap(); + let mut app = app(); + app.blocks.push(crate::tui::app::Block::Agent( + "Conversation context remains visible".into(), + )); + app.editor.insert_str("parked draft"); + app.apply(queued.update); + let mut images = crate::tui::image::ImageRuntime::disabled(); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, 40)).unwrap(); + terminal + .draw(|frame| crate::tui::ui::draw(frame, &mut app, &mut images)) + .unwrap(); + let screen: String = terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect(); + assert!(screen.contains("Conversation context remains visible")); + assert!(screen.contains("Choose a scope")); + assert!(!screen.contains("parked draft")); + assert!(!screen.contains("Agent request")); + assert!(!screen.contains("Request from")); + assert!(!screen.contains("question_0")); + assert!(!crate::tui::ui::native_links_obscured(&app)); + assert!( + app.viewport >= 18, + "conversation must retain at least half the available height" + ); + for _ in 0..2 { + app.handle_key(crossterm::event::KeyEvent::new( + crossterm::event::KeyCode::Enter, + crossterm::event::KeyModifiers::NONE, + )); + } + terminal + .draw(|frame| crate::tui::ui::draw(frame, &mut app, &mut images)) + .unwrap(); + let screen: String = terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect(); + assert!(screen.contains("Scope: \"Small patch\"")); + assert!(screen.contains("Answer 2: \"Notes\"")); + assert!(!screen.contains("question_0")); + assert!(!screen.contains("question_1")); + assert!(screen.contains("Conversation context remains visible")); + for (width, height) in [(1, 1), (8, 3), (30, 10)] { + let mut tiny = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(width, height)).unwrap(); + tiny.draw(|frame| crate::tui::ui::draw(frame, &mut app, &mut images)) + .unwrap(); + } + } + #[test] fn elicitation_admission_is_bounded_until_queued_forms_are_dropped() { let ingress = ingress(); @@ -495,7 +637,8 @@ mod tests { let mut app = app(); app.apply(queued.update); let visible = screen(&mut app); - assert!(visible.contains("question-agent")); + assert!(!visible.contains("Request from")); + assert!(!visible.contains("Agent request")); assert!(visible.contains("Question 1/3")); assert!(visible.contains("Terminal UI")); if include_selections { @@ -528,7 +671,7 @@ mod tests { press(&mut app, KeyCode::Enter); assert!(screen(&mut app).contains("non-blank")); assert_eq!(app.elicitations.len(), 1); - press(&mut app, KeyCode::Delete); + app.handle_key(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)); app.paste("Also docs, please"); press(&mut app, KeyCode::Enter); press(&mut app, KeyCode::Enter); diff --git a/src/tui/mod.rs b/src/tui/mod.rs index e0cf5620..6a139342 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -3500,8 +3500,10 @@ fn clipboard_paste_key(key: KeyEvent) -> bool { } fn paste_blocked(app: &App) -> bool { - !app.elicitations.is_empty() - || app.model_switch.is_some() + if !app.elicitations.is_empty() { + return !matches!(app.clipboard_route(), ClipboardRoute::Elicitation { .. }); + } + app.model_switch.is_some() || app.model_dialog.is_some() || app.effort_dialog.is_some() || (app.session_dialog.is_some() && !app.session_rename_active()) @@ -3560,6 +3562,9 @@ fn apply_clipboard_completion( Some(match result { ClipboardResult::NoImage => true, ClipboardResult::Text(text) => handle_paste(app, &text), + ClipboardResult::Attachment(_) if matches!(route, ClipboardRoute::Elicitation { .. }) => { + false + } ClipboardResult::Attachment(attachment) => attach_pasted(app, vec![attachment]), ClipboardResult::Error(error) => { app.note(error); @@ -3803,7 +3808,6 @@ fn draw_frame( ui::draw(frame, app, images); // Child views render no overlays, including command completions for // literal slash drafts. Only the root view uses those controls. - let form_obscures = !app.elicitations.is_empty(); let (app, obscured) = app .child_focus .as_ref() @@ -3816,7 +3820,7 @@ fn draw_frame( rows: app.row_links.clone(), left: app.transcript_left, top: app.transcript_top, - obscured: obscured || form_obscures, + obscured, } }) } diff --git a/src/tui/ui.rs b/src/tui/ui.rs index ec3c9b4e..9efff6c1 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -35,8 +35,11 @@ const AGENTS_WIDTH: u16 = 46; const MAX_PROMPT_ROWS: usize = 10; pub(super) fn native_links_obscured(app: &App) -> bool { - !app.elicitations.is_empty() - || app.model_switch.is_some() + if !app.elicitations.is_empty() { + // Forms occupy their own area; no transcript links are covered. + return false; + } + app.model_switch.is_some() || app.file_picker.is_some() || app.session_dialog.is_some() || app.model_dialog.is_some() @@ -58,8 +61,31 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { images.poll(); app.elicitations .retain(|pending| !pending.reply.is_closed()); - if let Some(pending) = app.elicitations.front_mut() { - pending.form.render(frame); + if !app.elicitations.is_empty() { + // Keep the conversation visible above a bounded question panel. The + // normal composer stays parked while the form owns keyboard input. + let area = frame.area(); + let panel_rows = (area.height / 2).min(18); + let [header, body, panel] = Layout::vertical([ + Constraint::Length(u16::from(area.height > 2)), + Constraint::Min(0), + Constraint::Length(panel_rows), + ]) + .areas(area); + app.child_back_area = Rect::default(); + app.session_area = draw_header(frame, app, header).unwrap_or_default(); + if let Some(child) = app + .child_focus + .as_ref() + .and_then(|id| app.child_views.get_mut(id)) + { + draw_body(frame, &mut child.app, images, body); + } else { + draw_body(frame, app, images, body); + } + if let Some(pending) = app.elicitations.front_mut() { + pending.form.render_in(frame, panel); + } return; } app.child_back_area = Rect::default(); From 4dc51d4827cd8f2e8d2c23714bc88edf760fcb07 Mon Sep 17 00:00:00 2001 From: daniel Date: Thu, 22 Oct 2026 15:12:50 +0100 Subject: [PATCH 3/6] fix(tui): render custom answers inline with choices --- docs/user/tui-and-sessions.md | 3 +- src/tui/elicitation.rs | 268 +++++++++++++++++++++++++------- src/tui/elicitation_requests.rs | 60 +++---- 3 files changed, 244 insertions(+), 87 deletions(-) diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index f05b21e2..25c43164 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -139,7 +139,8 @@ When an agent needs a decision, Kit can show a form with single-choice or multip - Use `Up` / `Down` to highlight an option and `Space` to select it. For multiple-choice fields, `Space` toggles each option independently. - Text and numeric fields use the composer editor: move the cursor with arrow keys, use `Alt+Left` / `Alt+Right` to move by word, and `Alt+Backspace` or `Ctrl+W` to delete the previous word. `Ctrl+U` / `Ctrl+K` delete to the start / end of the line. `Shift+Enter` inserts a newline; `Ctrl+V` / `Shift+Insert` and bracketed paste insert text at the cursor. Clipboard images are not accepted in form fields. Use `Left` / `Right` to choose false / true for a boolean field, or `Space` to toggle it. -- Press `Enter` to advance and `Shift+Tab` to go back without losing answers. Optional custom answers appear on a separate page. +- When a question offers **Other**, its text input appears below the choices on the same page. Press `Tab` to switch focus between the choices and Other. You can keep selected options and an Other answer together, or enter only an Other answer. +- Press `Enter` to advance and `Shift+Tab` to go back without losing answers. Required questions must have a selection or non-blank Other text before you can advance. Unrelated form fields remain on separate pages. - On the review page, press `Enter` to submit all answers together, or `Shift+Tab` to revise them. - Press `Delete` to clear a choice or boolean field; in text fields it deletes forward from the cursor. Use `PageUp` / `PageDown` to scroll the question or review. - Press `Esc` to dismiss the request, or `Ctrl+D` to explicitly decline. Neither action submits selections. diff --git a/src/tui/elicitation.rs b/src/tui/elicitation.rs index 9ddea5f6..1bf69359 100644 --- a/src/tui/elicitation.rs +++ b/src/tui/elicitation.rs @@ -18,6 +18,8 @@ pub(super) struct Form { message: String, fields: Vec, require_answer: Vec, + pages: Vec, + other_focus: bool, page: usize, scroll: u16, error: Option, @@ -27,15 +29,15 @@ pub(super) struct Form { // The marker is private immutable ownership, not shared mutable state. A // clipboard operation retains it, so a replacement form can never reuse its -// identity. Page and app route epoch fence navigation and intervening edits. +// identity. Focused field and app route epoch fence navigation and intervening edits. #[derive(Clone, Debug)] pub(super) struct ClipboardTarget { identity: Arc<()>, - page: usize, + field: usize, } impl PartialEq for ClipboardTarget { fn eq(&self, other: &Self) -> bool { - self.page == other.page && Arc::ptr_eq(&self.identity, &other.identity) + self.field == other.field && Arc::ptr_eq(&self.identity, &other.identity) } } impl Eq for ClipboardTarget {} @@ -512,16 +514,52 @@ fn answer_requirements(schema: &Value, fields: &[Field]) -> Result Option { + self.require_answer + .iter() + .find(|r| r.selection == selection) + .map(|r| r.custom) + } + fn focused_field(&self) -> Option { + let selection = *self.pages.get(self.page)?; + Some(if self.other_focus { + self.custom_field(selection).unwrap_or(selection) + } else { + selection + }) + } + fn validate_page(&self, page: usize) -> Result<(), (usize, String)> { + let selection = self.pages[page]; + self.fields[selection] + .validate() + .map_err(|e| (selection, e))?; + if let Some(custom) = self.custom_field(selection) { + let text = self.fields[custom].validate().map_err(|e| (custom, e))?; + let selected = self.fields[selection] + .options + .iter() + .any(|o| self.fields[selection].selected(o)); + if !selected && !text.as_str().is_some_and(|s| !s.trim().is_empty()) { + return Err(( + custom, + "Select an option or enter a non-blank custom answer.".into(), + )); + } + } + Ok(()) + } pub(super) fn clipboard_target(&self) -> Option { - self.fields - .get(self.page) + self.focused_field() + .and_then(|index| self.fields.get(index)) .filter(|field| { field.options.is_empty() && matches!(field.kind.as_str(), "string" | "number" | "integer") }) - .map(|_| ClipboardTarget { - identity: Arc::clone(&self.clipboard_identity), - page: self.page, + .and_then(|_| { + Some(ClipboardTarget { + identity: Arc::clone(&self.clipboard_identity), + field: self.focused_field()?, + }) }) } @@ -604,7 +642,12 @@ impl Form { }) .collect::, _>>()?; let require_answer = answer_requirements(schema, &fields)?; + let pages = (0..fields.len()) + .filter(|index| !require_answer.iter().any(|r| r.custom == *index)) + .collect(); Ok(Self { + pages, + other_focus: false, message: [ annotation(&request, "message"), annotation(schema, "title"), @@ -625,7 +668,9 @@ impl Form { } pub(super) fn paste(&mut self, text: &str) { - if let Some(field) = self.fields.get_mut(self.page) + if let Some(field) = self + .focused_field() + .and_then(|index| self.fields.get_mut(index)) && field.options.is_empty() && matches!(field.kind.as_str(), "string" | "number" | "integer") { @@ -650,7 +695,19 @@ impl Form { wire::ElicitationAction::Decline, )); } - if let Some(field) = self.fields.get_mut(self.page) + if key.code == KeyCode::Tab + && self + .pages + .get(self.page) + .and_then(|i| self.custom_field(*i)) + .is_some() + { + self.other_focus = !self.other_focus; + return None; + } + if let Some(field) = self + .focused_field() + .and_then(|index| self.fields.get_mut(index)) && field.options.is_empty() && matches!(field.kind.as_str(), "string" | "number" | "integer") && field.edit(key, self.editor_width) @@ -660,6 +717,7 @@ impl Form { } match key.code { KeyCode::BackTab => { + self.other_focus = false; self.page = self.page.saturating_sub(1); self.scroll = 0; self.error = None; @@ -667,38 +725,28 @@ impl Form { KeyCode::PageDown => self.scroll = self.scroll.saturating_add(5), KeyCode::PageUp => self.scroll = self.scroll.saturating_sub(5), KeyCode::Enter => { - if self.page == self.fields.len() { - let mut content = Map::new(); - for (index, field) in self.fields.iter().enumerate() { - match field.validate() { - Ok(value) => { - if !value.is_null() { - content.insert(field.name.clone(), value); - } - } - Err(error) => { - self.page = index; - self.error = Some(error); - return None; - } - } + let reviewing = self.page == self.pages.len(); + let pages = if reviewing { + 0..self.pages.len() + } else { + self.page..self.page + 1 + }; + for page in pages { + if let Err((field, error)) = self.validate_page(page) { + self.page = page; + self.other_focus = self.custom_field(self.pages[page]) == Some(field); + self.scroll = 0; + self.error = Some(error); + return None; } - for requirement in &self.require_answer { - let selected = match content.get(&self.fields[requirement.selection].name) { - Some(Value::String(_)) => true, - Some(Value::Array(values)) => !values.is_empty(), - _ => false, - }; - let custom = content - .get(&self.fields[requirement.custom].name) - .and_then(Value::as_str) - .is_some_and(|value| !value.trim().is_empty()); - if !selected && !custom { - self.page = requirement.custom; - self.scroll = 0; - self.error = - Some("Select an option or enter a non-blank custom answer.".into()); - return None; + } + if reviewing { + let mut content = Map::new(); + for field in &self.fields { + if let Ok(value) = field.validate() + && !value.is_null() + { + content.insert(field.name.clone(), value); } } return serde_json::from_value(Value::Object(Map::from_iter([ @@ -707,17 +755,16 @@ impl Form { ]))) .ok(); } - match self.fields[self.page].validate() { - Ok(_) => { - self.page += 1; - self.scroll = 0; - self.error = None; - } - Err(error) => self.error = Some(error), - } + self.page += 1; + self.other_focus = false; + self.scroll = 0; + self.error = None; } _ => { - if let Some(field) = self.fields.get_mut(self.page) { + if let Some(field) = self + .focused_field() + .and_then(|index| self.fields.get_mut(index)) + { match key.code { KeyCode::Up => { field.cursor = field.cursor.saturating_sub(1); @@ -766,14 +813,15 @@ impl Form { frame.render_widget(block, area); let parts = Layout::vertical([Constraint::Min(1), Constraint::Length(3)]).split(inner); let mut text = format!("{}\n\n", self.message); - if let Some(field) = self.fields.get(self.page) { + if let Some(&selection) = self.pages.get(self.page) { + let field = &self.fields[selection]; let title = annotation(&field.schema, "title"); text.push_str(&format!( "Question {}/{} — {} {}\n{}\n", self.page + 1, - self.fields.len(), + self.pages.len(), if title.is_empty() { "Answer" } else { &title }, - if field.required { + if field.required || self.custom_field(selection).is_some() { "(required)" } else { "(optional)" @@ -794,6 +842,60 @@ impl Form { } text.push('\n'); if !field.options.is_empty() { + let custom = self.custom_field(selection); + text.push_str(if self.other_focus { + "Focus: Other (Tab: choices)\n" + } else if custom.is_some() { + "Focus: choices (Tab: Other)\n" + } else { + "Focus: choices\n" + }); + if let Some(custom) = custom { + let other = &self.fields[custom]; + text.push_str(&format!( + "Other: {}\n", + annotation(&other.schema, "description") + )); + for key in ["minLength", "maxLength"] { + if !other.schema[key].is_null() { + text.push_str(&format!("Other {key}: {}\n", other.schema[key])); + } + } + if other.required { + text.push_str("Other text is required.\n"); + } + } + let choice_area = if let Some(custom) = custom { + let areas = Layout::vertical([Constraint::Min(1), Constraint::Length(4)]) + .split(parts[0]); + let other = &self.fields[custom]; + let editor_area = Layout::vertical([Constraint::Length(1), Constraint::Min(0)]) + .split(areas[1]); + frame.render_widget( + Paragraph::new(if self.other_focus { + "> Other" + } else { + " Other" + }), + editor_area[0], + ); + let (lines, (row, column)) = other.editor.wrapped(self.editor_width); + let first = + row.saturating_sub(usize::from(editor_area[1].height).saturating_sub(1)); + frame.render_widget( + Paragraph::new(display(&lines.join("\n"))).scroll((first as u16, 0)), + editor_area[1], + ); + if self.other_focus && editor_area[1].height > 0 && editor_area[1].width > 0 { + frame.set_cursor_position(( + editor_area[1].x + (column as u16).min(editor_area[1].width - 1), + editor_area[1].y + (row - first) as u16, + )); + } + areas[0] + } else { + parts[0] + }; let option = &field.options[field.cursor]; text.push_str(&format!( "Highlighted: {}\n{}\n", @@ -801,7 +903,7 @@ impl Form { )); let regions = Layout::vertical([Constraint::Percentage(60), Constraint::Percentage(40)]) - .split(parts[0]); + .split(choice_area); render_text(frame, regions[0], text, &mut self.scroll); let height = usize::from(regions[1].height).max(1); let start = field.cursor.saturating_sub(height - 1); @@ -814,7 +916,11 @@ impl Form { .map(|(i, o)| { format!( "{} [{}] {}", - if i == field.cursor { ">" } else { " " }, + if i == field.cursor && !self.other_focus { + ">" + } else { + " " + }, if field.selected(o) { "x" } else { " " }, o.label ) @@ -855,9 +961,10 @@ impl Form { } } else { text.push_str("Review answers — BackTab to modify; Enter to submit\n\n"); - for (index, field) in self.fields.iter().enumerate() { + for (index, &selection) in self.pages.iter().enumerate() { + let field = &self.fields[selection]; let value = field.answer().unwrap_or(Value::Null); - let display = if value.is_null() { + let mut display = if value.is_null() { "(omitted)".into() } else if !field.options.is_empty() { field @@ -870,6 +977,16 @@ impl Form { } else { value.to_string() }; + if let Some(custom) = self.custom_field(selection) { + let custom = self.fields[custom].editor.text(); + if !custom.trim().is_empty() { + if display == "(omitted)" || display.is_empty() { + display = custom.to_owned(); + } else { + display.push_str(&format!("; Other: {custom}")); + } + } + } let title = annotation(&field.schema, "title"); let label = if title.is_empty() { format!("Answer {}", index + 1) @@ -880,7 +997,7 @@ impl Form { } render_text(frame, parts[0], text, &mut self.scroll); } - frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • Shift+Tab: back • Space: select\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); + frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • Shift+Tab: back • Space: select • Tab: focus\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); } } @@ -1110,6 +1227,41 @@ mod tests { ); assert!(form(json!({"x":{"type":"_future"}}), json!([])).is_err()); } + #[test] + fn grouped_single_choice_keeps_generic_fields_and_inline_text() { + let mut f = Form::from_value(json!({"mode":"form","message":"Choose","requestedSchema":{ + "type":"object","properties":{ + "custom":{"type":"string","description":"Add context","maxLength":20}, + "generic":{"type":"string"}, + "pick":{"type":"string","oneOf":[{"const":"a","title":"First"}]} + },"_meta":{"kit/questions":{"order":["pick","custom","generic"], + "require_answer":[{"selection":"pick","custom":"custom"}]}} + }})) + .unwrap(); + assert_eq!(f.pages.len(), 2); + press(&mut f, KeyCode::Char(' ')); + press(&mut f, KeyCode::Tab); + f.paste("context"); + let clipboard = f.clipboard_target(); + press(&mut f, KeyCode::Tab); + assert_ne!(clipboard, f.clipboard_target()); + f.paste("ignored"); + press(&mut f, KeyCode::Enter); + f.paste("unrelated"); + press(&mut f, KeyCode::Enter); + press(&mut f, KeyCode::BackTab); + assert_eq!(f.page, 1); + press(&mut f, KeyCode::BackTab); + assert_eq!(f.page, 0); + press(&mut f, KeyCode::Enter); + press(&mut f, KeyCode::Enter); + let result = serde_json::to_value(press(&mut f, KeyCode::Enter).unwrap()).unwrap(); + assert_eq!( + result["content"], + json!({"pick":"a","custom":"context","generic":"unrelated"}) + ); + } + #[test] fn answer_requirement_metadata_rejects_unsafe_references() { let base = json!({"mode":"form","message":"Choose","requestedSchema":{"type":"object","properties":{ diff --git a/src/tui/elicitation_requests.rs b/src/tui/elicitation_requests.rs index 8630d60b..0af7fa56 100644 --- a/src/tui/elicitation_requests.rs +++ b/src/tui/elicitation_requests.rs @@ -568,6 +568,7 @@ mod tests { generation: 1, })), )); + let route = ingress.route.clone(); let origin = ingress.origin.clone(); let client = agent_client_protocol::Client .v2() @@ -639,43 +640,46 @@ mod tests { let visible = screen(&mut app); assert!(!visible.contains("Request from")); assert!(!visible.contains("Agent request")); - assert!(visible.contains("Question 1/3")); + assert!(visible.contains("Question 1/2")); assert!(visible.contains("Terminal UI")); + assert!(visible.contains("Other")); + // Choice focus does not accept text or paste. + app.paste("not an answer"); + press(&mut app, KeyCode::Char('z')); if include_selections { for key in [KeyCode::Char(' '), KeyCode::Down, KeyCode::Char(' ')] { press(&mut app, key); } - } - press(&mut app, KeyCode::Enter); - assert!(screen(&mut app).contains("custom answer")); - if include_selections { app.paste("Also docs, please"); } - press(&mut app, KeyCode::Enter); - let visible = screen(&mut app); - assert!(visible.contains("Question 3/3")); - assert!(visible.contains("Strategy")); - // Selecting a second option replaces the first when multiple - // is omitted, rather than returning two single-choice values. - for key in [KeyCode::Down, KeyCode::Char(' '), KeyCode::Up, KeyCode::Char(' '), KeyCode::Enter] { press(&mut app, key); } - if !include_selections { - // The tool's generated metadata requires a choice or - // non-blank custom text. Both empty and whitespace-only - // submissions must stay open and focus the custom field. - assert!(screen(&mut app).contains("Review answers")); + press(&mut app, KeyCode::Tab); + } else { press(&mut app, KeyCode::Enter); - let invalid = screen(&mut app); - assert!(invalid.contains("custom answer")); - assert!(invalid.contains("non-blank")); - assert_eq!(app.elicitations.len(), 1); + assert!(screen(&mut app).contains("non-blank")); app.paste(" "); press(&mut app, KeyCode::Enter); - press(&mut app, KeyCode::Enter); - assert!(screen(&mut app).contains("Review answers")); - press(&mut app, KeyCode::Enter); assert!(screen(&mut app).contains("non-blank")); - assert_eq!(app.elicitations.len(), 1); app.handle_key(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)); - app.paste("Also docs, please"); - press(&mut app, KeyCode::Enter); - press(&mut app, KeyCode::Enter); } + assert!(screen(&mut app).contains("Focus: Other")); + app.paste("Also docs, please"); + let stale = app.clipboard_route(); + press(&mut app, KeyCode::Tab); + assert!(screen(&mut app).contains("Focus: choices")); + assert_eq!(crate::tui::apply_clipboard_completion(&mut app, &route, 1, stale, + crate::tui::ClipboardResult::Text("stale".into())), None); + press(&mut app, KeyCode::Tab); + // Moving the cursor and inserting paste uses the same editor. + press(&mut app, KeyCode::Home); + app.paste("X"); + press(&mut app, KeyCode::Backspace); + press(&mut app, KeyCode::Enter); + let visible = screen(&mut app); + assert!(visible.contains("Question 2/2")); + assert!(visible.contains("Strategy")); + press(&mut app, KeyCode::BackTab); + assert!(screen(&mut app).contains("Also docs, please")); + press(&mut app, KeyCode::Enter); + // A second single selection replaces the first. + for key in [KeyCode::Down, KeyCode::Char(' '), KeyCode::Up, KeyCode::Char(' '), KeyCode::Enter] { press(&mut app, key); } + press(&mut app, KeyCode::BackTab); + press(&mut app, KeyCode::Enter); let review = screen(&mut app); assert!(review.contains("Review answers")); if include_selections { assert!(review.contains("Terminal UI, Agent runtime")); } From 603598ee3a9747a7ddf877e71919ad4bb33b4c9d Mon Sep 17 00:00:00 2001 From: daniel Date: Thu, 22 Oct 2026 15:50:11 +0100 Subject: [PATCH 4/6] fix(tui): allow Tab navigation through unanswered questions --- docs/user/tui-and-sessions.md | 7 ++- src/tui/elicitation.rs | 97 +++++++++++++++++++++++++++------ src/tui/elicitation_requests.rs | 2 +- 3 files changed, 85 insertions(+), 21 deletions(-) diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index 25c43164..45d597dd 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -139,9 +139,10 @@ When an agent needs a decision, Kit can show a form with single-choice or multip - Use `Up` / `Down` to highlight an option and `Space` to select it. For multiple-choice fields, `Space` toggles each option independently. - Text and numeric fields use the composer editor: move the cursor with arrow keys, use `Alt+Left` / `Alt+Right` to move by word, and `Alt+Backspace` or `Ctrl+W` to delete the previous word. `Ctrl+U` / `Ctrl+K` delete to the start / end of the line. `Shift+Enter` inserts a newline; `Ctrl+V` / `Shift+Insert` and bracketed paste insert text at the cursor. Clipboard images are not accepted in form fields. Use `Left` / `Right` to choose false / true for a boolean field, or `Space` to toggle it. -- When a question offers **Other**, its text input appears below the choices on the same page. Press `Tab` to switch focus between the choices and Other. You can keep selected options and an Other answer together, or enter only an Other answer. -- Press `Enter` to advance and `Shift+Tab` to go back without losing answers. Required questions must have a selection or non-blank Other text before you can advance. Unrelated form fields remain on separate pages. -- On the review page, press `Enter` to submit all answers together, or `Shift+Tab` to revise them. +- When a question offers **Other**, its text input appears below the choices on the same page. Press `Tab` to move from the choices to Other, then to the next question. You can keep selected options and an Other answer together, or enter only an Other answer. +- Press `Tab` to move forward through inputs and questions to review, even when an answer is blank. `Shift+Tab` moves backwards through the same inputs: from Other to choices, or from choices to the previous question’s last input. Neither key validates answers or submits the form; drafts are preserved. Unrelated form fields remain on separate pages. +- Press `Enter` to validate the current question and advance. A required choice question must have a selection or non-blank Other text. +- On the review page, press `Enter` to validate and submit all answers together, or `Shift+Tab` to revise them. Skipped required answers return you to the input that needs attention. `Tab` on review stays there without submitting. - Press `Delete` to clear a choice or boolean field; in text fields it deletes forward from the cursor. Use `PageUp` / `PageDown` to scroll the question or review. - Press `Esc` to dismiss the request, or `Ctrl+D` to explicitly decline. Neither action submits selections. diff --git a/src/tui/elicitation.rs b/src/tui/elicitation.rs index 1bf69359..8d0e565b 100644 --- a/src/tui/elicitation.rs +++ b/src/tui/elicitation.rs @@ -695,14 +695,26 @@ impl Form { wire::ElicitationAction::Decline, )); } - if key.code == KeyCode::Tab - && self - .pages - .get(self.page) - .and_then(|i| self.custom_field(*i)) - .is_some() - { - self.other_focus = !self.other_focus; + if matches!(key.code, KeyCode::Tab | KeyCode::BackTab) { + let backwards = + key.code == KeyCode::BackTab || key.modifiers.contains(KeyModifiers::SHIFT); + if backwards { + if self.other_focus { + self.other_focus = false; + } else if self.page > 0 { + self.page -= 1; + self.other_focus = self.custom_field(self.pages[self.page]).is_some(); + } + } else if self.page < self.pages.len() { + if !self.other_focus && self.custom_field(self.pages[self.page]).is_some() { + self.other_focus = true; + } else { + self.page += 1; + self.other_focus = false; + } + } + self.scroll = 0; + self.error = None; return None; } if let Some(field) = self @@ -716,12 +728,6 @@ impl Form { return None; } match key.code { - KeyCode::BackTab => { - self.other_focus = false; - self.page = self.page.saturating_sub(1); - self.scroll = 0; - self.error = None; - } KeyCode::PageDown => self.scroll = self.scroll.saturating_add(5), KeyCode::PageUp => self.scroll = self.scroll.saturating_sub(5), KeyCode::Enter => { @@ -844,7 +850,7 @@ impl Form { if !field.options.is_empty() { let custom = self.custom_field(selection); text.push_str(if self.other_focus { - "Focus: Other (Tab: choices)\n" + "Focus: Other (Tab: next; Shift+Tab: choices)\n" } else if custom.is_some() { "Focus: choices (Tab: Other)\n" } else { @@ -997,7 +1003,7 @@ impl Form { } render_text(frame, parts[0], text, &mut self.scroll); } - frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • Shift+Tab: back • Space: select • Tab: focus\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); + frame.render_widget(Paragraph::new(format!("{}\nEnter: next/submit • Shift+Tab: back • Space: select • Tab: forward\nEsc: cancel • Ctrl+D: decline • PgUp/PgDn: scroll", self.error.as_deref().unwrap_or(""))).wrap(Wrap { trim: false }), parts[1]); } } @@ -1227,6 +1233,63 @@ mod tests { ); assert!(form(json!({"x":{"type":"_future"}}), json!([])).is_err()); } + #[test] + fn tab_traverses_unanswered_inputs_symmetrically_without_submitting() { + let mut f = Form::from_value(json!({"mode":"form","message":"Choose","requestedSchema":{ + "type":"object","required":["plain"],"properties":{ + "plain":{"type":"string","minLength":1},"pick":{"type":"string","enum":["a"]}, + "custom":{"type":"string"} + },"_meta":{"kit/questions":{"order":["plain","pick","custom"], + "require_answer":[{"selection":"pick","custom":"custom"}]}} + }})) + .unwrap(); + assert!(press(&mut f, KeyCode::BackTab).is_none()); + assert_eq!((f.page, f.other_focus), (0, false)); + for expected in [(1, false), (1, true), (2, false), (2, false)] { + assert!(press(&mut f, KeyCode::Tab).is_none()); + assert_eq!((f.page, f.other_focus), expected); + assert!(f.error.is_none()); + } + // Both terminal encodings move backwards, including from review. + for (key, expected) in [ + (KeyEvent::new(KeyCode::Tab, KeyModifiers::SHIFT), (1, true)), + ( + KeyEvent::new(KeyCode::BackTab, KeyModifiers::NONE), + (1, false), + ), + (KeyEvent::new(KeyCode::Tab, KeyModifiers::SHIFT), (0, false)), + ( + KeyEvent::new(KeyCode::BackTab, KeyModifiers::NONE), + (0, false), + ), + ] { + assert!(f.key(key).is_none()); + assert_eq!((f.page, f.other_focus), expected); + } + assert!(f.fields.iter().all(|field| field.editor.text().is_empty())); + for _ in 0..3 { + press(&mut f, KeyCode::Tab); + } + assert!(press(&mut f, KeyCode::Enter).is_none()); + assert_eq!((f.page, f.other_focus), (0, false)); + assert!(f.error.is_some()); + f.paste("draft"); + let stale = f.clipboard_target(); + press(&mut f, KeyCode::Tab); + assert_ne!(f.clipboard_target(), stale); + assert!(f.error.is_none()); + press(&mut f, KeyCode::Tab); + f.paste(" "); + press(&mut f, KeyCode::Tab); + assert!(press(&mut f, KeyCode::Enter).is_none()); + assert_eq!((f.page, f.other_focus), (1, true)); + assert!(f.error.as_deref().unwrap().contains("non-blank")); + press(&mut f, KeyCode::BackTab); + assert!(f.error.is_none()); + press(&mut f, KeyCode::BackTab); + assert_eq!(f.fields[f.focused_field().unwrap()].editor.text(), "draft"); + } + #[test] fn grouped_single_choice_keeps_generic_fields_and_inline_text() { let mut f = Form::from_value(json!({"mode":"form","message":"Choose","requestedSchema":{ @@ -1243,7 +1306,7 @@ mod tests { press(&mut f, KeyCode::Tab); f.paste("context"); let clipboard = f.clipboard_target(); - press(&mut f, KeyCode::Tab); + press(&mut f, KeyCode::BackTab); assert_ne!(clipboard, f.clipboard_target()); f.paste("ignored"); press(&mut f, KeyCode::Enter); diff --git a/src/tui/elicitation_requests.rs b/src/tui/elicitation_requests.rs index 0af7fa56..6531c187 100644 --- a/src/tui/elicitation_requests.rs +++ b/src/tui/elicitation_requests.rs @@ -660,7 +660,7 @@ mod tests { assert!(screen(&mut app).contains("Focus: Other")); app.paste("Also docs, please"); let stale = app.clipboard_route(); - press(&mut app, KeyCode::Tab); + press(&mut app, KeyCode::BackTab); assert!(screen(&mut app).contains("Focus: choices")); assert_eq!(crate::tui::apply_clipboard_completion(&mut app, &route, 1, stale, crate::tui::ClipboardResult::Text("stale".into())), None); From 04c9fd7e69ff42e37b268a36cac5c7f7c8b7eaad Mon Sep 17 00:00:00 2001 From: daniel Date: Thu, 22 Oct 2026 16:24:09 +0100 Subject: [PATCH 5/6] feat(config): allow disabling the ask_user tool --- docs/user/compose-and-local-tools.md | 2 +- .../user/getting-started-and-configuration.md | 21 +++++ src/main.rs | 72 ++++++++++++++++ src/runtime.rs | 84 ++++++++++++++++--- 4 files changed, 168 insertions(+), 11 deletions(-) diff --git a/docs/user/compose-and-local-tools.md b/docs/user/compose-and-local-tools.md index c70abd10..c80e9512 100644 --- a/docs/user/compose-and-local-tools.md +++ b/docs/user/compose-and-local-tools.md @@ -4,7 +4,7 @@ Kit exposes `compose` for tool orchestration and, when the connected client supp ## Ask for a user decision -When an ACP v2 client advertises form elicitation, Kit also exposes `ask_user` as a standalone tool. It is not a Runlet function and cannot run in the background. Use it for decisions that need explicit user input, not credentials or secrets. +When an ACP v2 client advertises form elicitation and `ask_user` is enabled (the default), Kit also exposes `ask_user` as a standalone tool. It is not a Runlet function and cannot run in the background. Use it for decisions that need explicit user input, not credentials or secrets. To disable this tool, see [Disable native user questions](getting-started-and-configuration.md#disable-native-user-questions). A request contains a `message` and 1–8 `questions`. Each question has a stable `id`, `title`, `prompt`, and 1–32 `options` with stable IDs and display labels. Set `multiple: true` for multiple selections and `allow_custom: true` to offer a separate text field. Both flags default to false. The complete form request is limited to 128 KiB; shorten descriptions or split the batch if the tool reports that the form is too large. diff --git a/docs/user/getting-started-and-configuration.md b/docs/user/getting-started-and-configuration.md index 7e81a468..97840857 100644 --- a/docs/user/getting-started-and-configuration.md +++ b/docs/user/getting-started-and-configuration.md @@ -255,6 +255,27 @@ If the config file is missing, `set` creates it, `get` reports an error, and `un These commands only inspect or edit the local configuration: they do not run migrations, authenticate, or access the network. They do not open an interactive dialog. Use standard help such as `kit config --help` or `kit config set --help` for command syntax. Runtime commands still validate the settings they consume. +### Disable native user questions + +To prevent Kit's native agent from exposing the `ask_user` tool, set the top-level +`ask_user` boolean to `false`: + +```sh +kit config set ask_user false +``` + +This writes `ask_user = false` in `~/.kit/config.toml`. Restart the running Kit +process to apply the setting; configuration is read at startup. The default is +`true`, but the tool still requires an ACP v2 client that supports form elicitation. +This setting does not disable generic form display or change the TUI's advertised +form capability. + +To restore the default, remove the setting and restart Kit: + +```sh +kit config unset ask_user +``` + ### Configuration example A representative configuration is: diff --git a/src/main.rs b/src/main.rs index 8b6660f5..83f0d845 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1313,8 +1313,17 @@ const CONFIG_MIGRATIONS: &[ConfigMigration] = &[ ConfigMigration { apply: migrate_compaction_policy, }, + ConfigMigration { + apply: migrate_ask_user, + }, ]; +fn migrate_ask_user(config: &mut toml::Table) { + config + .entry("ask_user") + .or_insert(toml::Value::Boolean(true)); +} + // Additive policy: preserve explicit values and malformed shapes for validation. // This migration only changes the in-memory document, never the user's file. fn migrate_compaction_policy(config: &mut toml::Table) { @@ -1364,6 +1373,7 @@ fn migrate_config(mut config: toml::Table) -> toml::Table { #[derive(Debug, Default, Deserialize)] struct Config { + ask_user: Option, #[serde(default)] compaction: kit::compaction::policy::CompactionConfig, #[serde(default, deserialize_with = "deserialize_experimental_config")] @@ -2297,6 +2307,7 @@ async fn run_cli(cli: Cli) -> Result<(), Box> { }; let runtime = kit::Runtime::with_compaction(runtime, config.compaction)?; let runtime = kit::Runtime::with_eval(runtime, config.experimental.eval)?; + let runtime = kit::Runtime::with_ask_user(runtime, config.ask_user.unwrap_or(true))?; let runtime = kit::Runtime::with_hooks(runtime)?; let runtime = kit::Runtime::with_plugin_runtime(runtime, plugins)?; let runtime = kit::Runtime::with_telemetry(runtime, telemetry_settings.clone())?; @@ -2397,6 +2408,7 @@ async fn run_cli(cli: Cli) -> Result<(), Box> { }; let runtime = kit::Runtime::with_compaction(runtime, config.compaction)?; let runtime = kit::Runtime::with_eval(runtime, config.experimental.eval)?; + let runtime = kit::Runtime::with_ask_user(runtime, config.ask_user.unwrap_or(true))?; let runtime = kit::Runtime::with_hooks(runtime)?; let runtime = kit::Runtime::with_plugin_runtime(runtime, plugins)?; let runtime = kit::Runtime::with_telemetry(runtime, telemetry_settings.clone())?; @@ -2472,6 +2484,7 @@ async fn run_cli(cli: Cli) -> Result<(), Box> { )?; let runtime = kit::Runtime::with_compaction(runtime, config.compaction)?; let runtime = kit::Runtime::with_eval(runtime, config.experimental.eval)?; + let runtime = kit::Runtime::with_ask_user(runtime, config.ask_user.unwrap_or(true))?; let runtime = kit::Runtime::with_hooks(runtime)?; let runtime = kit::Runtime::with_plugin_runtime(runtime, plugins)?; let runtime = kit::Runtime::with_telemetry(runtime, telemetry_settings.clone())?; @@ -2936,6 +2949,65 @@ mod tests { } } + #[test] + fn config_ask_user_defaults_and_strict_boolean() { + assert!(Config::default().ask_user.unwrap_or(true)); + for (text, expected) in [ + ("", true), + ("ask_user = true", true), + ("ask_user = false", false), + ] { + let config: Config = toml::from_str(text).unwrap(); + assert_eq!(config.ask_user.unwrap_or(true), expected); + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.toml"); + fs::write(&path, text).unwrap(); + assert_eq!( + Config::load(&path).unwrap().ask_user.unwrap_or(true), + expected + ); + assert_eq!(fs::read_to_string(&path).unwrap(), text); + let migrated = super::migrate_config(toml::from_str(text).unwrap()); + assert_eq!(migrated["ask_user"].as_bool(), Some(expected)); + assert_eq!(super::migrate_config(migrated.clone()), migrated); + } + for value in ["'true'", "1", "0.0", "[]", "{}"] { + let text = format!("ask_user = {value}"); + assert!(toml::from_str::(&text).is_err(), "{text}"); + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.toml"); + fs::write(&path, &text).unwrap(); + assert!(Config::load(&path).is_err(), "{text}"); + assert_eq!(fs::read_to_string(&path).unwrap(), text); + } + } + + #[test] + fn config_ask_user_writer_preserves_settings() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.toml"); + fs::write( + &path, + "# preserved\nask_user = false # opt out\nmodel = 'old'\nfuture = 'keep'\n", + ) + .unwrap(); + kit::config_editor::set(&path, "model", "'new'").unwrap(); + let updated = fs::read_to_string(&path).unwrap(); + assert!(updated.contains("ask_user = false # opt out")); + assert!(updated.starts_with("# preserved\n")); + assert!(!Config::load(&path).unwrap().ask_user.unwrap_or(true)); + kit::config_editor::set(&path, "ask_user", "true").unwrap(); + assert!(Config::load(&path).unwrap().ask_user.unwrap_or(true)); + kit::config_editor::set(&path, "ask_user", "false").unwrap(); + assert!(!Config::load(&path).unwrap().ask_user.unwrap_or(true)); + kit::config_editor::unset(&path, "ask_user").unwrap(); + assert!(Config::load(&path).unwrap().ask_user.unwrap_or(true)); + let updated = fs::read_to_string(&path).unwrap(); + assert!(!updated.contains("ask_user")); + assert!(updated.contains("future = 'keep'")); + assert!(updated.contains("model = 'new'")); + } + #[cfg(feature = "tui")] #[test] fn config_experimental_voice_defaults_and_strict_boolean() { diff --git a/src/runtime.rs b/src/runtime.rs index f43d347a..aab2d5e1 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -503,6 +503,7 @@ impl std::fmt::Display for LogoutAuthenticationError { } pub struct Runtime { + ask_user_enabled: bool, eval: Option, root: PathBuf, adapter: SelectableAdapter, @@ -622,6 +623,7 @@ impl Runtime { prompt_cache_key: None, }); Ok(Arc::new(Self { + ask_user_enabled: true, eval: None, root, adapter, @@ -815,6 +817,31 @@ impl Runtime { Ok(Arc::new(runtime)) } + /// Controls native user questions before publishing the runtime to sessions. + /// ACP client form capability is still required when enabled. + pub fn with_ask_user(runtime: Arc, enabled: bool) -> Result, String> { + let mut runtime = Arc::try_unwrap(runtime).map_err(|_| { + "could not configure user questions after runtime was shared".to_string() + })?; + runtime.ask_user_enabled = enabled; + Ok(Arc::new(runtime)) + } + + fn user_question_tools( + &self, + ask_user: Option, + ) -> agentkit_tools_core::ToolRegistry { + let mut questions = agentkit_tools_core::ToolRegistry::new(); + if let Some(ask_user) = ask_user.filter(|_| self.ask_user_enabled) { + questions = questions.with(Observed::new(ask_user)); + } + if let Some(hooks) = &self.hooks { + crate::ahp::hooked_tools(questions, hooks) + } else { + questions + } + } + /// Enables evaluation only after explicit user opt-in and credential resolution. pub fn with_eval(runtime: Arc, enabled: bool) -> Result, String> { let mut runtime = Arc::try_unwrap(runtime) @@ -1978,16 +2005,8 @@ impl Runtime { crate::ahp::PromptRecord::Session(opened.observer.clone()), )); } - if let Some(ask_user) = context.ask_user { - // User interaction remains foreground-only, outside backgroundable compose. - let questions = agentkit_tools_core::ToolRegistry::new().with(Observed::new(ask_user)); - let questions = if let Some(hooks) = &self.hooks { - crate::ahp::hooked_tools(questions, hooks) - } else { - questions - }; - builder = builder.add_tool_source(questions); - } + // User interaction remains foreground-only, outside backgroundable compose. + builder = builder.add_tool_source(self.user_question_tools(context.ask_user)); let driver = builder .observer(context.integration.as_ref().clone()) .observer(crate::fatal::RetryLog) @@ -3402,3 +3421,48 @@ mod additional_directory_tests { #[cfg(test)] mod prompt_tests; + +#[cfg(test)] +#[allow(clippy::disallowed_methods, clippy::disallowed_macros)] +mod user_question_tests { + use super::*; + + #[tokio::test] + async fn ask_user_catalog_requires_setting_and_client_capability() { + let root = tempfile::tempdir().unwrap(); + let runtime = Runtime::new_with_provider_and_credentials( + root.path(), + "gpt-5.4", + ProviderKind::OpenAiSubscription, + crate::credentials::CredentialStorage::Memory, + ) + .unwrap(); + assert!(runtime.ask_user_enabled); + let (_client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + agent_client_protocol::Agent + .v2() + .connect_with(agent_transport, async move |connection| { + let mut runtime = runtime; + for enabled in [false, true, false] { + runtime = Runtime::with_ask_user(runtime, enabled).unwrap(); + for capable in [false, true] { + let tool = capable.then(|| { + crate::tools::ask_user::AskUserTool::new( + connection.clone(), + agentkit_acp::v2::wire::SessionId::new("catalog-test"), + ) + }); + let catalog = runtime.user_question_tools(tool); + assert_eq!( + catalog.get(&ToolName::new("ask_user")).is_some(), + enabled && capable, + ); + assert_eq!(catalog.specs().len(), usize::from(enabled && capable)); + } + } + Ok::<(), agent_client_protocol::Error>(()) + }) + .await + .unwrap(); + } +} From ca05cc22e09306c50993a55f7bb24a3e6d57a141 Mon Sep 17 00:00:00 2001 From: daniel Date: Thu, 22 Oct 2026 17:51:04 +0100 Subject: [PATCH 6/6] fix(tui): preserve durability warnings during user questions --- src/tui/elicitation_requests.rs | 46 ++++++++++++++++++++++++++++ src/tui/ui.rs | 53 ++++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/tui/elicitation_requests.rs b/src/tui/elicitation_requests.rs index 6531c187..fa943b00 100644 --- a/src/tui/elicitation_requests.rs +++ b/src/tui/elicitation_requests.rs @@ -286,6 +286,52 @@ mod tests { assert_eq!(app.editor.text(), "parked draft"); } + #[test] + fn elicitation_keeps_status_and_durability_warnings_below_controls() { + let ingress = ingress(); + let (queued, _receiver) = ingress.prepare(request("session")).unwrap(); + let mut app = app(); + app.apply(queued.update); + app.toast = Some(("Example status toast".into(), std::time::Instant::now())); + let mut images = crate::tui::image::ImageRuntime::disabled(); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 40)).unwrap(); + for exhausted in [false, true] { + app.storage_pending = !exhausted; + app.storage_exhausted = exhausted; + terminal + .draw(|frame| crate::tui::ui::draw(frame, &mut app, &mut images)) + .unwrap(); + let rows: Vec = terminal + .backend() + .buffer() + .content() + .chunks(100) + .map(|row| row.iter().map(|cell| cell.symbol()).collect()) + .collect(); + assert!(rows.iter().any(|row| row.contains("Choose a scope"))); + let controls = rows + .iter() + .position(|row| row.contains("Esc: cancel")) + .unwrap(); + let status = rows + .iter() + .position(|row| row.contains("Example status toast")) + .unwrap(); + let warning = rows + .iter() + .position(|row| { + row.contains(if exhausted { + "unpersisted data is at risk" + } else { + "data at risk on exit" + }) + }) + .unwrap(); + assert!(controls < status && status < warning); + } + } + #[test] fn elicitation_panel_keeps_conversation_visible_without_generated_chrome() { let ingress = ingress(); diff --git a/src/tui/ui.rs b/src/tui/ui.rs index 9efff6c1..df250f00 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -66,10 +66,12 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { // normal composer stays parked while the form owns keyboard input. let area = frame.area(); let panel_rows = (area.height / 2).min(18); - let [header, body, panel] = Layout::vertical([ + let [header, body, panel, status, durability] = Layout::vertical([ Constraint::Length(u16::from(area.height > 2)), Constraint::Min(0), Constraint::Length(panel_rows), + Constraint::Length(1), + Constraint::Length(u16::from(durability_warning(app).is_some())), ]) .areas(area); app.child_back_area = Rect::default(); @@ -86,6 +88,8 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { if let Some(pending) = app.elicitations.front_mut() { pending.form.render_in(frame, panel); } + draw_status(frame, app, status); + draw_durability_warning(frame, app, durability); return; } app.child_back_area = Rect::default(); @@ -183,23 +187,36 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { } // Durability stays visible on the start screen and over session pickers. // Pending data belongs to the process, not the currently selected session. - if app.runtime_unavailable() || app.storage_pending || app.storage_exhausted { - let area = frame.area(); - let warning = if app.runtime_unavailable() { - " Runtime status unavailable: agent, child, compaction and storage state unknown" - } else if app.storage_exhausted { - " Storage exhausted: shutting down; unpersisted data is at risk" - } else { - " Memory-only storage: awaiting disk recovery; data at risk on exit" - }; + let area = frame.area(); + draw_durability_warning( + frame, + app, + Rect::new( + area.x, + area.bottom().saturating_sub(1), + area.width, + u16::from(area.height > 0), + ), + ); +} + +fn durability_warning(app: &App) -> Option<&'static str> { + if app.runtime_unavailable() { + Some(" Runtime status unavailable: agent, child, compaction and storage state unknown") + } else if app.storage_exhausted { + Some(" Storage exhausted: shutting down; unpersisted data is at risk") + } else if app.storage_pending { + Some(" Memory-only storage: awaiting disk recovery; data at risk on exit") + } else { + None + } +} + +fn draw_durability_warning(frame: &mut Frame<'_>, app: &App, area: Rect) { + if let Some(warning) = durability_warning(app) { frame.render_widget( Paragraph::new(warning).style(Style::default().fg(theme::warn_color())), - Rect::new( - area.x, - area.bottom().saturating_sub(1), - area.width, - u16::from(area.height > 0), - ), + area, ); } } @@ -3249,7 +3266,9 @@ fn draw_status(frame: &mut Frame<'_>, app: &App, area: Rect) { Style::default().fg(theme::warn_color()), )); } - let hints = if app.editing_steer() { + let hints = if !app.elicitations.is_empty() { + "" + } else if app.editing_steer() { "⏎ save edit esc restore draft " } else if app.queue_focused && !app.pending_steers.is_empty() { if app.can_replace_steer