diff --git a/crates/helexa-acp/src/agent.rs b/crates/helexa-acp/src/agent.rs index f35c7ae..7239da3 100644 --- a/crates/helexa-acp/src/agent.rs +++ b/crates/helexa-acp/src/agent.rs @@ -35,7 +35,7 @@ use crate::prompt::build_system_prompt; use crate::provider::{ CompletionEvent, CompletionRequest, Message, MessageContent, Provider, Role, ToolCall, }; -use crate::session::{self, MODE_BYPASS, MODE_DEFAULT, SessionState, SessionStore}; +use crate::session::{self, MODE_BYPASS, MODE_DEFAULT, MODE_PLAN, SessionState, SessionStore}; use crate::store::{self, PersistedSession}; use crate::tool_runner::{AcpClientOps, ToolCallEvent, dispatch_tool_call}; use crate::tools; @@ -547,10 +547,14 @@ fn derive_session_title(history: &[Message]) -> Option { .filter(|s| !s.is_empty()) } -/// The two modes every Stage 3 session advertises. Stage 7 may grow -/// this list (e.g. "plan" for plan-only output, "ask" for read-only), -/// but Default + Bypass cover the two operationally distinct -/// permission policies. +/// The three modes every Stage 3 session advertises: +/// +/// - **Default** — writes / bash prompt the user. +/// - **Bypass Permissions** — auto-allow. +/// - **Plan** — read-and-plan-only. Writes are restricted to a +/// per-project plan directory under `$XDG_DATA_HOME/helexa-acp/plans/` +/// and bash is disabled. Designed for "draft the implementation +/// plan, then I'll review and let you execute" flows. fn default_mode_state() -> SessionModeState { SessionModeState::new( SessionModeId::new(MODE_DEFAULT), @@ -559,6 +563,8 @@ fn default_mode_state() -> SessionModeState { .description("Prompt for permission before writes or shell commands."), SessionMode::new(SessionModeId::new(MODE_BYPASS), "Bypass Permissions") .description("Auto-allow all tool calls. Use with care."), + SessionMode::new(SessionModeId::new(MODE_PLAN), "Plan") + .description("Write plans to the plan directory; no shell, no writes outside it."), ], ) } @@ -570,13 +576,17 @@ async fn handle_set_session_mode( let Some(state) = session::get(&inner.sessions, &req.session_id).await else { anyhow::bail!("unknown session id {}", req.session_id.0); }; - let accepted = req.mode_id.0.as_ref() == MODE_DEFAULT || req.mode_id.0.as_ref() == MODE_BYPASS; + let accepted = matches!( + req.mode_id.0.as_ref(), + MODE_DEFAULT | MODE_BYPASS | MODE_PLAN + ); if !accepted { anyhow::bail!( - "unknown mode '{}' — must be one of: {}, {}", + "unknown mode '{}' — must be one of: {}, {}, {}", req.mode_id.0, MODE_DEFAULT, - MODE_BYPASS + MODE_BYPASS, + MODE_PLAN ); } state.lock().await.mode_id = req.mode_id.clone(); @@ -639,8 +649,9 @@ async fn drive_prompt( // Snapshot the inputs under the session lock, then drop the lock // before any `await` that touches the network. `mode_id` is - // refreshed between tool rounds (the user can toggle modes - // mid-turn). + // refreshed at the top of every round (the user can toggle modes + // mid-turn and we want the next round's streaming + tool gating + // to reflect that). let (existing_history, model_id, cwd, cancel, mut mode_id) = { let mut state = session_arc.lock().await; // Fire the session's current cancel before installing a new @@ -668,8 +679,10 @@ async fn drive_prompt( }; let tool_specs = tools::all_tools(); - let system_prompt = build_system_prompt(&cwd, inner.system_prompt_path.as_deref(), &tool_specs) - .map_err(|e| anyhow::anyhow!("build system prompt: {e:#}"))?; + // Plan-mode write target. Resolved once because the cwd doesn't + // change for a session's lifetime; the directory is created + // lazily by the runtime when a write lands inside it. + let plan_dir = store::plan_dir_for(&cwd); let (provider, local_model) = match resolve_provider(&inner.providers, &inner.default_endpoint_name, &model_id) { @@ -692,14 +705,14 @@ async fn drive_prompt( let ops = AcpClientOps::new(cx.clone()); // `messages` is the rolling conversation we send to the provider - // each round. We seed it with the system prompt + the snapshot - // (which includes the new user turn) and grow it with each - // round's assistant turn + tool-result turns. + // each round. Slot 0 is the system prompt — rebuilt at the top + // of every round so a mid-turn mode toggle takes effect. We seed + // a placeholder here and overwrite it on the first iteration. let mut messages: Vec = Vec::with_capacity(existing_history.len() + 1); messages.push(Message { role: Role::System, content: MessageContent::Text { - text: system_prompt, + text: String::new(), }, }); messages.extend(existing_history); @@ -716,18 +729,43 @@ async fn drive_prompt( let mut stop_reason = StopReason::EndTurn; for round in 0..MAX_TOOL_ROUNDS { - tracing::info!( - session_id = %session_id.0, - round = round + 1, - of = MAX_TOOL_ROUNDS, - history_turns = messages.len(), - "prompt round: streaming" - ); if cancel.is_cancelled() { stop_reason = StopReason::Cancelled; break; } + // Refresh mode + rebuild system prompt at the top of every + // round. Cheap (one mutex acquisition + one string build); + // the win is that if the user flips the mode dropdown + // mid-turn — particularly the Plan ↔ Bypass transitions + // the plan-mode menu invites them to make — the new mode + // gates both this round's streaming *and* its tool + // dispatch. + mode_id = session_arc.lock().await.mode_id.clone(); + let system_prompt = build_system_prompt( + &cwd, + inner.system_prompt_path.as_deref(), + &tool_specs, + &mode_id, + plan_dir.as_deref(), + ) + .map_err(|e| anyhow::anyhow!("build system prompt: {e:#}"))?; + messages[0] = Message { + role: Role::System, + content: MessageContent::Text { + text: system_prompt, + }, + }; + + tracing::info!( + session_id = %session_id.0, + round = round + 1, + of = MAX_TOOL_ROUNDS, + mode = %mode_id.0, + history_turns = messages.len(), + "prompt round: streaming" + ); + // Tool descriptions reach the model via the Qwen3 `# Tools` // block in the system prompt, not via the OpenAI `tools` // request field — cortex/neuron pass that field through to @@ -905,10 +943,6 @@ async fn drive_prompt( messages.push(assistant_turn); } - // Refresh the mode in case the user toggled it during the - // streaming above (cheap — one mutex acquisition). - mode_id = session_arc.lock().await.mode_id.clone(); - // Dispatch every tool call sequentially. Parallelism is // tempting but would require Zed to handle interleaved // permission prompts; serial is friendlier. diff --git a/crates/helexa-acp/src/prompt.rs b/crates/helexa-acp/src/prompt.rs index 4b9b1b3..dc4cf06 100644 --- a/crates/helexa-acp/src/prompt.rs +++ b/crates/helexa-acp/src/prompt.rs @@ -12,11 +12,13 @@ //! OpenAI `tools` API field, so the tool list has to live in the //! prompt itself. +use agent_client_protocol::schema::SessionModeId; use anyhow::Context; use std::path::Path; use crate::provider::ToolSpec; use crate::qwen3; +use crate::session::MODE_PLAN; const DEFAULT_PROMPT: &str = "\ You are helexa-acp, a coding assistant working inside an editor. @@ -41,10 +43,21 @@ Be concise; the user is reading your output in an editor pane."; /// still gets the tool descriptions the model needs. /// - `tools`: the tools to advertise. Empty list → no `# Tools` /// block is appended at all. +/// - `mode`: current session mode. When the mode is [`MODE_PLAN`] +/// a plan-mode addendum describing the restrictions and the +/// completion menu is appended *after* the `# Tools` block so it +/// is the last thing the model reads before user input. +/// - `plan_dir`: resolved plan directory for the cwd. Only consulted +/// when `mode == MODE_PLAN`. `None` means the plan directory could +/// not be resolved (no `HOME` / `XDG_DATA_HOME`) — the addendum +/// still renders but with a placeholder so the model knows to +/// surface the error to the user rather than guess a path. pub fn build_system_prompt( cwd: &Path, override_path: Option<&Path>, tools: &[ToolSpec], + mode: &SessionModeId, + plan_dir: Option<&Path>, ) -> anyhow::Result { let template = match override_path { Some(path) => std::fs::read_to_string(path) @@ -53,17 +66,81 @@ pub fn build_system_prompt( }; let mut prompt = template.replace("{cwd}", &cwd.display().to_string()); prompt.push_str(&qwen3::render_tool_block(tools)); + if mode.0.as_ref() == MODE_PLAN { + prompt.push_str(&render_plan_mode_block(plan_dir)); + } Ok(prompt) } +/// Plan-mode instruction block. Tells the model: +/// +/// 1. Where it may write — only inside `plan_dir`. +/// 2. What it may *not* do — bash is disabled; writes outside +/// `plan_dir` are refused by the runtime. +/// 3. How to finish — emit the 3-option menu so the user can +/// switch modes and either kick off implementation (with or +/// without permission prompts) or keep iterating on the plan. +fn render_plan_mode_block(plan_dir: Option<&Path>) -> String { + let plan_path = plan_dir + .map(|p| p.display().to_string()) + .unwrap_or_else(|| "".to_string()); + format!( + "\n\n# Plan mode\n\ + \n\ + You are in **plan mode**. Your task is to draft a written\n\ + implementation plan for the user; you must NOT modify any\n\ + project files or run shell commands.\n\ + \n\ + Rules in plan mode:\n\ + \n\ + - `read_file` and `list_dir` are unrestricted — use them to\n\ + explore the codebase as needed.\n\ + - `write_file` and `edit_file` are allowed ONLY under the\n\ + plan directory: `{plan_path}`. The runtime will refuse any\n\ + write outside it.\n\ + - `bash` is disabled. Do not call it.\n\ + \n\ + Write the plan as one or more Markdown files under\n\ + `{plan_path}`. Use descriptive filenames\n\ + (`01-overview.md`, `02-data-model.md`, etc.). It is fine to\n\ + iterate — overwrite the file when you refine a section.\n\ + \n\ + When the plan is complete, do NOT begin implementation.\n\ + Instead, end your turn with this menu, verbatim, so the\n\ + user can choose how to proceed:\n\ + \n\ + ---\n\ + **Plan complete.** To proceed, switch the session mode in\n\ + the agent dropdown and send a follow-up message:\n\ + \n\ + 1. **Bypass Permissions** — implement the plan now, skipping\n\ + per-tool permission prompts.\n\ + 2. **Default** — implement the plan now, prompting before\n\ + each write or shell command.\n\ + 3. **Plan** (stay here) — refine the plan; reply with the\n\ + change you want and I will revise it.\n\ + ---\n" + ) +} + #[cfg(test)] mod tests { use super::*; + use crate::session::{MODE_DEFAULT, MODE_PLAN}; use std::io::Write; + fn default_mode() -> SessionModeId { + SessionModeId::new(MODE_DEFAULT) + } + fn plan_mode() -> SessionModeId { + SessionModeId::new(MODE_PLAN) + } + #[test] fn default_prompt_substitutes_cwd() { - let prompt = build_system_prompt(Path::new("/home/me/proj"), None, &[]).unwrap(); + let prompt = + build_system_prompt(Path::new("/home/me/proj"), None, &[], &default_mode(), None) + .unwrap(); assert!( prompt.contains("/home/me/proj"), "cwd not interpolated: {prompt}" @@ -75,6 +152,8 @@ mod tests { ); // With no tools, the # Tools block is absent. assert!(!prompt.contains("# Tools")); + // Default mode does not get the plan-mode addendum. + assert!(!prompt.contains("# Plan mode")); } #[test] @@ -84,7 +163,8 @@ mod tests { description: "Read a file.".into(), parameters: serde_json::json!({"type":"object","properties":{}, "required":[]}), }; - let prompt = build_system_prompt(Path::new("/x"), None, &[spec]).unwrap(); + let prompt = + build_system_prompt(Path::new("/x"), None, &[spec], &default_mode(), None).unwrap(); assert!(prompt.contains("# Tools")); assert!(prompt.contains("")); assert!(prompt.contains("\"name\":\"read_file\"")); @@ -100,8 +180,14 @@ mod tests { let path = tmp.path().to_path_buf(); drop(tmp); - let prompt = build_system_prompt(Path::new("/etc"), Some(path.as_path()), &[]) - .expect("read override"); + let prompt = build_system_prompt( + Path::new("/etc"), + Some(path.as_path()), + &[], + &default_mode(), + None, + ) + .expect("read override"); assert_eq!(prompt, "custom prompt for /etc only"); let _ = std::fs::remove_file(&path); @@ -113,11 +199,45 @@ mod tests { Path::new("/tmp"), Some(Path::new("/definitely/not/a/real/path")), &[], + &default_mode(), + None, ) .unwrap_err(); assert!(format!("{err:#}").contains("read system prompt")); } + #[test] + fn plan_mode_addendum_includes_plan_dir_and_menu() { + let plan_dir = Path::new("/home/me/.local/share/helexa-acp/plans/proj-deadbeef"); + let prompt = build_system_prompt( + Path::new("/home/me/proj"), + None, + &[], + &plan_mode(), + Some(plan_dir), + ) + .unwrap(); + assert!(prompt.contains("# Plan mode")); + assert!( + prompt.contains(plan_dir.to_str().unwrap()), + "plan dir not interpolated: {prompt}" + ); + // The 3-option menu must be present so the model emits it verbatim. + assert!(prompt.contains("Bypass Permissions")); + assert!(prompt.contains("**Default**")); + assert!(prompt.contains("3. **Plan**")); + // Bash disabled instruction must be present. + assert!(prompt.contains("`bash` is disabled")); + } + + #[test] + fn plan_mode_addendum_handles_unresolved_plan_dir() { + let prompt = + build_system_prompt(Path::new("/home/me/proj"), None, &[], &plan_mode(), None).unwrap(); + assert!(prompt.contains("# Plan mode")); + assert!(prompt.contains("could not be resolved")); + } + /// Tiny temp-file helper that doesn't pull in the `tempfile` crate. /// Writes under `target/` so it's cleaned up by `cargo clean`. fn tempfile_in_target(name: &str) -> TempHandle { diff --git a/crates/helexa-acp/src/session.rs b/crates/helexa-acp/src/session.rs index aa3931e..942176e 100644 --- a/crates/helexa-acp/src/session.rs +++ b/crates/helexa-acp/src/session.rs @@ -32,6 +32,13 @@ pub const MODE_DEFAULT: &str = "default"; /// favorite name (`bypassPermissions`) Zed clients tend to reference. pub const MODE_BYPASS: &str = "bypassPermissions"; +/// Mode id for read-and-plan-only operation. The model may read files +/// and list directories freely, may write *only* into the per-project +/// plan directory under `$XDG_DATA_HOME/helexa-acp/plans//`, +/// and cannot run shell commands. Designed for "draft the +/// implementation plan, then I'll review and let you execute" flows. +pub const MODE_PLAN: &str = "plan"; + /// State carried for a single ACP session. /// /// Mutated under `Mutex`; never share a clone across diff --git a/crates/helexa-acp/src/store.rs b/crates/helexa-acp/src/store.rs index 8c5acdd..ff0acde 100644 --- a/crates/helexa-acp/src/store.rs +++ b/crates/helexa-acp/src/store.rs @@ -28,6 +28,7 @@ use crate::provider::Message; const APP_DIRNAME: &str = "helexa-acp"; const SESSIONS_DIRNAME: &str = "sessions"; +const PLANS_DIRNAME: &str = "plans"; /// The shape persisted to disk for one session. Only what we can't /// rebuild from the running config goes in here: the conversation @@ -184,6 +185,63 @@ pub fn now_secs() -> u64 { .unwrap_or(0) } +/// Root directory for plan-mode artefacts. Mirrors [`sessions_dir`] +/// but under `…/helexa-acp/plans/` so plans and conversation +/// transcripts are siblings, not nested. +pub fn plans_root() -> Option { + sessions_dir().and_then(|s| s.parent().map(|p| p.join(PLANS_DIRNAME))) +} + +/// Per-project plan directory: +/// `$XDG_DATA_HOME/helexa-acp/plans//`. The id derives +/// from the session's cwd so plans for the same project survive +/// across cwd-changes (a `/home/foo/git/bar` ↔ symlinked +/// `/srv/checkout/bar` would technically diverge, accepted as a +/// won't-fix corner case). +pub fn plan_dir_for(cwd: &std::path::Path) -> Option { + plans_root().map(|root| root.join(project_id_for(cwd))) +} + +/// Deterministic, human-readable project identifier. Format: +/// `-<8-hex>` where the 8-hex suffix is FNV-1a of the +/// full path. Basename keeps the path skim-readable when poking +/// around `$XDG_DATA_HOME` by hand; the hash suffix disambiguates +/// repos that share a final path component (e.g. multiple +/// `/.../checkout/beat` checkouts). +/// +/// FNV-1a rather than `std::collections::hash::DefaultHasher` +/// because the latter (SipHash) reseeds per process, so it'd give +/// us a different project_id on every run. +pub fn project_id_for(cwd: &std::path::Path) -> String { + let basename = cwd + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or("unknown"); + let sanitised: String = basename + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '_' + } + }) + .collect(); + let hash = fnv1a_32(cwd.to_string_lossy().as_bytes()); + format!("{sanitised}-{hash:08x}") +} + +/// FNV-1a (32-bit). Deterministic, no third-party crate. Used for +/// project ids only — not cryptographic. +fn fnv1a_32(bytes: &[u8]) -> u32 { + let mut h: u32 = 0x811c_9dc5; + for b in bytes { + h ^= u32::from(*b); + h = h.wrapping_mul(0x0100_0193); + } + h +} + /// Format seconds-since-epoch as an ISO 8601 / RFC 3339 string /// (`YYYY-MM-DDTHH:MM:SSZ`) for `SessionInfo.updated_at`. Returns /// `None` for values outside the representable range, in which diff --git a/crates/helexa-acp/src/tool_runner.rs b/crates/helexa-acp/src/tool_runner.rs index 2fa5c79..18019c4 100644 --- a/crates/helexa-acp/src/tool_runner.rs +++ b/crates/helexa-acp/src/tool_runner.rs @@ -37,7 +37,8 @@ use serde::Deserialize; use serde_json::json; use tokio_util::sync::CancellationToken; -use crate::session::{MODE_BYPASS, MODE_DEFAULT}; +use crate::session::{MODE_BYPASS, MODE_DEFAULT, MODE_PLAN}; +use crate::store; use crate::tools::{BASH, EDIT_FILE, LIST_DIR, READ_FILE, WRITE_FILE}; /// Accumulated state of a single tool call streamed from the @@ -408,8 +409,61 @@ pub async fn dispatch_tool_call( ); } - // ── Permission gate ────────────────────────────────────────────── - if is_gated(&call.name) && mode.0.as_ref() != MODE_BYPASS { + // ── Plan-mode gate ─────────────────────────────────────────────── + // Plan mode is the most restrictive: bash is disabled outright, + // writes are confined to the plan directory, and there is no + // permission prompt (writes inside plan_dir auto-allow because + // writing the plan IS the whole purpose). Reads pass through. + if mode.0.as_ref() == MODE_PLAN { + let plan_dir = store::plan_dir_for(session_cwd); + match call.name.as_str() { + BASH => { + return finish_failed( + ops, + session_id, + &tool_call_id, + &call.id, + "plan mode: shell execution is disabled. Switch to Default or \ + Bypass Permissions to run commands.", + ); + } + WRITE_FILE | EDIT_FILE => { + let path = args_value + .get("path") + .and_then(|v| v.as_str()) + .map(std::path::PathBuf::from); + let inside_plan_dir = match (path.as_deref(), plan_dir.as_deref()) { + (Some(p), Some(pd)) => p.starts_with(pd), + _ => false, + }; + if !inside_plan_dir { + let plan_dir_str = plan_dir + .as_ref() + .map(|p| p.display().to_string()) + .unwrap_or_else(|| "".to_string()); + let attempted = path + .as_ref() + .map(|p| p.display().to_string()) + .unwrap_or_else(|| "".to_string()); + return finish_failed( + ops, + session_id, + &tool_call_id, + &call.id, + &format!( + "plan mode: writes are restricted to {plan_dir_str}; \ + refused write to {attempted}." + ), + ); + } + // Inside plan_dir: skip the permission prompt and + // fall through to execution. + } + _ => { + // read_file, list_dir: allowed without prompt. + } + } + } else if is_gated(&call.name) && mode.0.as_ref() != MODE_BYPASS { // Default mode (or any non-bypass id): always ask. The user's // "Allow" decision is per-call here; we don't carry over an // "Allow always" across calls — that's a Stage 7 polish item @@ -926,6 +980,9 @@ mod tests { fn mode_bypass() -> SessionModeId { SessionModeId::new(MODE_BYPASS) } + fn mode_plan() -> SessionModeId { + SessionModeId::new(MODE_PLAN) + } fn make_call(name: &str, args: serde_json::Value) -> ToolCallEvent { ToolCallEvent { @@ -1083,4 +1140,125 @@ mod tests { assert!(is_gated(EDIT_FILE)); assert!(is_gated(BASH)); } + + // ── Plan-mode gating ──────────────────────────────────────────── + + #[tokio::test] + async fn plan_mode_refuses_bash() { + let fake = FakeClient::default(); + let res = dispatch_tool_call( + &fake, + &sid(), + &mode_plan(), + Path::new("/tmp"), + make_call(BASH, json!({"command": "ls"})), + &CancellationToken::new(), + ) + .await; + assert!(res.is_error, "expected error: {}", res.content); + assert!( + res.content.contains("plan mode"), + "expected plan-mode error, got: {}", + res.content + ); + let events = fake.events(); + assert!( + !events.iter().any(|e| e.starts_with("CreateTerminal")), + "bash must not run in plan mode: {events:?}" + ); + assert!( + !events.iter().any(|e| e == "RequestPermission"), + "plan mode must not prompt for bash: {events:?}" + ); + } + + #[tokio::test] + async fn plan_mode_refuses_write_outside_plan_dir() { + let fake = FakeClient::default(); + let res = dispatch_tool_call( + &fake, + &sid(), + &mode_plan(), + Path::new("/home/me/proj"), + make_call( + WRITE_FILE, + json!({"path": "/home/me/proj/src/main.rs", "content": "fn main() {}"}), + ), + &CancellationToken::new(), + ) + .await; + assert!(res.is_error, "expected error: {}", res.content); + assert!( + res.content.contains("plan mode") && res.content.contains("/home/me/proj/src/main.rs"), + "expected refusal naming attempted path, got: {}", + res.content + ); + let events = fake.events(); + assert!( + !events.iter().any(|e| e.starts_with("Write")), + "no write must happen for refused path: {events:?}" + ); + } + + #[tokio::test] + async fn plan_mode_allows_write_inside_plan_dir_without_permission() { + // Skip if we can't resolve a plan dir in this environment + // (would happen with no HOME / XDG_DATA_HOME — neither + // realistic in CI nor for an interactive run). + let Some(plan_dir) = store::plan_dir_for(Path::new("/home/me/proj")) else { + eprintln!("skipping: plan_dir unresolvable in this env"); + return; + }; + let target = plan_dir.join("01-overview.md"); + + let fake = FakeClient::default(); + let res = dispatch_tool_call( + &fake, + &sid(), + &mode_plan(), + Path::new("/home/me/proj"), + make_call( + WRITE_FILE, + json!({"path": target.to_str().unwrap(), "content": "# Overview"}), + ), + &CancellationToken::new(), + ) + .await; + assert!( + !res.is_error, + "expected success writing inside plan dir, got: {}", + res.content + ); + let events = fake.events(); + assert!( + !events.iter().any(|e| e == "RequestPermission"), + "plan mode must not prompt for in-plan-dir writes: {events:?}" + ); + assert!( + events.iter().any(|e| e.starts_with("Write")), + "expected write to land: {events:?}" + ); + } + + #[tokio::test] + async fn plan_mode_allows_read_anywhere() { + let fake = FakeClient::default(); + fake.set_read(PathBuf::from("/etc/hostname"), Ok("host".into())); + let res = dispatch_tool_call( + &fake, + &sid(), + &mode_plan(), + Path::new("/home/me/proj"), + make_call(READ_FILE, json!({"path": "/etc/hostname"})), + &CancellationToken::new(), + ) + .await; + assert!(!res.is_error, "result: {}", res.content); + assert_eq!(res.content, "host"); + let events = fake.events(); + assert!( + !events.iter().any(|e| e == "RequestPermission"), + "reads in plan mode must not prompt: {events:?}" + ); + } }