From 09d4cc088a7a0fc98a599cbbecd7c0232cdde4fa Mon Sep 17 00:00:00 2001 From: Daniels-Main Date: Sun, 6 Sep 2026 16:49:44 +0200 Subject: [PATCH] feat: add personal repository ref and file actions --- Cargo.lock | 1 + README.md | 3 + ROADMAP.md | 13 +- TASKS.md | 5 +- crates/strand-core/Cargo.toml | 3 + crates/strand-core/src/lib.rs | 1 + crates/strand-core/src/user_actions.rs | 253 ++++++++++++ crates/strand-tauri/src/ai/bin.rs | 48 ++- crates/strand-tauri/src/commands.rs | 36 ++ crates/strand-tauri/src/main.rs | 4 + crates/strand-tauri/src/user_actions.rs | 383 ++++++++++++++++++ docs/learnings.md | 22 + ui/src/App.tsx | 20 +- ui/src/components/RepoRail.tsx | 4 + ui/src/components/RepoTabs.tsx | 4 + ui/src/components/RepositoryFiles.tsx | 2 + ui/src/components/Sidebar.tsx | 13 +- ui/src/lib/tauri.ts | 8 + ui/src/lib/userActions.test.ts | 54 +++ ui/src/lib/userActions.ts | 88 ++++ ui/src/stores/repo.test.ts | 9 + ui/src/stores/repo.ts | 3 +- ui/src/stores/settings.ts | 3 + ui/src/styles/user-actions.css | 13 + ui/src/views/SettingsDialog.tsx | 6 +- ui/src/views/UserActionDialog.tsx | 130 ++++++ ui/src/views/settings/IntegrationsSection.tsx | 4 +- ui/src/views/settings/UserActionsEditor.tsx | 69 ++++ website/docs/settings.md | 65 +++ 29 files changed, 1248 insertions(+), 19 deletions(-) create mode 100644 crates/strand-core/src/user_actions.rs create mode 100644 crates/strand-tauri/src/user_actions.rs create mode 100644 ui/src/lib/userActions.test.ts create mode 100644 ui/src/lib/userActions.ts create mode 100644 ui/src/styles/user-actions.css create mode 100644 ui/src/views/UserActionDialog.tsx create mode 100644 ui/src/views/settings/UserActionsEditor.tsx diff --git a/Cargo.lock b/Cargo.lock index 4ea18990..fec1e649 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5927,6 +5927,7 @@ dependencies = [ "notify", "serde", "serde_json", + "tempfile", "thiserror 1.0.69", "tracing", ] diff --git a/README.md b/README.md index 40ac8df7..61dafe68 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,9 @@ the resolved app appearance automatically. ## Features +- **Personal user actions** — define repository, ref, and working-tree file + commands in Settings → Integrations. Menu and palette entries open an exact + executable/argument/working-directory preview, with bounded output and cancellation. - **Responsive refreshes** — repository updates coalesce during bursts of agent edits, hidden diff panes load patches when opened, and Files reuses its inventory until paths or ignore rules change. Workspace scans run with diff --git a/ROADMAP.md b/ROADMAP.md index f048b0bf..bd253556 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2165,7 +2165,8 @@ and Store certification remain external gates. - Sparse checkout (cone mode first) - Patch import/mailbox and Git bundle workflows - Expanded submodule lifecycle (add/remove/deinit/sync/URL/nested status) -- Repository/ref/file custom actions with safe argv templates +- ☑ Repository/ref/file custom actions with safe argv templates + (`UserActionsEditor`, context menus / Quick Launch, `UserActionDialog`) - **CLI companion binary (`strand`)** — `strand ` opens the repo in the app; `strand diff/log/status/review --json` gives AI agents typed, full-context data the `git` porcelain can't (same serde types @@ -2808,6 +2809,16 @@ implementation rows while the July audit is labeled historical. This is a planning update, not a claim that these features shipped; existing local Git, GitHub/Azure review, Workbench and performance work retain their own status. +**Personal user actions shipped (2026-09-06, F15):** Settings → Integrations +now edits explicit executable/argv definitions for repositories, qualified refs, +and working-tree files. Context menus and Quick Launch capture the target and +require a resolved executable/arguments/cwd preview. Native execution revalidates +paths and ref IDs, preserves argv boundaries, bounds both output streams, and +cancels the process tree. Definitions stay in personal settings, separate from +Workbench and plugins. Automated tests and an isolated Windows WebView2 pass +covered literal spaces/metacharacters, stale selections, exact menu targets, +nonzero exits, output limits, keyboard operation, and cancellation. + --- ## Cross-cutting tracks (run in parallel with all milestones) diff --git a/TASKS.md b/TASKS.md index 4c7910bf..848252a6 100644 --- a/TASKS.md +++ b/TASKS.md @@ -120,9 +120,10 @@ Detailed comparison and sequencing: [`docs/git-client-1.0-audit.md`](./docs/git- - ☐ **F14 / P2 — Publish a new hosted repository.** Provider/account/visibility selection, concrete destination review, remote configuration and explicit initial push, with recovery from partial failure. -- ☐ **F15 / P2 — User-defined repository/ref/file actions.** Safe executable/ +- ☑ **F15 / P2 — User-defined repository/ref/file actions.** Safe executable/ argv templates, exact context, palette/menu discovery, preview, bounded output - and cancellation; editor/terminal templates and internal registries already exist. + and cancellation. (`UserActionsEditor`, `UserActionDialog`, + `repo_user_action_preview` / `repo_user_action_run`; personally persisted settings.) - ☐ **F18 / P3 — Advanced refs.** Git notes/replace-ref management and explicit tag retarget/re-annotation with current/new target review. Signed tags are F03; existing local Review notes are separate from Git notes. diff --git a/crates/strand-core/Cargo.toml b/crates/strand-core/Cargo.toml index ac677e40..a2953b71 100644 --- a/crates/strand-core/Cargo.toml +++ b/crates/strand-core/Cargo.toml @@ -13,3 +13,6 @@ serde.workspace = true serde_json.workspace = true thiserror.workspace = true tracing.workspace = true + +[dev-dependencies] +tempfile = "3" diff --git a/crates/strand-core/src/lib.rs b/crates/strand-core/src/lib.rs index e2dc385a..9471711e 100644 --- a/crates/strand-core/src/lib.rs +++ b/crates/strand-core/src/lib.rs @@ -30,6 +30,7 @@ pub mod remote; pub mod maintenance; pub mod conflict; pub mod external; +pub mod user_actions; pub mod gitconfig; pub mod history; pub mod ignore; diff --git a/crates/strand-core/src/user_actions.rs b/crates/strand-core/src/user_actions.rs new file mode 100644 index 00000000..763cfe46 --- /dev/null +++ b/crates/strand-core/src/user_actions.rs @@ -0,0 +1,253 @@ +//! Personally configured actions. Argument boundaries exist before substitution; +//! repository values are never parsed as command syntax or substituted twice. +use serde::{Deserialize, Serialize}; + +use crate::{Error, Repo, Result}; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct UserAction { + pub id: String, + pub name: String, + pub scope: String, + pub executable: String, + pub args: Vec, + pub cwd: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(tag = "kind", rename_all = "lowercase")] +pub enum ActionTarget { + Repository, + Ref { reference: String, oid: String }, + File { file: String }, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct ActionContext { + pub path: String, + pub target: ActionTarget, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct ActionPreview { + pub executable: String, + pub args: Vec, + pub cwd: String, +} + +fn invalid(message: &str) -> Error { + Error::Other(message.into()) +} + +/// Expand only template text. Escaped braces allow literal script/JSON arguments. +fn substitute(template: &str, vars: &[(&str, String)]) -> Result { + let mut out = String::new(); + let mut rest = template; + while !rest.is_empty() { + if rest.starts_with("{{") { + out.push('{'); + rest = &rest[2..]; + } else if rest.starts_with("}}") { + out.push('}'); + rest = &rest[2..]; + } else if rest.starts_with('{') { + let end = rest + .find('}') + .ok_or_else(|| invalid("Unclosed action placeholder"))?; + let key = &rest[1..end]; + let value = vars + .iter() + .find(|(name, _)| *name == key) + .ok_or_else(|| invalid(&format!("Unavailable action placeholder: {{{key}}}")))?; + out.push_str(&value.1); + rest = &rest[end + 1..]; + } else { + let ch = rest.chars().next().unwrap(); + out.push(ch); + rest = &rest[ch.len_utf8()..]; + } + } + Ok(out) +} + +impl Repo { + pub fn preview_user_action( + &self, + action: &UserAction, + context: &ActionContext, + ) -> Result { + if action.name.trim().is_empty() + || action.name.len() > 120 + || action.args.len() > 128 + || action.executable.trim().is_empty() + || action.executable.contains('\0') + || action.executable.len() > 4096 + || action.args.iter().map(String::len).sum::() > 24_000 + || action.args.iter().any(|arg| arg.contains('\0')) + { + return Err(invalid("Invalid action: use a name, a literal executable, and at most 128 arguments / 24 KB")); + } + let root = self.path().canonicalize()?; + let mut cwd = root.clone(); + let mut vars = vec![("repo", root.to_string_lossy().into_owned())]; + match &context.target { + ActionTarget::Repository if action.scope == "repository" => {} + ActionTarget::Ref { reference, oid } if action.scope == "ref" => { + if !reference.starts_with("refs/") { + return Err(invalid("Select a qualified branch or tag ref")); + } + let current = self + .git2()? + .find_reference(reference)? + .peel_to_commit()? + .id() + .to_string(); + if ¤t != oid { + return Err(invalid( + "Selected ref changed. Close this preview and select it again.", + )); + } + vars.extend([("ref", reference.clone()), ("oid", current)]); + } + ActionTarget::File { file } if action.scope == "file" => { + let full = self.workdir_path(file)?.canonicalize()?; + if !full.is_file() { + return Err(invalid("Select an existing working-tree file")); + } + // A replaced symlink must never redirect a preview outside this checkout. + if !full.starts_with(&root) { + return Err(invalid("File escapes the working tree")); + } + if action.cwd == "file-parent" { + cwd = full.parent().unwrap().to_owned(); + } + vars.extend([ + ("file", full.to_string_lossy().into_owned()), + ("relativeFile", file.clone()), + ]); + } + _ => return Err(invalid("Action scope does not match the selected context")), + } + if action.cwd != "repository" && !(action.cwd == "file-parent" && action.scope == "file") { + return Err(invalid( + "Working directory must be the repository or selected file's parent", + )); + } + let mut args = Vec::new(); + if std::path::Path::new(&action.executable) + .file_stem() + .is_some_and(|name| name.eq_ignore_ascii_case("git")) + { + args.extend(crate::GIT_SAFE_CONFIG.iter().map(|arg| arg.to_string())); + } + for arg in &action.args { + args.push(substitute(arg, &vars)?); + } + if args.iter().map(String::len).sum::() > 28_000 { + return Err(invalid("Resolved arguments exceed 28 KB")); + } + Ok(ActionPreview { + executable: action.executable.clone(), + args, + cwd: cwd.to_string_lossy().into_owned(), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn action(scope: &str, args: &[&str]) -> UserAction { + UserAction { + id: "test".into(), + name: "Test".into(), + scope: scope.into(), + executable: "probe".into(), + args: args.iter().map(|s| s.to_string()).collect(), + cwd: "repository".into(), + } + } + + #[test] + fn substitution_preserves_boundaries_and_does_not_reexpand_values() { + let temp = tempfile::tempdir().unwrap(); + let dir = temp.path().join("repo space & %PATH% {oid}"); + std::fs::create_dir(&dir).unwrap(); + git2::Repository::init(&dir).unwrap(); + let file = "file & %PATH% {repo}.txt"; + std::fs::write(dir.join(file), "text").unwrap(); + let repo = Repo::discover(&dir).unwrap(); + let context = ActionContext { + path: dir.to_string_lossy().into_owned(), + target: ActionTarget::File { file: file.into() }, + }; + let mut definition = action( + "file", + &["--", "{relativeFile}", "prefix={file}", "", "{{literal}}"], + ); + definition.cwd = "file-parent".into(); + definition.executable = dir.join("tool {repo}").to_string_lossy().into_owned(); + let preview = repo.preview_user_action(&definition, &context).unwrap(); + assert_eq!(preview.executable, definition.executable); + assert_eq!(preview.args[1], file); + assert!(preview.args[2].ends_with(file)); + assert_eq!(&preview.args[3..], &["", "{literal}"]); + assert_eq!(preview.cwd, dir.canonicalize().unwrap().to_string_lossy()); + definition.args = vec!["{ref}".into()]; + assert!(repo.preview_user_action(&definition, &context).is_err()); + let outside = ActionContext { + target: ActionTarget::File { + file: "../outside".into(), + }, + ..context.clone() + }; + assert!(repo + .preview_user_action(&action("file", &[]), &outside) + .is_err()); + std::fs::remove_file(dir.join(file)).unwrap(); + assert!(repo + .preview_user_action(&action("file", &[]), &context) + .is_err()); + } + + #[test] + fn stale_refs_and_wrong_scopes_are_rejected() { + let dir = tempfile::tempdir().unwrap(); + let git = git2::Repository::init(dir.path()).unwrap(); + let tree = git.treebuilder(None).unwrap().write().unwrap(); + let tree = git.find_tree(tree).unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + let oid = git + .commit(Some("refs/heads/main"), &sig, &sig, "one", &tree, &[]) + .unwrap(); + let context = ActionContext { + path: dir.path().to_string_lossy().into_owned(), + target: ActionTarget::Ref { + reference: "refs/heads/main".into(), + oid: oid.to_string(), + }, + }; + let repo = Repo::discover(dir.path()).unwrap(); + assert!(repo + .preview_user_action(&action("ref", &["{ref}", "{oid}"]), &context) + .is_ok()); + assert!(repo + .preview_user_action(&action("file", &[]), &context) + .is_err()); + git.commit( + Some("refs/heads/main"), + &sig, + &sig, + "two", + &tree, + &[&git.find_commit(oid).unwrap()], + ) + .unwrap(); + let repo = Repo::discover(dir.path()).unwrap(); + assert!(repo + .preview_user_action(&action("ref", &[]), &context) + .is_err()); + } +} diff --git a/crates/strand-tauri/src/ai/bin.rs b/crates/strand-tauri/src/ai/bin.rs index 1e1ceef4..70c19acd 100644 --- a/crates/strand-tauri/src/ai/bin.rs +++ b/crates/strand-tauri/src/ai/bin.rs @@ -498,15 +498,34 @@ fn join_stream_thread(handle: Option>) -> usize { handle.and_then(|thread| thread.join().ok()).unwrap_or(0) } -struct CapturedOutput { - text: String, - exceeded: bool, +pub(crate) struct CapturedOutput { + pub(crate) text: String, + pub(crate) exceeded: bool, } fn drain_pipe( + pipe: R, + max_bytes: usize, + output_exceeded: Arc, +) -> std::thread::JoinHandle { + drain_pipe_impl(pipe, max_bytes, output_exceeded, true) +} + +/// User-command transcripts preserve whitespace; provider parsers keep their +/// existing trimmed output through `drain_pipe`. +pub(crate) fn drain_pipe_untrimmed( + pipe: R, + max_bytes: usize, + output_exceeded: Arc, +) -> std::thread::JoinHandle { + drain_pipe_impl(pipe, max_bytes, output_exceeded, false) +} + +fn drain_pipe_impl( mut pipe: R, max_bytes: usize, output_exceeded: Arc, + trim: bool, ) -> std::thread::JoinHandle { std::thread::spawn(move || { let mut retained = Vec::new(); @@ -525,14 +544,15 @@ fn drain_pipe( } } } + let text = String::from_utf8_lossy(&retained); CapturedOutput { - text: String::from_utf8_lossy(&retained).trim().to_string(), + text: if trim { text.trim().to_string() } else { text.into_owned() }, exceeded, } }) } -fn join_pipe(handle: Option>) -> CapturedOutput { +pub(crate) fn join_pipe(handle: Option>) -> CapturedOutput { handle .and_then(|h| h.join().ok()) .unwrap_or(CapturedOutput { @@ -626,6 +646,24 @@ pub(crate) struct WindowsJob(windows_sys::Win32::Foundation::HANDLE); #[cfg(windows)] impl WindowsJob { + pub(crate) fn kill_on_close(&self) -> Result<(), String> { + use windows_sys::Win32::System::JobObjects::{ + SetInformationJobObject, JobObjectExtendedLimitInformation, + JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }; + // SAFETY: the initialized structure and live job handle match the API. + unsafe { + let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed(); + info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + if SetInformationJobObject(self.0, JobObjectExtendedLimitInformation, + (&info as *const JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), + std::mem::size_of::() as u32) == 0 { + return Err("Could not configure action cleanup on app exit".into()); + } + } + Ok(()) + } + pub(crate) fn assign(child: &Child) -> Result { use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::System::JobObjects::{AssignProcessToJobObject, CreateJobObjectW}; diff --git a/crates/strand-tauri/src/commands.rs b/crates/strand-tauri/src/commands.rs index e0ec2642..a686c35f 100644 --- a/crates/strand-tauri/src/commands.rs +++ b/crates/strand-tauri/src/commands.rs @@ -1496,6 +1496,42 @@ pub async fn repo_maintenance( result } +#[tauri::command(async)] +pub async fn repo_user_action_preview( + action: strand_core::user_actions::UserAction, + context: strand_core::user_actions::ActionContext, +) -> CmdResult { + run_blocking("user action preview", move || { + crate::user_actions::preview(&action, &context).map_err(|message| CmdError { message }) + }).await +} + +#[tauri::command(async)] +pub async fn repo_user_action_run( + action: strand_core::user_actions::UserAction, + context: strand_core::user_actions::ActionContext, + preview: strand_core::user_actions::ActionPreview, + op_id: String, + on_started: Channel<()>, + state: State<'_, AppState>, +) -> CmdResult { + let cancel = ai::bin::AiCancelHandle::new(); + { + let mut ops = state.ops.lock().map_err(|_| CmdError { message: "operation registry unavailable".into() })?; + if ops.contains_key(&op_id) { return Err(CmdError { message: "Action is already running".into() }); } + ops.insert(op_id.clone(), OperationCancelHandle::Ai(cancel.clone())); + } + // UI replays an early cancellation after this registration handshake. + let _ = on_started.send(()); + let result = run_blocking("user action", move || { + let current = crate::user_actions::preview(&action, &context).map_err(|message| CmdError { message })?; + if current != preview { return Err(CmdError { message: "Action context or executable changed. Preview again before running.".into() }); } + crate::user_actions::run(¤t, &cancel).map_err(|message| CmdError { message }) + }).await; + deregister_op(&state, &Some(op_id)); + result +} + #[tauri::command(async)] pub fn repo_tag_create( path: String, diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index 0b3fad89..be314e68 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -10,6 +10,7 @@ mod path_env; mod pull_requests; mod state; mod terminal; +mod user_actions; use tauri::Manager; @@ -274,6 +275,8 @@ fn main() { commands::repo_remote_set_urls, commands::repo_remote_set_default, commands::repo_maintenance, + commands::repo_user_action_preview, + commands::repo_user_action_run, commands::repo_tag_create, commands::repo_tag_delete, commands::repo_tag_push, @@ -369,6 +372,7 @@ fn main() { tauri::RunEvent::Exit | tauri::RunEvent::ExitRequested { .. } ) { app.state::().terminals.close_all(None); + user_actions::shutdown(); } }); } diff --git a/crates/strand-tauri/src/user_actions.rs b/crates/strand-tauri/src/user_actions.rs new file mode 100644 index 00000000..e7473615 --- /dev/null +++ b/crates/strand-tauri/src/user_actions.rs @@ -0,0 +1,383 @@ +//! Explicit user commands, separate from Workbench/plugin registries. +use crate::ai::bin::{self, AiCancelHandle}; +use serde::Serialize; +use std::{ + path::Path, + process::Stdio, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, + time::{Duration, Instant}, +}; +use strand_core::{ + user_actions::{ActionContext, ActionPreview, UserAction}, + Repo, +}; + +const OUTPUT_LIMIT: usize = 128 * 1024; // Per pipe; never buffer an unbounded transcript. +const TIMEOUT: Duration = Duration::from_secs(600); + +#[cfg(unix)] +static LIVE_GROUPS: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); + +/// App exit must stop action descendants as well as embedded terminals. +pub fn shutdown() { + #[cfg(unix)] + if let Ok(groups) = LIVE_GROUPS.lock() { + for pid in groups.iter() { + // SAFETY: these are only process groups created by action commands. + unsafe { + libc::kill(-(*pid as i32), libc::SIGKILL); + } + } + } + // On Windows, KILL_ON_JOB_CLOSE handles app exit (including a crash). +} + +pub fn preview(action: &UserAction, context: &ActionContext) -> Result { + let mut preview = Repo::discover(&context.path) + .map_err(|e| e.to_string())? + .preview_user_action(action, context) + .map_err(|e| e.to_string())?; + let path = Path::new(&action.executable); + if !path.is_absolute() && (action.executable.contains(['/', '\\']) || action.executable == ".") + { + return Err("Use an absolute executable path or a command installed on PATH".into()); + } + #[cfg(windows)] + let name = if path + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("exe")) + { + &action.executable[..action.executable.len() - 4] + } else { + &action.executable + }; + #[cfg(not(windows))] + let name = &action.executable; + let executable = bin::resolve_cli( + name, + path.is_absolute().then_some(action.executable.as_str()), + ) + .ok_or_else(|| { + "Executable not found. Use an installed command or absolute executable path.".to_string() + })?; + // Batch shims introduce cmd.exe reparsing of repository-controlled values. + // Users can invoke a native interpreter with a script path as an argument. + #[cfg(windows)] + if !executable + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("exe")) + { + return Err("Actions require a native .exe on Windows; pass scripts as arguments to their interpreter.".into()); + } + preview.executable = executable.to_string_lossy().into_owned(); + Ok(preview) +} + +#[derive(Debug, Serialize)] +pub struct ActionOutcome { + pub stdout: String, + pub stderr: String, + pub exit_code: Option, + pub status: String, + pub truncated: bool, + pub duration_ms: u64, +} + +pub fn run(preview: &ActionPreview, cancel: &AiCancelHandle) -> Result { + capture(preview, cancel, TIMEOUT) +} + +fn capture( + preview: &ActionPreview, + cancel: &AiCancelHandle, + timeout: Duration, +) -> Result { + if cancel.is_cancelled() { + return Err("cancelled".into()); + } + let mut command = bin::base_command(Path::new(&preview.executable), true); + command + .args(&preview.args) + .current_dir(&preview.cwd) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + capture_command(command, cancel, timeout) +} + +fn capture_command( + mut command: std::process::Command, + cancel: &AiCancelHandle, + timeout: Duration, +) -> Result { + let start = Instant::now(); + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + command.process_group(0); + } + let mut child = command + .spawn() + .map_err(|e| format!("Could not start action: {e}"))?; + #[cfg(windows)] + let job = match bin::WindowsJob::assign(&child).and_then(|job| { + job.kill_on_close()?; + Ok(job) + }) { + Ok(job) => job, + Err(error) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(error); + } + }; + #[cfg(unix)] + if let Ok(mut groups) = LIVE_GROUPS.lock() { + groups.push(child.id()); + } + let exceeded = Arc::new(AtomicBool::new(false)); + let stdout = child + .stdout + .take() + .map(|pipe| bin::drain_pipe_untrimmed(pipe, OUTPUT_LIMIT, exceeded.clone())); + let stderr = child + .stderr + .take() + .map(|pipe| bin::drain_pipe_untrimmed(pipe, OUTPUT_LIMIT, exceeded.clone())); + let (status, exit_code) = loop { + if cancel.is_cancelled() { + break ("cancelled", None); + } + if exceeded.load(Ordering::Acquire) { + break ("output-limit", None); + } + if start.elapsed() >= timeout { + break ("timed-out", None); + } + match child.try_wait() { + Ok(Some(exit)) => { + break ( + if exit.success() { + "completed" + } else { + "failed" + }, + exit.code(), + ) + } + Ok(None) => std::thread::sleep(Duration::from_millis(25)), + Err(_) => break ("failed", None), + } + }; + // Kill descendants even after a natural parent exit: they may still hold + // stdout/stderr open. Joining reader threads first can hang cancellation. + #[cfg(unix)] + bin::kill_process_tree(&mut child); + #[cfg(windows)] + bin::kill_process_tree(&mut child, &job); + let _ = child.wait(); + #[cfg(unix)] + if let Ok(mut groups) = LIVE_GROUPS.lock() { + groups.retain(|pid| *pid != child.id()); + } + let stdout = bin::join_pipe(stdout); + let stderr = bin::join_pipe(stderr); + let truncated = stdout.exceeded || stderr.exceeded; + Ok(ActionOutcome { + stdout: stdout.text, + stderr: stderr.text, + exit_code, + status: if truncated && status != "cancelled" { + "output-limit" + } else { + status + } + .into(), + truncated, + duration_ms: start.elapsed().as_millis() as u64, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + // A child of the test executable provides real argv/pipe/process behavior + // on every platform, without shell quoting or installed test runtimes. + #[test] + fn action_child() { + let Ok(mode) = std::env::var("STRAND_ACTION_TEST") else { + return; + }; + match mode.as_str() { + "echo" => { + println!("{:?}", std::env::args().collect::>()); + println!("cwd={}", std::env::current_dir().unwrap().display()); + eprintln!("stderr retained"); + } + "fail" => { + eprintln!("intentional failure"); + std::process::exit(7); + } + "flood" => loop { + println!("{}", "x".repeat(8192)); + }, + "wait" => { + println!("started"); + std::thread::sleep(Duration::from_secs(60)); + } + "descendant" => { + let mut child = child_command("mark"); + child.stdout(Stdio::inherit()).stderr(Stdio::inherit()); + child.spawn().unwrap(); + println!("spawned child"); + std::thread::sleep(Duration::from_secs(60)); + } + "parent-exit" => { + child_command("wait") + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .spawn() + .unwrap(); + } + "mark" => { + std::thread::sleep(Duration::from_secs(2)); + std::fs::write(std::env::var("STRAND_ACTION_MARKER").unwrap(), "escaped").unwrap(); + } + _ => panic!("bad mode"), + } + } + + fn child_command(mode: &str) -> std::process::Command { + let mut command = bin::base_command(&std::env::current_exe().unwrap(), true); + command + .args([ + "--exact", + "user_actions::tests::action_child", + "--nocapture", + ]) + .env("STRAND_ACTION_TEST", mode) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + command + } + + #[test] + fn captures_exact_arguments_working_directory_and_both_pipes() { + let temp = tempfile::tempdir().unwrap(); + let dir = temp.path().join("space & %PATH% {repo}"); + std::fs::create_dir(&dir).unwrap(); + let mut command = child_command("echo"); + let hostile = "a b;&%PATH%{repo}'$(echo)"; + command.args(["--skip", hostile]).current_dir(&dir); + let output = + capture_command(command, &AiCancelHandle::new(), Duration::from_secs(15)).unwrap(); + assert_eq!(output.status, "completed"); + assert!(output.stdout.contains(hostile), "{}", output.stdout); + assert!(output.stdout.contains("space & %PATH% {repo}")); + assert!(output.stderr.contains("stderr retained")); + let output = capture_command( + child_command("fail"), + &AiCancelHandle::new(), + Duration::from_secs(15), + ) + .unwrap(); + assert_eq!(output.exit_code, Some(7)); + assert_eq!(output.status, "failed"); + assert!(output.stderr.contains("intentional failure")); + } + + #[test] + fn bounds_output_and_timeout_and_does_not_wait_for_exited_parents_descendants() { + let output = capture_command( + child_command("flood"), + &AiCancelHandle::new(), + Duration::from_secs(15), + ) + .unwrap(); + assert_eq!(output.status, "output-limit"); + assert!(output.truncated); + assert!(output.stdout.len() <= OUTPUT_LIMIT); + let output = capture_command( + child_command("wait"), + &AiCancelHandle::new(), + Duration::from_millis(300), + ) + .unwrap(); + assert_eq!(output.status, "timed-out"); + let start = Instant::now(); + let output = capture_command( + child_command("parent-exit"), + &AiCancelHandle::new(), + Duration::from_secs(15), + ) + .unwrap(); + assert_eq!(output.status, "completed"); + assert!(start.elapsed() < Duration::from_secs(10)); + } + + #[test] + fn cancellation_stops_descendants_and_pre_cancel_never_spawns() { + let dir = tempfile::tempdir().unwrap(); + let marker = dir.path().join("marker"); + let mut command = child_command("descendant"); + command.env("STRAND_ACTION_MARKER", &marker); + let cancel = AiCancelHandle::new(); + let trigger = cancel.clone(); + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(700)); + trigger.cancel(); + }); + let output = capture_command(command, &cancel, Duration::from_secs(10)).unwrap(); + assert_eq!(output.status, "cancelled"); + assert!(output.stdout.contains("spawned child")); + std::thread::sleep(Duration::from_secs(2)); + assert!(!marker.exists()); + assert_eq!( + capture( + &ActionPreview { + executable: "missing".into(), + args: vec![], + cwd: ".".into() + }, + &cancel, + TIMEOUT + ) + .unwrap_err(), + "cancelled" + ); + } + + #[test] + fn resolves_installed_executable_before_repository_cwd_and_rejects_relative_program() { + // `git init` is delegated to core's normal creation fixture helpers elsewhere; + // this test uses this worktree's actual repository read-only. + let context = ActionContext { + path: env!("CARGO_MANIFEST_DIR").into(), + target: strand_core::user_actions::ActionTarget::Repository, + }; + let mut action = UserAction { + id: "test".into(), + name: "Test".into(), + scope: "repository".into(), + executable: std::env::current_exe() + .unwrap() + .to_string_lossy() + .into_owned(), + args: vec![], + cwd: "repository".into(), + }; + let resolved = preview(&action, &context).unwrap(); + assert!(Path::new(&resolved.executable).is_absolute()); + action.executable = if cfg!(windows) { "git.EXE" } else { "git" }.into(); + let resolved = preview(&action, &context).unwrap(); + assert!(Path::new(&resolved.executable).is_absolute()); + assert!(resolved.args.iter().any(|arg| arg == "core.fsmonitor=")); + action.executable = "./repo-program".into(); + assert!(preview(&action, &context).unwrap_err().contains("absolute")); + } +} diff --git a/docs/learnings.md b/docs/learnings.md index e50db001..6377e8a6 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -2494,3 +2494,25 @@ Pierre reads `navigator.userAgent` during module evaluation; Node 22's built-in `navigator` hid a failure on CI's Node 20. Stub browser globals and restore them after the test, while retaining real integration assertions. Reproduce this class of failure locally with `--no-experimental-global-navigator`. + +## Personal actions preserve argv and captured targets (2026-09-06) + +User actions are personal executable/argv definitions, separate from Workbench +registries and community plugins. Establish argument boundaries before +single-pass placeholder substitution; never interpolate repository-controlled +values into an implicit shell or recursively expand substituted text. Resolve +the executable before adopting the repository cwd. Windows actions require a +native executable, with script paths passed to their interpreter as arguments; +batch shims introduce another command parser. + +Menus capture the invoked repository/ref/file, including inactive repository +tabs. Palette actions require an exact active target, and every run revalidates +the resolved preview and captured ref ID. A sidebar ref click only reveals its +graph row; Enter selects the tip's commit for ref palette actions. Do not infer +a ref from HEAD or a commit shared by several branches/tags. + +Bound stdout and stderr independently. Cancel the whole process tree on close, +timeout, output overflow, or selection changes. Stop descendants even after a +natural parent exit **before** joining pipe readers: inherited stdout/stderr can +otherwise keep the reader joins blocked indefinitely. Replay early cancellation +after native operation registration to cover a closed dialog during IPC startup. diff --git a/ui/src/App.tsx b/ui/src/App.tsx index ea335854..e5d3cd04 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -95,6 +95,7 @@ import { WorktreeMergeDialog } from './views/WorktreeMergeDialog'; import { ForcePushDialog } from './views/ForcePushDialog'; import { BranchNetworkDialog, type BranchNetworkDialogMode } from './views/BranchNetworkDialog'; import { CommandPalette, type PaletteAction } from './views/Palette'; +import { USER_ACTION_EVENT, userActionPalette, type ActionRequest } from './lib/userActions'; import { RepoSwitcher } from './views/RepoSwitcher'; import type { CrashCheck, @@ -119,6 +120,7 @@ const SettingsDialog = lazy(() => import('./views/SettingsDialog').then((m) => ( const BranchCleanupDialog = lazy(() => import('./views/BranchCleanupDialog').then((m) => ({ default: m.BranchCleanupDialog }))); const RebaseEditor = lazy(() => import('./views/RebaseEditor').then((m) => ({ default: m.RebaseEditor }))); const MaintenanceDialog = lazy(() => import('./views/MaintenanceDialog').then((m) => ({ default: m.MaintenanceDialog }))); +const UserActionDialog = lazy(() => import('./views/UserActionDialog').then((m) => ({ default: m.UserActionDialog }))); const WorkspaceManagerDialog = lazy(() => import('./views/WorkspaceManagerDialog').then((m) => ({ default: m.WorkspaceManagerDialog }))); const PullRequests = lazy(() => import('./views/PullRequests').then((m) => ({ default: m.PullRequests }))); @@ -351,6 +353,13 @@ export function App() { // null = closed; otherwise which remote-management flavour (add/rename/url). const [remoteDialog, setRemoteDialog] = useState(null); const [maintenanceOpen, setMaintenanceOpen] = useState(false); + const userActions = useSettings((state) => state.userActions); + const [userActionRequest, setUserActionRequest] = useState<(ActionRequest & { key: string }) | null>(null); + useEffect(() => { + const open = (event: Event) => setUserActionRequest({ ...(event as CustomEvent).detail, key: crypto.randomUUID() }); + window.addEventListener(USER_ACTION_EVENT, open); + return () => window.removeEventListener(USER_ACTION_EVENT, open); + }, []); const [fileEntryDialog, setFileEntryDialog] = useState<{ dir: string; directory: boolean } | null>(null); // null = closed; otherwise the branch to rename. const [renameBranchDialog, setRenameBranchDialog] = useState<{ name: string } | null>(null); @@ -2019,7 +2028,9 @@ export function App() { icon: 'history', run: () => { void openByPath(r.path); }, })); - return [...base, ...repoActions, ...workspaceActions, ...recentActions]; + return [...base, { id: 'manage-user-actions', label: 'Manage user actions…', group: 'Actions', + keywords: 'custom scripts executable templates repository ref file', run: () => openSettingsAt('user-actions') } satisfies PaletteAction, + ...(paletteOpen ? userActionPalette(userActions, () => showToast('Selection changed. Open Quick Launch again.', 'error')) : []), ...repoActions, ...workspaceActions, ...recentActions]; }, [setView, selectFile, onFetch, onSync, onPull, onPush, onPushAllTags, openViaDialog, openByPath, setTheme, recents, showToast, meta, abortOperation, requestCommitSearch, requestDiffSearch, requestSuggestCommitMessage, requestSelectSinceBaseline, openInEditor, openInTerminal, openSettingsAt, @@ -2030,7 +2041,7 @@ export function App() { workspaces, activeWorkspaceId, importCodeWorkspaceFlow, pruneWorktrees, activePullRequestKey, activePullRequestFollowed, activePullRequestCanUpdateBranch, toggleActivePullRequest, customCommands, customCommandContext, - customizeWorkbench, openWorkbench, showWorkbenchWork, workbenchEditing]); + customizeWorkbench, openWorkbench, showWorkbenchWork, workbenchEditing, userActions, paletteOpen]); const surfaceRenderers = useMemo(() => new Map setMaintenanceOpen(false)} onToast={showToast} /> )} + {userActionRequest && ( + setUserActionRequest(null)} + onManage={() => { setUserActionRequest(null); openSettingsAt('user-actions'); }} /> + )} {fileEntryDialog && meta && ( )} + )} + + {running ? : <> + + + + } + }> +
+ +
Repository
{request.context.path}
Selected context
+
{target.kind === 'repository' ? 'Repository root' : target.kind === 'file' ? target.file : `${target.reference} · ${target.oid}`}
+

