Compare commits

...
1 Commits
Author SHA1 Message Date
Miguel PalhasandClaude Opus 5 706942ddd7 feat(maestro): install activity hooks for codex too
ci / web (push) Successful in 51s
ci / flake (push) Successful in 3m52s
ci / rust (push) Successful in 6m47s
The hook installer only ever wrote ~/.claude/settings.json, so a codex
session reported no activity and never posted its first prompt — leaving
it on the daemon's generated handle with no haiku name.

Adds a Target enum (claude-code, codex) over the hook file's location and
event set. One script and one marker serve both; codex swaps Notification
for PermissionRequest and has no StopFailure, so it never reports Error.
Both agents spell the prompt event UserPromptSubmit and put the text in
the payload's `prompt`, so autoname needs no change.

Codex will not run a hook it has not been told to trust; `maestro hooks
install` says so, and the approval stays the operator's.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-04 10:44:14 +01:00
6 changed files with 428 additions and 145 deletions
+23 -11
View File
@@ -96,20 +96,32 @@ pub async fn dispatch(
} }
/// `maestro hooks install` (#106): write the shipped hook script under the /// `maestro hooks install` (#106): write the shipped hook script under the
/// user's data dir and register it in their Claude Code settings. Idempotent /// user's data dir and register it in every agent's hook file. Idempotent and
/// and foreign-hook-preserving — the guarantees live in [`maestro_hooks`]. /// foreign-hook-preserving — the guarantees live in [`maestro_hooks`]. One
/// agent failing is reported and does not stop the others.
#[allow(clippy::result_large_err)] #[allow(clippy::result_large_err)]
fn run_hooks_install() -> Result<(), CliError> { fn run_hooks_install() -> Result<(), CliError> {
let home = dirs::home_dir().ok_or(CliError::NoHome)?; let home = dirs::home_dir().ok_or(CliError::NoHome)?;
let changed = maestro_hooks::install_for_home(&home)?; let script = maestro_hooks::script_path(&home);
if changed { for (target, result) in maestro_hooks::install_for_home(&home) {
println!( let agent = target.label();
"installed claude-code activity hook: {} → {}", let settings = target.settings_path(&home);
maestro_hooks::script_path(&home).display(), match result {
maestro_hooks::settings_path(&home).display() Ok(true) => {
); println!(
} else { "installed {agent} activity hook: {} → {}",
println!("claude-code activity hook already up to date"); script.display(),
settings.display()
);
if target == maestro_hooks::Target::Codex {
// Codex will not run a hook it has not been told to
// trust, and that approval is the operator's to give.
println!(" codex will ask you to trust this hook the next time it starts");
}
}
Ok(false) => println!("{agent} activity hook already up to date"),
Err(e) => eprintln!("{agent} activity hook not installed: {e}"),
}
} }
Ok(()) Ok(())
} }
+16 -10
View File
@@ -51,17 +51,23 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
// First-start hook auto-install (#106). Best-effort, before serving: a // First-start hook auto-install (#106). Best-effort, before serving: a
// session spawned right away should already carry the hook. // session spawned right away should already carry the hook.
if config.hooks_auto_install { if config.hooks_auto_install {
match crate::hooks::auto_install(&config.home_dir) { for (target, outcome) in crate::hooks::auto_install(&config.home_dir) {
crate::hooks::AutoInstallOutcome::Installed => { let agent = target.label();
tracing::info!("installed claude-code activity hook (#106)"); match outcome {
crate::hooks::AutoInstallOutcome::Installed => {
tracing::info!(agent, "installed activity hook (#106)");
}
crate::hooks::AutoInstallOutcome::Upgraded => {
tracing::info!(agent, "updated activity hook to this build (#106)");
}
crate::hooks::AutoInstallOutcome::Failed => {
tracing::warn!(
agent,
"hooks auto-install failed; run `maestro hooks install` manually"
);
}
_ => {}
} }
crate::hooks::AutoInstallOutcome::Upgraded => {
tracing::info!("updated claude-code activity hook to this build (#106)");
}
crate::hooks::AutoInstallOutcome::Failed => {
tracing::warn!("hooks auto-install failed; run `maestro hooks install` manually");
}
_ => {}
} }
} }
// No worktree dir to pre-create: without an override the worktrees live // No worktree dir to pre-create: without an override the worktrees live
+107 -37
View File
@@ -1,37 +1,53 @@
//! First-start hook auto-install (#106). //! First-start hook auto-install (#106).
//! //!
//! The daemon runs [`auto_install`] once at startup: unless the user's //! The daemon runs [`auto_install`] once at startup: for every agent maestro
//! `~/.claude/settings.json` already carries exactly what this build ships //! knows how to hook ([`maestro_hooks::Target`]), unless that agent's hook
//! (and `[hooks].auto_install` hasn't opted out — checked by the caller), the //! file already carries exactly what this build ships (and `[hooks]
//! same installer `maestro hooks install` uses writes it. That covers //! .auto_install` hasn't opted out — checked by the caller), the same
//! upgrades as well as first starts: an install from a maestro that had fewer //! installer `maestro hooks install` uses writes it. That covers upgrades as
//! events — #131 added `UserPromptSubmit` to a hook #106 had already put in //! well as first starts: an install from a maestro that had fewer events —
//! everyone's settings — is rewritten rather than left half-wired. Best-effort: //! #131 added `UserPromptSubmit` to a hook #106 had already put in everyone's
//! a failure is warned about and never blocks startup. The same global hook //! settings — is rewritten rather than left half-wired. Best-effort: a
//! serves every Claude Code session on the machine, so nothing here targets //! failure is warned about and never blocks startup, and one target failing
//! repositories or worktrees. //! does not stop the others. The same global hook serves every session on the
//! machine, so nothing here targets repositories or worktrees.
use std::path::Path; use std::path::Path;
/// Install the Claude Code activity hook for `home_dir` unless the current use maestro_hooks::Target;
/// one is already there or the home dir is unresolvable (`""`). Returns what
/// happened; an older install being brought up to date is /// Install the activity hook for every [`Target`] under `home_dir`, skipping
/// [`AutoInstallOutcome::Upgraded`], so the log can say so. /// any whose install is already current. Returns what happened per target —
pub fn auto_install(home_dir: &str) -> AutoInstallOutcome { /// an older install being brought up to date is
/// [`AutoInstallOutcome::Upgraded`], so the log can say so. An unresolvable
/// home dir (`""`) reports every target as [`AutoInstallOutcome::Skipped`].
pub fn auto_install(home_dir: &str) -> Vec<(Target, AutoInstallOutcome)> {
Target::ALL
.iter()
.map(|&target| (target, auto_install_target(target, home_dir)))
.collect()
}
fn auto_install_target(target: Target, home_dir: &str) -> AutoInstallOutcome {
if home_dir.is_empty() { if home_dir.is_empty() {
return AutoInstallOutcome::Skipped; return AutoInstallOutcome::Skipped;
} }
let settings = maestro_hooks::settings_path(Path::new(home_dir)); let home = Path::new(home_dir);
let script = maestro_hooks::script_path(Path::new(home_dir)); let settings = target.settings_path(home);
if maestro_hooks::is_current(&settings, &script) { let script = maestro_hooks::script_path(home);
if maestro_hooks::is_current(target, &settings, &script) {
return AutoInstallOutcome::AlreadyPresent; return AutoInstallOutcome::AlreadyPresent;
} }
let stale = maestro_hooks::is_installed(&settings); let stale = maestro_hooks::is_installed(&settings);
match maestro_hooks::install_for_home(Path::new(home_dir)) { match maestro_hooks::install_target(target, home) {
Ok(_) if stale => AutoInstallOutcome::Upgraded, Ok(_) if stale => AutoInstallOutcome::Upgraded,
Ok(_) => AutoInstallOutcome::Installed, Ok(_) => AutoInstallOutcome::Installed,
Err(e) => { Err(e) => {
tracing::warn!(error = %e, "hooks auto-install failed (non-fatal)"); tracing::warn!(
target = target.label(),
error = %e,
"hooks auto-install failed (non-fatal)"
);
AutoInstallOutcome::Failed AutoInstallOutcome::Failed
} }
} }
@@ -52,15 +68,50 @@ pub enum AutoInstallOutcome {
mod tests { mod tests {
use super::*; use super::*;
/// The outcome recorded for one target.
fn outcome(outcomes: &[(Target, AutoInstallOutcome)], target: Target) -> AutoInstallOutcome {
outcomes
.iter()
.find(|(t, _)| *t == target)
.map(|(_, o)| *o)
.expect("every target is reported")
}
/// Everything happens under a temp `$HOME`; the developer's own /// Everything happens under a temp `$HOME`; the developer's own
/// `~/.claude/settings.json` is never read or written. /// `~/.claude/settings.json` and `~/.codex/hooks.json` are never read or
/// written.
#[test] #[test]
fn installs_when_settings_carry_no_maestro_hook() { fn installs_when_settings_carry_no_maestro_hook() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let outcome = auto_install(dir.path().to_str().unwrap()); let outcomes = auto_install(dir.path().to_str().unwrap());
assert_eq!(outcome, AutoInstallOutcome::Installed); for target in Target::ALL {
let settings = dir.path().join(".claude/settings.json"); assert_eq!(
assert!(maestro_hooks::is_installed(&settings)); outcome(&outcomes, target),
AutoInstallOutcome::Installed,
"{}",
target.label()
);
assert!(maestro_hooks::is_installed(
&target.settings_path(dir.path())
));
}
}
/// Every agent maestro knows about is hooked, not just Claude Code: a
/// codex session reports activity and its first prompt the same way.
#[test]
fn codex_gets_its_own_hook_file() {
let dir = tempfile::tempdir().unwrap();
auto_install(dir.path().to_str().unwrap());
let hooks = dir.path().join(".codex/hooks.json");
let root: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(&hooks).unwrap()).unwrap();
let events = root["hooks"].as_object().unwrap();
assert!(events.contains_key("PermissionRequest"));
assert!(
!events.contains_key("Notification"),
"codex has no Notification"
);
} }
#[test] #[test]
@@ -69,14 +120,22 @@ mod tests {
let home = dir.path().to_str().unwrap(); let home = dir.path().to_str().unwrap();
auto_install(home); auto_install(home);
// A current install is left exactly as it is. // A current install is left exactly as it is.
let settings = dir.path().join(".claude/settings.json"); let before: Vec<String> = Target::ALL
let before = std::fs::read_to_string(&settings).unwrap(); .iter()
assert_eq!(auto_install(home), AutoInstallOutcome::AlreadyPresent); .map(|t| std::fs::read_to_string(t.settings_path(dir.path())).unwrap())
assert_eq!( .collect();
before, let outcomes = auto_install(home);
std::fs::read_to_string(&settings).unwrap(), for (i, target) in Target::ALL.iter().enumerate() {
"already-installed settings must not be rewritten" assert_eq!(
); outcome(&outcomes, *target),
AutoInstallOutcome::AlreadyPresent
);
assert_eq!(
before[i],
std::fs::read_to_string(target.settings_path(dir.path())).unwrap(),
"already-installed settings must not be rewritten"
);
}
} }
/// An install from an older maestro — present, but missing an event this /// An install from an older maestro — present, but missing an event this
@@ -87,7 +146,7 @@ mod tests {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path().to_str().unwrap(); let home = dir.path().to_str().unwrap();
auto_install(home); auto_install(home);
let settings = dir.path().join(".claude/settings.json"); let settings = Target::ClaudeCode.settings_path(dir.path());
let script = maestro_hooks::script_path(dir.path()); let script = maestro_hooks::script_path(dir.path());
let mut root: serde_json::Value = let mut root: serde_json::Value =
@@ -96,12 +155,23 @@ mod tests {
std::fs::write(&settings, serde_json::to_string_pretty(&root).unwrap()).unwrap(); std::fs::write(&settings, serde_json::to_string_pretty(&root).unwrap()).unwrap();
std::fs::write(&script, "#!/bin/sh\nexit 0\n").unwrap(); std::fs::write(&script, "#!/bin/sh\nexit 0\n").unwrap();
assert_eq!(auto_install(home), AutoInstallOutcome::Upgraded); let outcomes = auto_install(home);
assert!(maestro_hooks::is_current(&settings, &script)); assert_eq!(
outcome(&outcomes, Target::ClaudeCode),
AutoInstallOutcome::Upgraded
);
assert!(maestro_hooks::is_current(
Target::ClaudeCode,
&settings,
&script
));
} }
#[test] #[test]
fn unresolvable_home_is_skipped() { fn unresolvable_home_is_skipped() {
assert_eq!(auto_install(""), AutoInstallOutcome::Skipped); let outcomes = auto_install("");
for target in Target::ALL {
assert_eq!(outcome(&outcomes, target), AutoInstallOutcome::Skipped);
}
} }
} }
+142 -55
View File
@@ -1,4 +1,4 @@
//! Claude Code activity hook (#106). //! Agent activity hook (#106).
//! //!
//! Two halves, both shared by the `maestro hooks install` CLI subcommand and //! Two halves, both shared by the `maestro hooks install` CLI subcommand and
//! the daemon's first-start auto-install: //! the daemon's first-start auto-install:
@@ -6,28 +6,40 @@
//! * [`HOOK_SCRIPT`] — the hook itself, written to disk at install time. It //! * [`HOOK_SCRIPT`] — the hook itself, written to disk at install time. It
//! posts an [`ActivityState`](maestro-protocol) to //! posts an [`ActivityState`](maestro-protocol) to
//! `$MAESTRO_URL/api/sessions/$MAESTRO_SESSION_ID/activity` (#105). The same //! `$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 //! global hook runs in every agent session on the machine, so it exits
//! 0 and silently when either #103 env var is unset, and it never lets a //! 0 and silently when either #103 env var is unset, and it never lets a
//! daemon failure escape (short curl timeout, status ignored). //! daemon failure escape (short curl timeout, status ignored).
//! * [`install`] — idempotent registration of that script in //! * [`install`] — idempotent registration of that script in one [`Target`]'s
//! `~/.claude/settings.json`. Only entries maestro itself wrote (identified //! hook file. Only entries maestro itself wrote (identified by
//! by [`HOOK_MARKER`]) are touched; every other hook and every other field //! [`HOOK_MARKER`]) are touched; every other hook and every other field in
//! in the file is preserved verbatim. //! the file is preserved verbatim.
//!
//! One script serves every [`Target`]: Claude Code and Codex read the same
//! `{"hooks": {Event: [{"hooks": [{"type": "command", "command": …}]}]}}`
//! shape and hand the hook the same JSON on stdin. Only the file's location
//! and the event set differ.
use std::path::Path; use std::path::Path;
/// Marker substring every maestro-installed hook command carries. The /// Marker substring every maestro-installed hook command carries. The
/// installer rewrites or removes exactly these entries — nothing else in the /// installer rewrites or removes exactly these entries — nothing else in the
/// user's settings file. /// user's settings file.
///
/// It still says `claude` although the script now serves every [`Target`]:
/// the marker is how an install from an older maestro is recognised, and
/// renaming it would orphan every entry already on disk.
pub const HOOK_MARKER: &str = "maestro-claude-hook"; pub const HOOK_MARKER: &str = "maestro-claude-hook";
/// File name of the installed script (carries [`HOOK_MARKER`]). /// File name of the installed script (carries [`HOOK_MARKER`]). One script
/// for every [`Target`] — see the note on [`HOOK_MARKER`] for the name.
pub const SCRIPT_FILE_NAME: &str = "maestro-claude-hook.sh"; pub const SCRIPT_FILE_NAME: &str = "maestro-claude-hook.sh";
/// Hook events installed only with the `haiku-autoname` feature (#131). /// Hook events installed only with the `haiku-autoname` feature (#131), for
/// `UserPromptSubmit` carries the prompt the user just submitted; the script /// every [`Target`] — Claude Code and Codex both spell the event
/// forwards the whole payload to `/api/sessions/:id/prompt`, which names a /// `UserPromptSubmit` and both put the submitted text in the payload's
/// still-provisionally-named session after its first one. /// `prompt`. The script forwards the whole payload to
/// `/api/sessions/:id/prompt`, which names a still-provisionally-named
/// session after its first one.
#[cfg(feature = "haiku-autoname")] #[cfg(feature = "haiku-autoname")]
pub const PROMPT_EVENTS: [(&str, &str); 1] = [("UserPromptSubmit", "Prompt")]; pub const PROMPT_EVENTS: [(&str, &str); 1] = [("UserPromptSubmit", "Prompt")];
@@ -43,7 +55,7 @@ pub const PROMPT_EVENTS: [(&str, &str); 0] = [];
/// Claude Code's own name for "the turn ended on an API error"; there is no /// Claude Code's own name for "the turn ended on an API error"; there is no
/// failure signal in `Stop` or `SessionEnd` payloads, so those never report /// failure signal in `Stop` or `SessionEnd` payloads, so those never report
/// `Error`. /// `Error`.
pub const HOOK_EVENTS: [(&str, &str); 5] = [ pub const CLAUDE_EVENTS: [(&str, &str); 5] = [
// Tool starting: the agent is demonstrably producing work, even if the // Tool starting: the agent is demonstrably producing work, even if the
// tool itself runs for minutes. // tool itself runs for minutes.
("PreToolUse", "Active"), ("PreToolUse", "Active"),
@@ -58,17 +70,86 @@ pub const HOOK_EVENTS: [(&str, &str); 5] = [
("StopFailure", "Error"), ("StopFailure", "Error"),
]; ];
/// Codex hook event → `ActivityState` spelling posted to the daemon.
///
/// Codex's event set is close to Claude Code's but not identical: a pending
/// approval arrives as `PermissionRequest` rather than `Notification`, and
/// there is no `StopFailure`, so a codex session never reports `Error`.
pub const CODEX_EVENTS: [(&str, &str); 4] = [
("PreToolUse", "Active"),
("PostToolUse", "Active"),
// Codex is blocked on the operator approving a tool call.
("PermissionRequest", "AwaitingInput"),
("Stop", "AwaitingInput"),
];
/// An agent whose hook file maestro writes into.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Target {
/// `~/.claude/settings.json`.
ClaudeCode,
/// `~/.codex/hooks.json`. Codex asks the operator to trust a hook it has
/// not seen before, so an install here does nothing until they approve it
/// once inside codex — maestro never writes that approval itself.
Codex,
}
impl Target {
/// Every target an install covers. Both files are written whether or not
/// the agent is on the machine: an unused hook file costs nothing, and
/// the alternative is a session spawned after the daemon started missing
/// the hook because its agent arrived later.
pub const ALL: [Target; 2] = [Target::ClaudeCode, Target::Codex];
/// How the target is named in logs and CLI output.
pub fn label(self) -> &'static str {
match self {
Target::ClaudeCode => "claude-code",
Target::Codex => "codex",
}
}
/// The file this target's hook entries are registered in, under `home`.
pub fn settings_path(self, home: &Path) -> std::path::PathBuf {
match self {
Target::ClaudeCode => home.join(".claude/settings.json"),
Target::Codex => home.join(".codex/hooks.json"),
}
}
/// The activity events this target emits.
pub fn activity_events(self) -> &'static [(&'static str, &'static str)] {
match self {
Target::ClaudeCode => &CLAUDE_EVENTS,
Target::Codex => &CODEX_EVENTS,
}
}
/// Every event installed for this target: its activity events plus, with
/// `haiku-autoname` on, the prompt event both agents spell the same way.
pub fn events(self) -> impl Iterator<Item = (&'static str, &'static str)> {
self.activity_events()
.iter()
.chain(PROMPT_EVENTS.iter())
.copied()
}
}
/// The shipped hook script. POSIX sh so any `/bin/sh` works. /// The shipped hook script. POSIX sh so any `/bin/sh` works.
pub const HOOK_SCRIPT: &str = r#"#!/bin/sh pub const HOOK_SCRIPT: &str = r#"#!/bin/sh
# Installed by maestro (#106): report Claude Code activity to its daemon. # Installed by maestro (#106): report agent activity to its daemon.
# Exits silently unless running inside a session maestro spawned (#103 env). # One script for every agent maestro installs hooks into; the state to post
# is chosen by the caller, so the two event sets differ only in what is
# registered where. Exits silently unless running inside a session maestro
# spawned (#103 env).
# Event -> ActivityState mapping: # Event -> ActivityState mapping:
# PreToolUse -> Active (tool started; agent is working) # PreToolUse -> Active (tool started; agent is working)
# PostToolUse -> Active (tool finished) # PostToolUse -> Active (tool finished)
# Notification -> AwaitingInput (permission / idle notice) # Notification -> AwaitingInput (claude code: permission / idle)
# Stop -> AwaitingInput (turn ended; waiting on the user) # PermissionRequest -> AwaitingInput (codex: waiting on an approval)
# StopFailure -> Error (turn ended on an API error) # Stop -> AwaitingInput (turn ended; waiting on the user)
# Prompt -> the payload is forwarded to the prompt route (#131) # StopFailure -> Error (claude code: turn ended on an error)
# Prompt -> the payload is forwarded to the prompt route (#131)
state="$1" state="$1"
[ -n "$MAESTRO_SESSION_ID" ] || exit 0 [ -n "$MAESTRO_SESSION_ID" ] || exit 0
[ -n "$MAESTRO_URL" ] || exit 0 [ -n "$MAESTRO_URL" ] || exit 0
@@ -115,34 +196,29 @@ pub fn script_path(home: &Path) -> std::path::PathBuf {
home.join(".local/share/maestro").join(SCRIPT_FILE_NAME) home.join(".local/share/maestro").join(SCRIPT_FILE_NAME)
} }
/// The user-level Claude Code settings file for a given home directory. /// Whether the file at `settings_path` already carries a maestro hook
pub fn settings_path(home: &Path) -> std::path::PathBuf { /// (#106). Presence only: an install from an older release answers `true`
home.join(".claude/settings.json") /// even when it is missing events this build ships — see [`is_current`].
}
/// Whether `settings_path` already carries a maestro hook (#106). Presence
/// only: an install from an older release answers `true` even when it is
/// missing events this build ships — see [`is_current`].
pub fn is_installed(settings_path: &Path) -> bool { pub fn is_installed(settings_path: &Path) -> bool {
settings_json(settings_path) settings_json(settings_path)
.map(|root| find_maestro_commands(&root).next().is_some()) .map(|root| find_maestro_commands(&root).next().is_some())
.unwrap_or(false) .unwrap_or(false)
} }
/// Whether what is on disk is what this build ships: the script at /// Whether what is on disk is what this build ships for `target`: the script
/// `script_path` byte-for-byte, and exactly one settings entry per event in /// at `script_path` byte-for-byte, and exactly one settings entry per one of
/// [`HOOK_EVENTS`] and [`PROMPT_EVENTS`], pointing at that script. /// the target's [`Target::events`], pointing at that script.
/// ///
/// This is the upgrade question. A maestro that gains an event — #131 added /// This is the upgrade question, asked per [`Target`]. A maestro that gains
/// `UserPromptSubmit` long after #106 had installed itself — leaves every /// an event — #131 added `UserPromptSubmit` long after #106 had installed
/// existing user with a present but incomplete install, which /// itself — leaves every existing user with a present but incomplete
/// [`is_installed`] cannot tell apart from a current one. /// install, which [`is_installed`] cannot tell apart from a current one.
pub fn is_current(settings_path: &Path, script_path: &Path) -> bool { pub fn is_current(target: Target, settings_path: &Path, script_path: &Path) -> bool {
if std::fs::read_to_string(script_path).ok().as_deref() != Some(HOOK_SCRIPT) { if std::fs::read_to_string(script_path).ok().as_deref() != Some(HOOK_SCRIPT) {
return false; return false;
} }
settings_json(settings_path) settings_json(settings_path)
.map(|root| maestro_entries(&root) == expected_entries(script_path)) .map(|root| maestro_entries(&root) == expected_entries(target, script_path))
.unwrap_or(false) .unwrap_or(false)
} }
@@ -173,14 +249,13 @@ fn maestro_entries(root: &serde_json::Value) -> Vec<(String, String)> {
found found
} }
/// What [`install`] writes for `script_path`, in the same shape. /// What [`install`] writes for `target` and `script_path`, in the same shape.
fn expected_entries(script_path: &Path) -> Vec<(String, String)> { fn expected_entries(target: Target, script_path: &Path) -> Vec<(String, String)> {
let mut want: Vec<(String, String)> = HOOK_EVENTS let mut want: Vec<(String, String)> = target
.iter() .events()
.chain(PROMPT_EVENTS.iter())
.map(|(event, state)| { .map(|(event, state)| {
( (
(*event).to_string(), event.to_string(),
format!("{} {state}", script_path.display()), format!("{} {state}", script_path.display()),
) )
}) })
@@ -207,28 +282,40 @@ pub fn write_script(path: &Path) -> Result<(), HooksError> {
Ok(()) Ok(())
} }
/// Install the full hook setup for `home`: write the script under /// Install the full hook setup for one target under `home`: write the script
/// `.local/share/maestro/` and register it in `.claude/settings.json`. /// under `.local/share/maestro/` and register it in the target's hook file.
/// Idempotent; returns whether anything changed on disk. /// Idempotent; returns whether anything changed on disk.
pub fn install_for_home(home: &Path) -> Result<bool, HooksError> { pub fn install_target(target: Target, home: &Path) -> Result<bool, HooksError> {
let script = script_path(home); let script = script_path(home);
let settings = settings_path(home); let settings = target.settings_path(home);
// A script left over from an older release counts as a change even when // A script left over from an older release counts as a change even when
// the settings entries pointing at it are already right. // the settings entries pointing at it are already right.
let changed = !is_current(&settings, &script); let changed = !is_current(target, &settings, &script);
write_script(&script)?; write_script(&script)?;
install(&settings, &script)?; install(target, &settings, &script)?;
Ok(changed) Ok(changed)
} }
/// Register the hook command for every event in [`HOOK_EVENTS`] in the /// Install for every [`Target::ALL`], reporting each one separately. One
/// settings file at `settings_path`. Idempotent: existing maestro entries are /// target failing — an unwritable file, unparseable JSON — never stops the
/// updated in place wherever they sit, stale ones removed, duplicates folded; /// others: an agent whose hook could not be written is one agent without
/// every non-maestro entry and every unrelated top-level field survives. /// activity reporting, not a failed install.
pub fn install_for_home(home: &Path) -> Vec<(Target, Result<bool, HooksError>)> {
Target::ALL
.iter()
.map(|&target| (target, install_target(target, home)))
.collect()
}
/// Register the hook command for every one of `target`'s [`Target::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 /// Never inspects or writes anywhere but `settings_path` — callers pass the
/// user's real home or a temp one in tests, never a repository. /// user's real home or a temp one in tests, never a repository.
pub fn install(settings_path: &Path, script_path: &Path) -> Result<(), HooksError> { pub fn install(target: Target, settings_path: &Path, script_path: &Path) -> Result<(), HooksError> {
let mut root = settings_json(settings_path)?; let mut root = settings_json(settings_path)?;
// Strip every maestro entry wherever it currently sits, so an older // Strip every maestro entry wherever it currently sits, so an older
@@ -236,7 +323,7 @@ pub fn install(settings_path: &Path, script_path: &Path) -> Result<(), HooksErro
strip_maestro_entries(&mut root); strip_maestro_entries(&mut root);
let hooks = ensure_object(&mut root, "hooks"); let hooks = ensure_object(&mut root, "hooks");
for (event, state) in HOOK_EVENTS.iter().chain(PROMPT_EVENTS.iter()).copied() { for (event, state) in target.events() {
let groups = ensure_array(hooks, event); let groups = ensure_array(hooks, event);
let mut group = serde_json::Map::new(); let mut group = serde_json::Map::new();
group.insert( group.insert(
+136 -32
View File
@@ -1,15 +1,24 @@
//! Tests for the shipped hook script (run as a real `/bin/sh` subprocess) //! Tests for the shipped hook script (run as a real `/bin/sh` subprocess)
//! and the settings-file installer. Everything happens under a temp `$HOME`; //! and the settings-file installer. Everything happens under a temp `$HOME`;
//! the developer's own `~/.claude/settings.json` is never touched. //! the developer's own `~/.claude/settings.json` and `~/.codex/hooks.json`
//! are never touched.
use std::io::{Read, Write}; use std::io::{Read, Write};
use std::process::{Command, Stdio}; use std::process::{Command, Stdio};
use super::*; use super::*;
/// How many commands one install writes: every activity event, plus the /// How many commands one install writes for a target: its activity events,
/// prompt event when `haiku-autoname` is on (#131). /// plus the prompt event when `haiku-autoname` is on (#131).
const INSTALLED_EVENTS: usize = HOOK_EVENTS.len() + PROMPT_EVENTS.len(); fn installed_events(target: Target) -> usize {
target.events().count()
}
/// The Claude Code hook file under a temp home. Most installer tests only
/// need one target, and this is the one they were written against.
fn settings_path(home: &Path) -> std::path::PathBuf {
Target::ClaudeCode.settings_path(home)
}
fn sh(script: &str, args: &[&str], envs: &[(&str, &str)], payload: &[u8]) -> std::process::Output { fn sh(script: &str, args: &[&str], envs: &[(&str, &str)], payload: &[u8]) -> std::process::Output {
let mut cmd = Command::new("/bin/sh"); let mut cmd = Command::new("/bin/sh");
@@ -70,7 +79,7 @@ fn owned_envs<'a>(envs: &'a [(&'static str, String)]) -> Vec<(&'static str, &'a
/// exit 0, no output, stdin drained. /// exit 0, no output, stdin drained.
#[test] #[test]
fn script_exits_silently_without_maestro_env() { fn script_exits_silently_without_maestro_env() {
for (event, _) in HOOK_EVENTS { for (event, _) in CLAUDE_EVENTS {
let out = sh(HOOK_SCRIPT, &[event], &[], br#"{"session_id":"x"}"#); let out = sh(HOOK_SCRIPT, &[event], &[], br#"{"session_id":"x"}"#);
assert!(out.status.success(), "{event:?}: {:?}", out.stderr); assert!(out.status.success(), "{event:?}: {:?}", out.stderr);
assert!(out.stdout.is_empty(), "{event:?}: {:?}", out.stdout); assert!(out.stdout.is_empty(), "{event:?}: {:?}", out.stdout);
@@ -103,7 +112,7 @@ fn script_posts_mapped_state_to_the_activity_endpoint() {
/// Every event maps to the state the contract specifies. /// Every event maps to the state the contract specifies.
#[test] #[test]
fn event_state_mapping_matches_the_contract() { fn event_state_mapping_matches_the_contract() {
let map: std::collections::HashMap<&str, &str> = HOOK_EVENTS.into_iter().collect(); let map: std::collections::HashMap<&str, &str> = CLAUDE_EVENTS.into_iter().collect();
assert_eq!(map["PreToolUse"], "Active"); assert_eq!(map["PreToolUse"], "Active");
assert_eq!(map["PostToolUse"], "Active"); assert_eq!(map["PostToolUse"], "Active");
assert_eq!(map["Notification"], "AwaitingInput"); assert_eq!(map["Notification"], "AwaitingInput");
@@ -203,10 +212,10 @@ fn fresh_install_registers_every_event() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
write_script(&script_path(home)).unwrap(); write_script(&script_path(home)).unwrap();
install(&settings_path(home), &script_path(home)).unwrap(); install(Target::ClaudeCode, &settings_path(home), &script_path(home)).unwrap();
let cmds = installed_commands(&settings_path(home)); let cmds = installed_commands(&settings_path(home));
assert_eq!(cmds.len(), INSTALLED_EVENTS); assert_eq!(cmds.len(), installed_events(Target::ClaudeCode));
let joined = cmds.join("\n"); let joined = cmds.join("\n");
assert_eq!(joined.matches("Active").count(), 2); assert_eq!(joined.matches("Active").count(), 2);
assert!(joined.contains("Error")); assert!(joined.contains("Error"));
@@ -221,15 +230,15 @@ fn fresh_install_registers_every_event() {
fn install_is_idempotent() { fn install_is_idempotent() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
install(&settings_path(home), &script_path(home)).unwrap(); install(Target::ClaudeCode, &settings_path(home), &script_path(home)).unwrap();
let once = std::fs::read_to_string(settings_path(home)).unwrap(); let once = std::fs::read_to_string(settings_path(home)).unwrap();
install(&settings_path(home), &script_path(home)).unwrap(); install(Target::ClaudeCode, &settings_path(home), &script_path(home)).unwrap();
let twice = std::fs::read_to_string(settings_path(home)).unwrap(); let twice = std::fs::read_to_string(settings_path(home)).unwrap();
assert_eq!(once, twice); assert_eq!(once, twice);
assert_eq!( assert_eq!(
installed_commands(&settings_path(home)).len(), installed_commands(&settings_path(home)).len(),
INSTALLED_EVENTS installed_events(Target::ClaudeCode)
); );
} }
@@ -257,12 +266,12 @@ fn install_updates_an_older_maestro_hook_in_place() {
.path() .path()
.join(".local/share/maestro") .join(".local/share/maestro")
.join(SCRIPT_FILE_NAME); .join(SCRIPT_FILE_NAME);
install(&settings, &new_script).unwrap(); install(Target::ClaudeCode, &settings, &new_script).unwrap();
let cmds = installed_commands(&settings); let cmds = installed_commands(&settings);
assert_eq!( assert_eq!(
cmds.len(), cmds.len(),
INSTALLED_EVENTS, installed_events(Target::ClaudeCode),
"old entry replaced, not kept: {cmds:?}" "old entry replaced, not kept: {cmds:?}"
); );
assert!(!cmds.iter().any(|c| c == old_script)); assert!(!cmds.iter().any(|c| c == old_script));
@@ -298,7 +307,7 @@ fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() {
.path() .path()
.join(".local/share/maestro") .join(".local/share/maestro")
.join(SCRIPT_FILE_NAME); .join(SCRIPT_FILE_NAME);
install(&settings, &new_script).unwrap(); install(Target::ClaudeCode, &settings, &new_script).unwrap();
let root: serde_json::Value = let root: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap(); serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap();
@@ -311,7 +320,7 @@ fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() {
let cmds = installed_commands(&settings); let cmds = installed_commands(&settings);
assert_eq!( assert_eq!(
cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(), cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(),
INSTALLED_EVENTS, installed_events(Target::ClaudeCode),
"{cmds:?}" "{cmds:?}"
); );
} }
@@ -321,9 +330,15 @@ fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() {
fn is_installed_tracks_reality() { fn is_installed_tracks_reality() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
assert!(!is_installed(&settings_path(home))); for target in Target::ALL {
install_for_home(home).unwrap(); assert!(!is_installed(&target.settings_path(home)));
assert!(is_installed(&settings_path(home))); }
for (_, result) in install_for_home(home) {
result.unwrap();
}
for target in Target::ALL {
assert!(is_installed(&target.settings_path(home)));
}
} }
/// The full home-level install leaves the script executable. /// The full home-level install leaves the script executable.
@@ -331,7 +346,9 @@ fn is_installed_tracks_reality() {
fn install_for_home_writes_an_executable_script() { fn install_for_home_writes_an_executable_script() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
install_for_home(home).unwrap(); for (_, result) in install_for_home(home) {
result.unwrap();
}
#[cfg(unix)] #[cfg(unix)]
{ {
use std::os::unix::fs::PermissionsExt; use std::os::unix::fs::PermissionsExt;
@@ -350,7 +367,7 @@ fn install_for_home_writes_an_executable_script() {
fn install_registers_the_prompt_event() { fn install_registers_the_prompt_event() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
install(&settings_path(home), &script_path(home)).unwrap(); install(Target::ClaudeCode, &settings_path(home), &script_path(home)).unwrap();
let root: serde_json::Value = let root: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(settings_path(home)).unwrap()).unwrap(); serde_json::from_str(&std::fs::read_to_string(settings_path(home)).unwrap()).unwrap();
@@ -367,7 +384,7 @@ fn install_registers_the_prompt_event() {
fn install_registers_no_prompt_event_without_the_feature() { fn install_registers_no_prompt_event_without_the_feature() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
install(&settings_path(home), &script_path(home)).unwrap(); install(Target::ClaudeCode, &settings_path(home), &script_path(home)).unwrap();
let root: serde_json::Value = let root: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(settings_path(home)).unwrap()).unwrap(); serde_json::from_str(&std::fs::read_to_string(settings_path(home)).unwrap()).unwrap();
@@ -436,8 +453,12 @@ fn the_script_posts_a_prompt_payload_unchanged() {
fn is_current_rejects_an_install_missing_an_event() { fn is_current_rejects_an_install_missing_an_event() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
install_for_home(home).unwrap(); install_target(Target::ClaudeCode, home).unwrap();
assert!(is_current(&settings_path(home), &script_path(home))); assert!(is_current(
Target::ClaudeCode,
&settings_path(home),
&script_path(home)
));
// Drop one event, as an older install would have left the file. // Drop one event, as an older install would have left the file.
let mut root: serde_json::Value = let mut root: serde_json::Value =
@@ -450,7 +471,11 @@ fn is_current_rejects_an_install_missing_an_event() {
.unwrap(); .unwrap();
assert!(is_installed(&settings_path(home))); assert!(is_installed(&settings_path(home)));
assert!(!is_current(&settings_path(home), &script_path(home))); assert!(!is_current(
Target::ClaudeCode,
&settings_path(home),
&script_path(home)
));
} }
/// A settings file that is complete but points at a script from an older /// A settings file that is complete but points at a script from an older
@@ -459,9 +484,13 @@ fn is_current_rejects_an_install_missing_an_event() {
fn is_current_rejects_a_stale_script() { fn is_current_rejects_a_stale_script() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
install_for_home(home).unwrap(); install_target(Target::ClaudeCode, home).unwrap();
std::fs::write(script_path(home), "#!/bin/sh\nexit 0\n").unwrap(); std::fs::write(script_path(home), "#!/bin/sh\nexit 0\n").unwrap();
assert!(!is_current(&settings_path(home), &script_path(home))); assert!(!is_current(
Target::ClaudeCode,
&settings_path(home),
&script_path(home)
));
} }
/// Nothing installed at all is not current. /// Nothing installed at all is not current.
@@ -469,18 +498,93 @@ fn is_current_rejects_a_stale_script() {
fn is_current_rejects_a_missing_install() { fn is_current_rejects_a_missing_install() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
assert!(!is_current(&settings_path(home), &script_path(home))); assert!(!is_current(
Target::ClaudeCode,
&settings_path(home),
&script_path(home)
));
} }
/// `install_for_home` reports a stale script as a change, even when the /// `install_target` reports a stale script as a change, even when the
/// settings entries pointing at it already read correctly. /// settings entries pointing at it already read correctly.
#[test] #[test]
fn install_for_home_reports_a_stale_script_as_a_change() { fn install_target_reports_a_stale_script_as_a_change() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let home = dir.path(); let home = dir.path();
assert!(install_for_home(home).unwrap()); assert!(install_target(Target::ClaudeCode, home).unwrap());
assert!(!install_for_home(home).unwrap()); assert!(!install_target(Target::ClaudeCode, home).unwrap());
std::fs::write(script_path(home), "#!/bin/sh\nexit 0\n").unwrap(); std::fs::write(script_path(home), "#!/bin/sh\nexit 0\n").unwrap();
assert!(install_for_home(home).unwrap()); assert!(install_target(Target::ClaudeCode, home).unwrap());
}
/// Codex is hooked in its own file, with its own event set: the two agents
/// share the script but not the events, so neither file carries the other's
/// spellings (#131 follow-up — a codex session was never named because
/// nothing installed its `UserPromptSubmit` hook).
#[test]
fn each_target_gets_its_own_file_and_events() {
let dir = tempfile::tempdir().unwrap();
let home = dir.path();
for (_, result) in install_for_home(home) {
result.unwrap();
}
let claude = events_in(&Target::ClaudeCode.settings_path(home));
let codex = events_in(&Target::Codex.settings_path(home));
assert_eq!(
Target::Codex.settings_path(home),
home.join(".codex/hooks.json")
);
// Claude Code's failure signal has no codex equivalent, and codex's
// approval notice is spelled differently.
assert!(claude.contains(&"StopFailure".to_string()));
assert!(claude.contains(&"Notification".to_string()));
assert!(!codex.contains(&"StopFailure".to_string()));
assert!(!codex.contains(&"Notification".to_string()));
assert!(codex.contains(&"PermissionRequest".to_string()));
// Both report work and both report a finished turn.
for event in ["PreToolUse", "PostToolUse", "Stop"] {
assert!(claude.contains(&event.to_string()), "claude {event}");
assert!(codex.contains(&event.to_string()), "codex {event}");
}
assert_eq!(
installed_commands(&Target::Codex.settings_path(home)).len(),
installed_events(Target::Codex)
);
}
/// #131: a codex session names itself the same way a Claude Code one does —
/// same event spelling, same script, same `Prompt` argument.
#[cfg(feature = "haiku-autoname")]
#[test]
fn codex_reports_its_first_prompt_too() {
let dir = tempfile::tempdir().unwrap();
let home = dir.path();
install_target(Target::Codex, home).unwrap();
let root: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(Target::Codex.settings_path(home)).unwrap())
.unwrap();
let cmd = root["hooks"]["UserPromptSubmit"][0]["hooks"][0]["command"]
.as_str()
.unwrap()
.to_string();
assert!(cmd.ends_with(" Prompt"), "{cmd}");
assert!(cmd.contains(SCRIPT_FILE_NAME));
}
/// Every event name a settings file registers a maestro hook under.
fn events_in(settings: &Path) -> Vec<String> {
serde_json::from_str::<serde_json::Value>(&std::fs::read_to_string(settings).unwrap())
.unwrap()
.get("hooks")
.and_then(|h| h.as_object())
.into_iter()
.flatten()
.map(|(event, _)| event.clone())
.collect()
} }
+4
View File
@@ -250,6 +250,10 @@ Two rulings specific to a reported `Error` (#114):
process changes do not clear it — only the agent itself may withdraw what process changes do not clear it — only the agent itself may withdraw what
the agent itself reported. Re-reporting `Error` extends the TTL rather than the agent itself reported. Re-reporting `Error` extends the TTL rather than
stacking reports. stacking reports.
- **Only Claude Code reports `Error`.** Codex has no counterpart to
`StopFailure` in its hook event set, so a codex turn that ends on an API
error reports `AwaitingInput` and its card carries no error mark. A card
without the mark means "nothing reported a failure", not "nothing failed".
If a client receives an `ActivityState` variant it does not know, it must not If a client receives an `ActivityState` variant it does not know, it must not
fail rendering: treat it as `Idle` (neutral/dim, line-2 rules apply as usual) fail rendering: treat it as `Idle` (neutral/dim, line-2 rules apply as usual)