From eb605a3d05bd59d5013f1f678ee6062e4d166a92 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:18:23 +0100 Subject: [PATCH] feat(maestro): ship claude-code activity hook + idempotent installer --- Cargo.lock | 9 + Cargo.toml | 1 + crates/maestro-hooks/Cargo.toml | 15 ++ crates/maestro-hooks/src/lib.rs | 239 ++++++++++++++++++++++++++ crates/maestro-hooks/src/tests.rs | 268 ++++++++++++++++++++++++++++++ 5 files changed, 532 insertions(+) create mode 100644 crates/maestro-hooks/Cargo.toml create mode 100644 crates/maestro-hooks/src/lib.rs create mode 100644 crates/maestro-hooks/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 646ccfb..5db5452 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1137,6 +1137,15 @@ dependencies = [ "uuid", ] +[[package]] +name = "maestro-hooks" +version = "0.1.0" +dependencies = [ + "serde_json", + "tempfile", + "thiserror 1.0.69", +] + [[package]] name = "maestro-installer" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 7fb7deb..a15a776 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ dirs = "5" futures-util = { version = "0.3", default-features = false, features = ["sink", "std"] } libc = "0.2" maestro-config = { path = "crates/maestro-config" } +maestro-hooks = { path = "crates/maestro-hooks" } maestro-names = { path = "crates/maestro-names" } maestro-protocol = { path = "crates/maestro-protocol" } maestro-testing = { path = "crates/maestro-testing" } diff --git a/crates/maestro-hooks/Cargo.toml b/crates/maestro-hooks/Cargo.toml new file mode 100644 index 0000000..1c6b004 --- /dev/null +++ b/crates/maestro-hooks/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "maestro-hooks" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +publish.workspace = true +description = "Agent activity hooks (#106): shipped script + idempotent settings.json installer" + +[dependencies] +serde_json.workspace = true +thiserror.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/maestro-hooks/src/lib.rs b/crates/maestro-hooks/src/lib.rs new file mode 100644 index 0000000..429f317 --- /dev/null +++ b/crates/maestro-hooks/src/lib.rs @@ -0,0 +1,239 @@ +//! Claude Code activity hook (#106). +//! +//! Two halves, both shared by the `maestro hooks install` CLI subcommand and +//! the daemon's first-start auto-install: +//! +//! * [`HOOK_SCRIPT`] — the hook itself, written to disk at install time. It +//! posts an [`ActivityState`](maestro-protocol) to +//! `$MAESTRO_URL/api/sessions/$MAESTRO_SESSION_ID/activity` (#105). The same +//! global hook runs in every Claude Code session on the machine, so it exits +//! 0 and silently when either #103 env var is unset, and it never lets a +//! daemon failure escape (short curl timeout, status ignored). +//! * [`install`] — idempotent registration of that script in +//! `~/.claude/settings.json`. Only entries maestro itself wrote (identified +//! by [`HOOK_MARKER`]) are touched; every other hook and every other field +//! in the file is preserved verbatim. + +use std::path::Path; + +/// Marker substring every maestro-installed hook command carries. The +/// installer rewrites or removes exactly these entries — nothing else in the +/// user's settings file. +pub const HOOK_MARKER: &str = "maestro-claude-hook"; + +/// File name of the installed script (carries [`HOOK_MARKER`]). +pub const SCRIPT_FILE_NAME: &str = "maestro-claude-hook.sh"; + +/// Claude Code hook event → `ActivityState` spelling posted to the daemon. +pub const HOOK_EVENTS: [(&str, &str); 3] = [ + // Tool finished: the agent is demonstrably producing work. + ("PostToolUse", "Active"), + // Permission / idle notices: Claude is blocked on the user. + ("Notification", "AwaitingInput"), + // Turn ended: waiting on the user again. + ("Stop", "AwaitingInput"), +]; + +/// The shipped hook script. POSIX sh so any `/bin/sh` works. +pub const HOOK_SCRIPT: &str = r#"#!/bin/sh +# Installed by maestro (#106): report Claude Code activity to its daemon. +# Exits silently unless running inside a session maestro spawned (#103 env). +# Event -> ActivityState mapping: +# PostToolUse -> Active (tool finished; agent is working) +# Notification -> AwaitingInput (permission / idle notice) +# Stop -> AwaitingInput (turn ended; waiting on the user) +state="$1" +[ -n "$MAESTRO_SESSION_ID" ] || exit 0 +[ -n "$MAESTRO_URL" ] || exit 0 +# Drain the hook payload so the parent never sees a broken pipe. +cat >/dev/null 2>&1 +curl -fsS --connect-timeout 1 --max-time 2 \ + -H 'Content-Type: application/json' \ + -d "{\"state\":\"$state\"}" \ + "$MAESTRO_URL/api/sessions/$MAESTRO_SESSION_ID/activity" \ + >/dev/null 2>&1 || true +exit 0 +"#; + +#[derive(Debug, thiserror::Error)] +pub enum HooksError { + #[error("io: {0}")] + Io(#[from] std::io::Error), + #[error("json: {0}")] + Json(#[from] serde_json::Error), +} + +/// Where the script is written for a given home directory: +/// `/.local/share/maestro/maestro-claude-hook.sh`. +pub fn script_path(home: &Path) -> std::path::PathBuf { + home.join(".local/share/maestro").join(SCRIPT_FILE_NAME) +} + +/// The user-level Claude Code settings file for a given home directory. +pub fn settings_path(home: &Path) -> std::path::PathBuf { + home.join(".claude/settings.json") +} + +/// Whether `settings_path` already carries a maestro hook (#106). +pub fn is_installed(settings_path: &Path) -> bool { + settings_json(settings_path) + .map(|root| find_maestro_commands(&root).next().is_some()) + .unwrap_or(false) +} + +/// Write [`HOOK_SCRIPT`] to `path`, executable. Parent directories are +/// created as needed — this is always under the caller's chosen data dir, +/// never inside a repository. +pub fn write_script(path: &Path) -> Result<(), HooksError> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + std::fs::write(path, HOOK_SCRIPT)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(path)?.permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(path, perms)?; + } + Ok(()) +} + +/// Install the full hook setup for `home`: write the script under +/// `.local/share/maestro/` and register it in `.claude/settings.json`. +/// Idempotent; returns whether anything changed on disk. +pub fn install_for_home(home: &Path) -> Result { + let script = script_path(home); + let settings = settings_path(home); + let before = std::fs::read_to_string(&settings).unwrap_or_default(); + write_script(&script)?; + install(&settings, &script)?; + let after = std::fs::read_to_string(&settings).unwrap_or_default(); + Ok(before != after) +} + +/// Register the hook command for every event in [`HOOK_EVENTS`] in the +/// settings file at `settings_path`. Idempotent: existing maestro entries are +/// updated in place wherever they sit, stale ones removed, duplicates folded; +/// every non-maestro entry and every unrelated top-level field survives. +/// +/// Never inspects or writes anywhere but `settings_path` — callers pass the +/// user's real home or a temp one in tests, never a repository. +pub fn install(settings_path: &Path, script_path: &Path) -> Result<(), HooksError> { + let mut root = settings_json(settings_path)?; + + // Strip every maestro entry wherever it currently sits, so an older + // install under different events is replaced rather than duplicated. + strip_maestro_entries(&mut root); + + let hooks = ensure_object(&mut root, "hooks"); + for (event, state) in HOOK_EVENTS { + let groups = ensure_array(hooks, event); + let mut group = serde_json::Map::new(); + group.insert( + "hooks".into(), + serde_json::json!([{ + "type": "command", + "command": format!("{} {state}", script_path.display()), + }]), + ); + groups.push(serde_json::Value::Object(group)); + } + + write_atomic(settings_path, &root) +} + +fn settings_json(path: &Path) -> Result { + match std::fs::read_to_string(path) { + Ok(s) => Ok(serde_json::from_str(&s)?), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + Ok(serde_json::Value::Object(Default::default())) + } + Err(e) => Err(e.into()), + } +} + +/// Every command string in the file belonging to a maestro hook. +fn find_maestro_commands(root: &serde_json::Value) -> impl Iterator { + root.get("hooks") + .and_then(|h| h.as_object()) + .into_iter() + .flat_map(|events| events.values()) + .filter_map(|groups| groups.as_array()) + .flatten() + .filter_map(|group| group.get("hooks")) + .filter_map(|inner| inner.as_array()) + .flatten() + .filter_map(|hook| hook.get("command")) + .filter_map(|cmd| cmd.as_str()) + .filter(|cmd| cmd.contains(HOOK_MARKER)) +} + +/// Remove all maestro inner-hook entries from every event group, dropping +/// groups left empty. Non-maestro siblings survive untouched. +fn strip_maestro_entries(root: &mut serde_json::Value) { + let Some(events) = root.get_mut("hooks").and_then(|h| h.as_object_mut()) else { + return; + }; + for (_, groups) in events.iter_mut() { + let Some(list) = groups.as_array_mut() else { + continue; + }; + for group in list.iter_mut() { + let Some(inner) = group.get_mut("hooks").and_then(|h| h.as_array_mut()) else { + continue; + }; + inner.retain(|hook| { + !hook + .get("command") + .and_then(|c| c.as_str()) + .is_some_and(|c| c.contains(HOOK_MARKER)) + }); + } + list.retain(|group| { + group + .get("hooks") + .and_then(|h| h.as_array()) + .is_some_and(|h| !h.is_empty()) + }); + } + // An event left with no groups at all (ours emptied it, or the user's + // file carried an empty list) reads better gone. + events.retain(|_, groups| groups.as_array().is_some_and(|g| !g.is_empty())); +} + +fn ensure_object<'a>( + root: &'a mut serde_json::Value, + key: &str, +) -> &'a mut serde_json::Map { + if !root.get(key).map(|v| v.is_object()).unwrap_or(false) { + root[key] = serde_json::json!({}); + } + root.get_mut(key).unwrap().as_object_mut().unwrap() +} + +fn ensure_array<'a>( + parent: &'a mut serde_json::Map, + key: &str, +) -> &'a mut Vec { + let v = parent + .entry(key.to_string()) + .or_insert_with(|| serde_json::json!([])); + if !v.is_array() { + *v = serde_json::json!([]); + } + v.as_array_mut().unwrap() +} + +fn write_atomic(path: &Path, value: &serde_json::Value) -> Result<(), HooksError> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + let tmp = path.with_extension("json.maestro-tmp"); + std::fs::write(&tmp, serde_json::to_string_pretty(value)? + "\n")?; + std::fs::rename(&tmp, path)?; + Ok(()) +} + +#[cfg(test)] +mod tests; diff --git a/crates/maestro-hooks/src/tests.rs b/crates/maestro-hooks/src/tests.rs new file mode 100644 index 0000000..79acd80 --- /dev/null +++ b/crates/maestro-hooks/src/tests.rs @@ -0,0 +1,268 @@ +//! Tests for the shipped hook script (run as a real `/bin/sh` subprocess) +//! and the settings-file installer. Everything happens under a temp `$HOME`; +//! the developer's own `~/.claude/settings.json` is never touched. + +use std::io::{Read, Write}; +use std::process::{Command, Stdio}; + +use super::*; + +fn sh(script: &str, args: &[&str], envs: &[(&str, &str)]) -> std::process::Output { + let mut cmd = Command::new("/bin/sh"); + // `sh -c CMD NAME ARGS…`: NAME fills $0, so the real arguments start at $1. + let mut argv = vec!["-c", script, HOOK_MARKER]; + argv.extend(args.iter().copied()); + cmd.args(&argv); + // A clean env: only what the test passes, plus PATH so curl can spawn. + cmd.env_clear(); + cmd.env("PATH", std::env::var("PATH").unwrap_or_default()); + for (k, v) in envs { + cmd.env(k, v); + } + let mut child = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn /bin/sh"); + // Feed a Claude-Code-style payload on stdin; the script must drain it. + child + .stdin + .take() + .unwrap() + .write_all(br#"{"session_id":"x"}"#) + .ok(); + child.wait_with_output().expect("wait") +} + +/// The hook must be invisible outside maestro: no env vars (#103 unset) ⇒ +/// exit 0, no output, stdin drained. +#[test] +fn script_exits_silently_without_maestro_env() { + for (event, _) in HOOK_EVENTS { + let out = sh(HOOK_SCRIPT, &[event], &[]); + assert!(out.status.success(), "{event:?}: {:?}", out.stderr); + assert!(out.stdout.is_empty(), "{event:?}: {:?}", out.stdout); + assert!(out.stderr.is_empty(), "{event:?}: {:?}", out.stderr); + } +} + +/// With both env vars set the script POSTs the mapped state to the daemon's +/// activity endpoint and still exits 0. +#[test] +fn script_posts_mapped_state_to_the_activity_endpoint() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let server = move || { + let (mut sock, _) = listener.accept().unwrap(); + let mut buf = [0u8; 4096]; + let n = sock.read(&mut buf).unwrap(); + let raw = String::from_utf8_lossy(&buf[..n]).to_string(); + sock.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n") + .unwrap(); + raw + }; + let handle = std::thread::spawn(server); + + let url = format!("http://{addr}"); + let session = "01234567-89ab-cdef-0123-456789abcdef"; + // The installer bakes the mapped state into the command's argv; the + // script relays whatever it is given. + let out = sh( + HOOK_SCRIPT, + &["Active"], + &[("MAESTRO_URL", &url), ("MAESTRO_SESSION_ID", session)], + ); + assert!(out.status.success()); + + let request = handle.join().unwrap(); + assert!(request.starts_with("POST "), "{request}"); + assert!( + request.contains(&format!("/api/sessions/{session}/activity")), + "{request}" + ); + assert!(request.contains(r#""state":"Active""#), "{request}"); +} + +/// Every event maps to the state the issue specifies. +#[test] +fn event_state_mapping_matches_the_contract() { + let map: std::collections::HashMap<&str, &str> = HOOK_EVENTS.into_iter().collect(); + assert_eq!(map["PostToolUse"], "Active"); + assert_eq!(map["Stop"], "AwaitingInput"); + assert_eq!(map["Notification"], "AwaitingInput"); +} + +/// A dead or unreachable daemon must never fail — or slow down — a session. +#[test] +fn script_ignores_daemon_failure() { + // Port 1 on localhost is closed in practice; curl fails fast. + let out = sh( + HOOK_SCRIPT, + &["Stop"], + &[ + ("MAESTRO_URL", "http://127.0.0.1:1"), + ("MAESTRO_SESSION_ID", "00000000-0000-0000-0000-000000000000"), + ], + ); + assert!(out.status.success()); +} + +// --- installer ------------------------------------------------------------- + +fn installed_commands(settings: &Path) -> Vec { + serde_json::from_str::(&std::fs::read_to_string(settings).unwrap()) + .unwrap() + .get("hooks") + .and_then(|h| h.as_object()) + .into_iter() + .flat_map(|events| events.values()) + .filter_map(|g| g.as_array()) + .flatten() + .filter_map(|g| g.get("hooks")) + .filter_map(|h| h.as_array()) + .flatten() + .filter_map(|h| h.get("command")) + .filter_map(|c| c.as_str()) + .map(str::to_string) + .collect() +} + +/// Fresh install registers exactly one command per event, all pointing at the +/// script with the right state argument. +#[test] +fn fresh_install_registers_every_event() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + write_script(&script_path(home)).unwrap(); + install(&settings_path(home), &script_path(home)).unwrap(); + + let cmds = installed_commands(&settings_path(home)); + assert_eq!(cmds.len(), 3); + let joined = cmds.join("\n"); + assert!(joined.contains("Active")); + assert_eq!(joined.matches("AwaitingInput").count(), 2); + for cmd in &cmds { + assert!(cmd.contains(SCRIPT_FILE_NAME)); + } +} + +/// Running install twice changes nothing further — no duplicates. +#[test] +fn install_is_idempotent() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + install(&settings_path(home), &script_path(home)).unwrap(); + let once = std::fs::read_to_string(settings_path(home)).unwrap(); + + install(&settings_path(home), &script_path(home)).unwrap(); + let twice = std::fs::read_to_string(settings_path(home)).unwrap(); + assert_eq!(once, twice); + assert_eq!(installed_commands(&settings_path(home)).len(), 3); +} + +/// An older maestro entry under the same events is updated in place, not +/// duplicated next to the new one. +#[test] +fn install_updates_an_older_maestro_hook_in_place() { + let dir = tempfile::tempdir().unwrap(); + let settings = dir.path().join(".claude/settings.json"); + std::fs::create_dir_all(settings.parent().unwrap()).unwrap(); + let old_script = "/old/location/maestro-claude-hook.sh Active"; + std::fs::write( + &settings, + serde_json::json!({ + "model": "sonnet", + "hooks": { + "Stop": [{"hooks": [{"type": "command", "command": old_script}]}], + }, + }) + .to_string(), + ) + .unwrap(); + + let new_script = dir + .path() + .join(".local/share/maestro") + .join(SCRIPT_FILE_NAME); + install(&settings, &new_script).unwrap(); + + let cmds = installed_commands(&settings); + assert_eq!(cmds.len(), 3, "old entry replaced, not kept: {cmds:?}"); + assert!(!cmds.iter().any(|c| c == old_script)); + let root: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap(); + assert_eq!(root["model"], "sonnet", "unrelated fields survive"); +} + +/// A stale maestro entry under an event we no longer use is cleaned up too; +/// the user's own hooks are never touched. +#[test] +fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() { + let dir = tempfile::tempdir().unwrap(); + let settings = dir.path().join(".claude/settings.json"); + std::fs::create_dir_all(settings.parent().unwrap()).unwrap(); + std::fs::write( + &settings, + serde_json::json!({ + "hooks": { + "PreToolUse": [{"matcher": "Bash", "hooks": [ + {"type": "command", "command": "my-own-linter.sh"}, + ]}], + "UserPromptSubmit": [{"hooks": [ + {"type": "command", "command": "/old/maestro-claude-hook.sh Active"}, + ]}], + }, + }) + .to_string(), + ) + .unwrap(); + + let new_script = dir + .path() + .join(".local/share/maestro") + .join(SCRIPT_FILE_NAME); + install(&settings, &new_script).unwrap(); + + let root: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap(); + // Foreign hook untouched. + let pre = root["hooks"]["PreToolUse"].as_array().unwrap(); + assert_eq!(pre[0]["hooks"][0]["command"], "my-own-linter.sh"); + // Stale maestro event gone. + assert!(root["hooks"].get("UserPromptSubmit").is_none()); + // Our three events present, nothing else maestro-related anywhere. + let cmds = installed_commands(&settings); + assert_eq!( + cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(), + 3, + "{cmds:?}" + ); +} + +/// `is_installed` flips only once a maestro entry exists. +#[test] +fn is_installed_tracks_reality() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + assert!(!is_installed(&settings_path(home))); + install_for_home(home).unwrap(); + assert!(is_installed(&settings_path(home))); +} + +/// The full home-level install leaves the script executable. +#[test] +fn install_for_home_writes_an_executable_script() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + install_for_home(home).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(script_path(home)) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o111, 0o111); + } +}