Commands run with your account permissions and no added shell. Unsaved editor changes are not written. Closing this dialog cancels a running action; completed changes are not undone.

+ {stale &&

Selection changed. Close this dialog and invoke the action again.

} + {error &&

{error}

} + {preview &&
+
Executable
{preview.command.executable}
Working directory
{preview.command.cwd}
+

Arguments (each numbered row is one argument)

+
    {preview.command.args.map((arg, index) =>
  1. {JSON.stringify(arg)}
  2. )}
+ {preview.command.args.length === 0 &&

No arguments.

} +
} + {running &&

Running… Output is captured when the action stops.

} + {outcome &&
+

{outcome.status} · exit {outcome.exit_code ?? '—'} · {outcome.duration_ms} ms{outcome.truncated ? ' · Output limit reached' : ''}

+

Standard output

{outcome.stdout || '(empty)'}
+

Standard error

{outcome.stderr || '(empty)'}
+
} +

128 KiB retained per output stream. Excess output or a 10-minute timeout stops the process tree. Actions are personal settings; they are never loaded from repositories or remote plugins.

+
+ + ); +} diff --git a/ui/src/views/settings/IntegrationsSection.tsx b/ui/src/views/settings/IntegrationsSection.tsx index 5d41ea83..8f46df39 100644 --- a/ui/src/views/settings/IntegrationsSection.tsx +++ b/ui/src/views/settings/IntegrationsSection.tsx @@ -1,4 +1,5 @@ import { useState } from 'react'; +import { UserActionsEditor } from './UserActionsEditor'; import { Select } from '../../components/Select'; import { @@ -17,7 +18,7 @@ import { useSettings, type ExternalTool } from '../../stores/settings'; * terminal"). Preset apps per platform plus a custom command template with * `{file}` / `{line}` / `{dir}` placeholders. */ -export function IntegrationsSection() { +export function IntegrationsSection({ focusUserActions = false }: { focusUserActions?: boolean }) { const editorTool = useSettings((s) => s.editorTool); const terminalTool = useSettings((s) => s.terminalTool); const set = useSettings((s) => s.set); @@ -48,6 +49,7 @@ export function IntegrationsSection() { return tauri.repoOpenInTerminal(path, template); }} /> + ); } diff --git a/ui/src/views/settings/UserActionsEditor.tsx b/ui/src/views/settings/UserActionsEditor.tsx new file mode 100644 index 00000000..1ccb8255 --- /dev/null +++ b/ui/src/views/settings/UserActionsEditor.tsx @@ -0,0 +1,69 @@ +import { useEffect, useRef, useState } from 'react'; +import '../../styles/user-actions.css'; +import { Select } from '../../components/Select'; +import { parseActionArgs, type UserAction } from '../../lib/userActions'; +import { useSettings } from '../../stores/settings'; + +const fresh = (): UserAction => ({ id: crypto.randomUUID(), name: '', scope: 'repository', executable: 'git', args: ['status', '--short'], cwd: 'repository' }); + +export function UserActionsEditor({ focusOnMount = false }: { focusOnMount?: boolean }) { + const actions = useSettings((state) => state.userActions); + const [draft, setDraft] = useState(fresh); + const [args, setArgs] = useState('["status", "--short"]'); + const [message, setMessage] = useState(''); + const savedRef = useRef(null); + useEffect(() => { + if (!focusOnMount) return; + const frame = requestAnimationFrame(() => { savedRef.current?.focus(); savedRef.current?.scrollIntoView({ block: 'center' }); }); + return () => cancelAnimationFrame(frame); + }, [focusOnMount]); + function edit(action: UserAction) { + setDraft({ ...action }); setArgs(JSON.stringify(action.args, null, 2)); setMessage(''); + } + function save() { + try { + if (!draft.name.trim() || !draft.executable.trim()) throw new Error('Enter a name and executable.'); + const saved = { ...draft, name: draft.name.trim(), executable: draft.executable.trim(), args: parseActionArgs(args) }; + const current = useSettings.getState().userActions; + if (current.length >= 100 && !current.some((action) => action.id === draft.id)) throw new Error('Limit of 100 saved actions reached.'); + useSettings.getState().set('userActions', [...current.filter((action) => action.id !== draft.id), saved]); + setMessage('Action saved. Run it from its context menu or Quick Launch.'); + } catch (error) { setMessage(error instanceof Error ? error.message : String(error)); } + } + return ( +
+

User actions

+

Personal executable commands for repositories, refs, or working-tree files. These are separate from Workbench commands and bundled plugins.

+ +
+
+ + + +

Installed command or absolute executable path, without quotes. On Windows, use a native .exe; pass script paths as arguments to their interpreter.

+