Compare commits

...
Author SHA1 Message Date
Miguel PalhasandClaude Sonnet 5 f801b12e87 fix(maestro): suffix branch name on new-branch worktree collision
git worktree add -b fails outright when the branch already exists, even
if the target directory is free. A random fast-forward handle commonly
collides with a branch an earlier, already-removed worktree left behind.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-04 13:29:11 +01:00
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
Miguel Palhas 52877abe29 fix(maestro): focus the kill button in kill dialogs
ci / web (push) Successful in 37s
ci / flake (push) Successful in 2m50s
ci / rust (push) Successful in 5m46s
Enter now confirms the kill the operator just asked for; Escape and the scrim still cancel.
2026-09-04 08:24:32 +01:00
11 changed files with 567 additions and 164 deletions
+23 -11
View File
@@ -96,20 +96,32 @@ pub async fn dispatch(
}
/// `maestro hooks install` (#106): write the shipped hook script under the
/// user's data dir and register it in their Claude Code settings. Idempotent
/// and foreign-hook-preserving — the guarantees live in [`maestro_hooks`].
/// user's data dir and register it in every agent's hook file. Idempotent and
/// 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)]
fn run_hooks_install() -> Result<(), CliError> {
let home = dirs::home_dir().ok_or(CliError::NoHome)?;
let changed = maestro_hooks::install_for_home(&home)?;
if changed {
println!(
"installed claude-code activity hook: {} → {}",
maestro_hooks::script_path(&home).display(),
maestro_hooks::settings_path(&home).display()
);
} else {
println!("claude-code activity hook already up to date");
let script = maestro_hooks::script_path(&home);
for (target, result) in maestro_hooks::install_for_home(&home) {
let agent = target.label();
let settings = target.settings_path(&home);
match result {
Ok(true) => {
println!(
"installed {agent} activity hook: {} → {}",
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(())
}
+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
// session spawned right away should already carry the hook.
if config.hooks_auto_install {
match crate::hooks::auto_install(&config.home_dir) {
crate::hooks::AutoInstallOutcome::Installed => {
tracing::info!("installed claude-code activity hook (#106)");
for (target, outcome) in crate::hooks::auto_install(&config.home_dir) {
let agent = target.label();
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
+107 -37
View File
@@ -1,37 +1,53 @@
//! First-start hook auto-install (#106).
//!
//! The daemon runs [`auto_install`] once at startup: unless the user's
//! `~/.claude/settings.json` already carries exactly what this build ships
//! (and `[hooks].auto_install` hasn't opted out — checked by the caller), the
//! same installer `maestro hooks install` uses writes it. That covers
//! upgrades as well as first starts: an install from a maestro that had fewer
//! events — #131 added `UserPromptSubmit` to a hook #106 had already put in
//! everyone's settings — is rewritten rather than left half-wired. Best-effort:
//! a failure is warned about and never blocks startup. The same global hook
//! serves every Claude Code session on the machine, so nothing here targets
//! repositories or worktrees.
//! The daemon runs [`auto_install`] once at startup: for every agent maestro
//! knows how to hook ([`maestro_hooks::Target`]), unless that agent's hook
//! file already carries exactly what this build ships (and `[hooks]
//! .auto_install` hasn't opted out — checked by the caller), the same
//! installer `maestro hooks install` uses writes it. That covers upgrades as
//! well as first starts: an install from a maestro that had fewer events —
//! #131 added `UserPromptSubmit` to a hook #106 had already put in everyone's
//! settings — is rewritten rather than left half-wired. Best-effort: a
//! failure is warned about and never blocks startup, and one target failing
//! 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;
/// Install the Claude Code activity hook for `home_dir` unless the current
/// one is already there or the home dir is unresolvable (`""`). Returns what
/// happened; an older install being brought up to date is
/// [`AutoInstallOutcome::Upgraded`], so the log can say so.
pub fn auto_install(home_dir: &str) -> AutoInstallOutcome {
use maestro_hooks::Target;
/// Install the activity hook for every [`Target`] under `home_dir`, skipping
/// any whose install is already current. Returns what happened per target —
/// 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() {
return AutoInstallOutcome::Skipped;
}
let settings = maestro_hooks::settings_path(Path::new(home_dir));
let script = maestro_hooks::script_path(Path::new(home_dir));
if maestro_hooks::is_current(&settings, &script) {
let home = Path::new(home_dir);
let settings = target.settings_path(home);
let script = maestro_hooks::script_path(home);
if maestro_hooks::is_current(target, &settings, &script) {
return AutoInstallOutcome::AlreadyPresent;
}
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(_) => AutoInstallOutcome::Installed,
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
}
}
@@ -52,15 +68,50 @@ pub enum AutoInstallOutcome {
mod tests {
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
/// `~/.claude/settings.json` is never read or written.
/// `~/.claude/settings.json` and `~/.codex/hooks.json` are never read or
/// written.
#[test]
fn installs_when_settings_carry_no_maestro_hook() {
let dir = tempfile::tempdir().unwrap();
let outcome = auto_install(dir.path().to_str().unwrap());
assert_eq!(outcome, AutoInstallOutcome::Installed);
let settings = dir.path().join(".claude/settings.json");
assert!(maestro_hooks::is_installed(&settings));
let outcomes = auto_install(dir.path().to_str().unwrap());
for target in Target::ALL {
assert_eq!(
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]
@@ -69,14 +120,22 @@ mod tests {
let home = dir.path().to_str().unwrap();
auto_install(home);
// A current install is left exactly as it is.
let settings = dir.path().join(".claude/settings.json");
let before = std::fs::read_to_string(&settings).unwrap();
assert_eq!(auto_install(home), AutoInstallOutcome::AlreadyPresent);
assert_eq!(
before,
std::fs::read_to_string(&settings).unwrap(),
"already-installed settings must not be rewritten"
);
let before: Vec<String> = Target::ALL
.iter()
.map(|t| std::fs::read_to_string(t.settings_path(dir.path())).unwrap())
.collect();
let outcomes = auto_install(home);
for (i, target) in Target::ALL.iter().enumerate() {
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
@@ -87,7 +146,7 @@ mod tests {
let dir = tempfile::tempdir().unwrap();
let home = dir.path().to_str().unwrap();
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 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(&script, "#!/bin/sh\nexit 0\n").unwrap();
assert_eq!(auto_install(home), AutoInstallOutcome::Upgraded);
assert!(maestro_hooks::is_current(&settings, &script));
let outcomes = auto_install(home);
assert_eq!(
outcome(&outcomes, Target::ClaudeCode),
AutoInstallOutcome::Upgraded
);
assert!(maestro_hooks::is_current(
Target::ClaudeCode,
&settings,
&script
));
}
#[test]
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);
}
}
}
+40 -12
View File
@@ -111,16 +111,33 @@ impl WorktreeManager {
// Clear registrations whose directory is gone first, so a
// previously-leaked entry doesn't push this branch to a suffix.
prune(base_repo)?;
// Anything occupying the target — a tree we left behind, one the
// operator made, an unrelated directory — is sidestepped with a
// numeric suffix on the directory; the branch stays whatever was
// requested (#77). The daemon never adopts what it finds.
let mut path = self.target_path(base_repo, branch);
let mut n = 1;
while path.symlink_metadata().is_ok() {
n += 1;
path = self.target_path_suffixed(base_repo, branch, n);
}
let (branch, path) = if create_branch {
// Worktrees are commonly abandoned without deleting their branch,
// so a random handle can collide with one of its own past runs.
// Unlike the `add_existing` arm below, the suffix goes on the
// branch name itself — the path is derived from it anyway.
let mut candidate = branch.to_string();
let mut path = self.target_path(base_repo, &candidate);
let mut n = 1;
while path.symlink_metadata().is_ok() || branch_exists(base_repo, &candidate) {
n += 1;
candidate = format!("{branch}-{n}");
path = self.target_path(base_repo, &candidate);
}
(candidate, path)
} else {
// Existing branch: the name must stay exact, so only the
// directory — anything occupying it, ours or the operator's — is
// sidestepped with a numeric suffix (#77). The daemon never
// adopts what it finds.
let mut path = self.target_path(base_repo, branch);
let mut n = 1;
while path.symlink_metadata().is_ok() {
n += 1;
path = self.target_path_suffixed(base_repo, branch, n);
}
(branch.to_string(), path)
};
// The parent (`<repo>/worktrees` or the override root) may not exist
// yet; the target itself must not.
if let Some(parent) = path.parent() {
@@ -129,10 +146,10 @@ impl WorktreeManager {
let mut cmd = Command::new("git");
cmd.arg("-C").arg(base_repo).args(["worktree", "add"]);
if create_branch {
cmd.arg("-b").arg(branch).arg(&path);
cmd.arg("-b").arg(&branch).arg(&path);
} else {
// Existing branch: `worktree add <path> <branch>` with no `-b`.
cmd.arg(&path).arg(branch);
cmd.arg(&path).arg(&branch);
}
let out = cmd.output()?;
check(out)?;
@@ -266,6 +283,17 @@ fn branch_relpath(branch: &str) -> Option<PathBuf> {
Some(candidate.to_path_buf())
}
/// Whether `refs/heads/<branch>` already exists in `base_repo`.
fn branch_exists(base_repo: &Path, branch: &str) -> bool {
Command::new("git")
.arg("-C")
.arg(base_repo)
.args(["show-ref", "--verify", "--quiet"])
.arg(format!("refs/heads/{branch}"))
.status()
.is_ok_and(|s| s.success())
}
/// `git -C <base_repo> worktree prune` — drops registrations whose directory
/// no longer exists.
fn prune(base_repo: &Path) -> Result<(), WorktreeError> {
+51
View File
@@ -497,6 +497,57 @@ async fn occupied_path_gets_suffixed_directory_and_both_worktrees_work() {
);
}
/// A random handle can reuse the name of a branch left behind by an earlier,
/// already-removed worktree — `git worktree add -b` refuses outright in that
/// case, even though the directory is free. The daemon suffixes the branch
/// itself (not just the directory) and both land on `-2`.
#[tokio::test(flavor = "multi_thread")]
async fn colliding_branch_name_gets_suffixed_too() {
let (_daemon, client) = bring_up().await;
let base = TempDir::new().unwrap();
init_repo(base.path());
// No worktree directory in the way, but the branch already exists.
run_git(&["branch", "handle"], base.path());
let s = client
.spawn_session(&worktree_req(base.path(), "handle", true))
.await
.expect("spawn onto a taken branch name");
let suffixed = base.path().join("worktrees").join("handle-2");
assert_eq!(
s.cwd, suffixed,
"the directory must follow the suffixed branch name"
);
assert!(suffixed.is_dir());
let registrations = git_worktree_list(base.path());
assert!(
registrations.iter().any(|l| l.contains("handle-2")),
"the -2 branch's worktree must be registered: {registrations:?}"
);
let branches = String::from_utf8(
std::process::Command::new("git")
.arg("-C")
.arg(base.path())
.args(["branch", "--list"])
.output()
.unwrap()
.stdout,
)
.unwrap();
assert!(
branches.contains("handle-2"),
"the branch itself must be named handle-2: {branches}"
);
let _origin = throwaway_origin(base.path());
client.kill_session(s.id).await.expect("kill");
}
/// #77/#88: the target path holds a plain directory the daemon never made and
/// git has never heard of. The daemon neither adopts it nor errors — it builds
/// beside it and leaves every byte of it alone.
+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
//! the daemon's first-start auto-install:
@@ -6,28 +6,40 @@
//! * [`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
//! 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
//! 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.
//! * [`install`] — idempotent registration of that script in one [`Target`]'s
//! hook file. Only entries maestro itself wrote (identified by
//! [`HOOK_MARKER`]) are touched; every other hook and every other field in
//! 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;
/// Marker substring every maestro-installed hook command carries. The
/// installer rewrites or removes exactly these entries — nothing else in the
/// 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";
/// 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";
/// Hook events installed only with the `haiku-autoname` feature (#131).
/// `UserPromptSubmit` carries the prompt the user just submitted; the script
/// forwards the whole payload to `/api/sessions/:id/prompt`, which names a
/// still-provisionally-named session after its first one.
/// Hook events installed only with the `haiku-autoname` feature (#131), for
/// every [`Target`] — Claude Code and Codex both spell the event
/// `UserPromptSubmit` and both put the submitted text in the payload's
/// `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")]
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
/// failure signal in `Stop` or `SessionEnd` payloads, so those never report
/// `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 itself runs for minutes.
("PreToolUse", "Active"),
@@ -58,17 +70,86 @@ pub const HOOK_EVENTS: [(&str, &str); 5] = [
("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.
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).
# Installed by maestro (#106): report agent activity to its daemon.
# 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:
# PreToolUse -> Active (tool started; agent is working)
# PostToolUse -> Active (tool finished)
# Notification -> AwaitingInput (permission / idle notice)
# Stop -> AwaitingInput (turn ended; waiting on the user)
# StopFailure -> Error (turn ended on an API error)
# Prompt -> the payload is forwarded to the prompt route (#131)
# PreToolUse -> Active (tool started; agent is working)
# PostToolUse -> Active (tool finished)
# Notification -> AwaitingInput (claude code: permission / idle)
# PermissionRequest -> AwaitingInput (codex: waiting on an approval)
# Stop -> AwaitingInput (turn ended; waiting on the user)
# StopFailure -> Error (claude code: turn ended on an error)
# Prompt -> the payload is forwarded to the prompt route (#131)
state="$1"
[ -n "$MAESTRO_SESSION_ID" ] || 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)
}
/// 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). Presence
/// only: an install from an older release answers `true` even when it is
/// missing events this build ships — see [`is_current`].
/// Whether the file at `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 {
settings_json(settings_path)
.map(|root| find_maestro_commands(&root).next().is_some())
.unwrap_or(false)
}
/// Whether what is on disk is what this build ships: the script at
/// `script_path` byte-for-byte, and exactly one settings entry per event in
/// [`HOOK_EVENTS`] and [`PROMPT_EVENTS`], pointing at that script.
/// Whether what is on disk is what this build ships for `target`: the script
/// at `script_path` byte-for-byte, and exactly one settings entry per one of
/// the target's [`Target::events`], pointing at that script.
///
/// This is the upgrade question. A maestro that gains an event — #131 added
/// `UserPromptSubmit` long after #106 had installed itself — leaves every
/// existing user with a present but incomplete install, which
/// [`is_installed`] cannot tell apart from a current one.
pub fn is_current(settings_path: &Path, script_path: &Path) -> bool {
/// This is the upgrade question, asked per [`Target`]. A maestro that gains
/// an event — #131 added `UserPromptSubmit` long after #106 had installed
/// itself — leaves every existing user with a present but incomplete
/// install, which [`is_installed`] cannot tell apart from a current one.
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) {
return false;
}
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)
}
@@ -173,14 +249,13 @@ fn maestro_entries(root: &serde_json::Value) -> Vec<(String, String)> {
found
}
/// What [`install`] writes for `script_path`, in the same shape.
fn expected_entries(script_path: &Path) -> Vec<(String, String)> {
let mut want: Vec<(String, String)> = HOOK_EVENTS
.iter()
.chain(PROMPT_EVENTS.iter())
/// What [`install`] writes for `target` and `script_path`, in the same shape.
fn expected_entries(target: Target, script_path: &Path) -> Vec<(String, String)> {
let mut want: Vec<(String, String)> = target
.events()
.map(|(event, state)| {
(
(*event).to_string(),
event.to_string(),
format!("{} {state}", script_path.display()),
)
})
@@ -207,28 +282,40 @@ pub fn write_script(path: &Path) -> Result<(), HooksError> {
Ok(())
}
/// Install the full hook setup for `home`: write the script under
/// `.local/share/maestro/` and register it in `.claude/settings.json`.
/// Install the full hook setup for one target under `home`: write the script
/// under `.local/share/maestro/` and register it in the target's hook file.
/// 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 settings = settings_path(home);
let settings = target.settings_path(home);
// A script left over from an older release counts as a change even when
// 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)?;
install(&settings, &script)?;
install(target, &settings, &script)?;
Ok(changed)
}
/// 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.
/// Install for every [`Target::ALL`], reporting each one separately. One
/// target failing — an unwritable file, unparseable JSON — never stops the
/// others: an agent whose hook could not be written is one agent without
/// 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
/// 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)?;
// 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);
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 mut group = serde_json::Map::new();
group.insert(
+136 -32
View File
@@ -1,15 +1,24 @@
//! 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.
//! the developer's own `~/.claude/settings.json` and `~/.codex/hooks.json`
//! are never touched.
use std::io::{Read, Write};
use std::process::{Command, Stdio};
use super::*;
/// How many commands one install writes: every activity event, plus the
/// prompt event when `haiku-autoname` is on (#131).
const INSTALLED_EVENTS: usize = HOOK_EVENTS.len() + PROMPT_EVENTS.len();
/// How many commands one install writes for a target: its activity events,
/// plus the prompt event when `haiku-autoname` is on (#131).
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 {
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.
#[test]
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"}"#);
assert!(out.status.success(), "{event:?}: {:?}", out.stderr);
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.
#[test]
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["PostToolUse"], "Active");
assert_eq!(map["Notification"], "AwaitingInput");
@@ -203,10 +212,10 @@ 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();
install(Target::ClaudeCode, &settings_path(home), &script_path(home)).unwrap();
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");
assert_eq!(joined.matches("Active").count(), 2);
assert!(joined.contains("Error"));
@@ -221,15 +230,15 @@ fn fresh_install_registers_every_event() {
fn install_is_idempotent() {
let dir = tempfile::tempdir().unwrap();
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();
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();
assert_eq!(once, twice);
assert_eq!(
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()
.join(".local/share/maestro")
.join(SCRIPT_FILE_NAME);
install(&settings, &new_script).unwrap();
install(Target::ClaudeCode, &settings, &new_script).unwrap();
let cmds = installed_commands(&settings);
assert_eq!(
cmds.len(),
INSTALLED_EVENTS,
installed_events(Target::ClaudeCode),
"old entry replaced, not kept: {cmds:?}"
);
assert!(!cmds.iter().any(|c| c == old_script));
@@ -298,7 +307,7 @@ fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() {
.path()
.join(".local/share/maestro")
.join(SCRIPT_FILE_NAME);
install(&settings, &new_script).unwrap();
install(Target::ClaudeCode, &settings, &new_script).unwrap();
let root: serde_json::Value =
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);
assert_eq!(
cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(),
INSTALLED_EVENTS,
installed_events(Target::ClaudeCode),
"{cmds:?}"
);
}
@@ -321,9 +330,15 @@ fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() {
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)));
for target in Target::ALL {
assert!(!is_installed(&target.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.
@@ -331,7 +346,9 @@ fn is_installed_tracks_reality() {
fn install_for_home_writes_an_executable_script() {
let dir = tempfile::tempdir().unwrap();
let home = dir.path();
install_for_home(home).unwrap();
for (_, result) in install_for_home(home) {
result.unwrap();
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
@@ -350,7 +367,7 @@ fn install_for_home_writes_an_executable_script() {
fn install_registers_the_prompt_event() {
let dir = tempfile::tempdir().unwrap();
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 =
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() {
let dir = tempfile::tempdir().unwrap();
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 =
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() {
let dir = tempfile::tempdir().unwrap();
let home = dir.path();
install_for_home(home).unwrap();
assert!(is_current(&settings_path(home), &script_path(home)));
install_target(Target::ClaudeCode, home).unwrap();
assert!(is_current(
Target::ClaudeCode,
&settings_path(home),
&script_path(home)
));
// Drop one event, as an older install would have left the file.
let mut root: serde_json::Value =
@@ -450,7 +471,11 @@ fn is_current_rejects_an_install_missing_an_event() {
.unwrap();
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
@@ -459,9 +484,13 @@ fn is_current_rejects_an_install_missing_an_event() {
fn is_current_rejects_a_stale_script() {
let dir = tempfile::tempdir().unwrap();
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();
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.
@@ -469,18 +498,93 @@ fn is_current_rejects_a_stale_script() {
fn is_current_rejects_a_missing_install() {
let dir = tempfile::tempdir().unwrap();
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.
#[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 home = dir.path();
assert!(install_for_home(home).unwrap());
assert!(!install_for_home(home).unwrap());
assert!(install_target(Target::ClaudeCode, home).unwrap());
assert!(!install_target(Target::ClaudeCode, home).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
the agent itself reported. Re-reporting `Error` extends the TTL rather than
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
fail rendering: treat it as `Idle` (neutral/dim, line-2 rules apply as usual)
+12 -4
View File
@@ -35,10 +35,18 @@ The branch name is used verbatim as a relative path, so `feature/login` becomes
back to a short hash of the branch.
If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
a free one. The suffix names the **directory only**. The branch is whatever was
requested; a second worktree of the same branch is not what happens here, because
git refuses to check the same branch out twice and the branch picker already
marks branches that are checked out elsewhere.
a free one.
For a *new* branch (`create_branch: true`), the suffix goes on the branch name
itself, and the directory follows since it is derived from the branch — a
random handle can otherwise reuse the name of a branch some earlier, already
removed worktree left behind, and `git worktree add -b` refuses outright even
though the directory is free.
For an *existing* branch (`create_branch: false`) the name must stay exact, so
only the directory is suffixed — a second worktree of the same branch is not
what happens here, because git refuses to check the same branch out twice and
the branch picker already marks branches that are checked out elsewhere.
### The `worktree_root` override
+33
View File
@@ -0,0 +1,33 @@
// @vitest-environment happy-dom
import { beforeEach, describe, expect, it } from "vitest";
import {
closeForceKillDialog,
openForceKillDialog,
openKillConfirmDialog,
} from "./killDialog.js";
describe("kill dialogs", () => {
beforeEach(() => {
closeForceKillDialog();
document.body.innerHTML = "";
});
it("focuses the kill button, so Enter confirms", () => {
const el = openKillConfirmDialog({
name: "alpha",
blockers: null,
onKill: () => Promise.resolve(),
});
expect(document.activeElement).toBe(el.querySelector(".kill-dialog-kill"));
});
it("focuses the force button too", () => {
const el = openForceKillDialog({
name: "alpha",
blockers: null,
reason: "worktree is dirty.",
onForce: () => Promise.resolve(),
});
expect(document.activeElement).toBe(el.querySelector(".kill-dialog-force"));
});
});
+3 -3
View File
@@ -132,9 +132,9 @@ function mount(p: KillDialogSpec): HTMLElement {
};
window.addEventListener("keydown", onKeydown, true);
open = { el, onKeydown };
// Cancel takes the focus in both prompts: a kill is never one keystroke
// away from whatever asked for the dialog.
cancel.focus();
// The acting button takes the focus in both prompts: the operator already
// asked for the kill, so Enter confirms it. Escape and the scrim cancel.
confirm.focus();
return el;
}