fix: resolve hooks from original project path in CLI workspace sessions (#593)

This commit is contained in:
Nathan Brake
2026-04-10 15:15:50 -04:00
committed by GitHub
parent 967c2b8f8e
commit 184cdeff48
6 changed files with 305 additions and 94 deletions
+61
View File
@@ -1,8 +1,69 @@
fn main() {
check_stale_build_cache();
#[cfg(feature = "serve")]
build_frontend();
}
/// Detect stale build caches by tracking Cargo.lock content hash.
///
/// When Cargo.lock changes (dependency updates, feature additions, branch
/// switches in worktrees), the target/ directory can contain incompatible
/// artifacts that cause cryptic compilation errors like "can't find crate"
/// or "found possibly newer version of crate." This check catches that
/// early with a clear message instead of letting the build fail inscrutably.
fn check_stale_build_cache() {
use std::path::Path;
// Re-run this check whenever Cargo.lock changes.
println!("cargo:rerun-if-changed=Cargo.lock");
let lockfile = Path::new("Cargo.lock");
let target_dir = std::env::var("OUT_DIR")
.ok()
.and_then(|out| {
// OUT_DIR is something like target/debug/build/agent-of-empires-xxx/out
// Walk up to find the target/ root.
let mut p = Path::new(&out).to_path_buf();
while p.pop() {
if p.file_name().is_some_and(|n| n == "target") {
return Some(p);
}
}
None
})
.unwrap_or_else(|| Path::new("target").to_path_buf());
let hash_file = target_dir.join(".cargo-lock-hash");
let Ok(lock_content) = std::fs::read(lockfile) else {
return; // No Cargo.lock, nothing to check.
};
// Simple, fast hash: use the file length + first/last 1KB as a fingerprint.
// This avoids pulling in a hash crate in build.rs.
let len = lock_content.len();
let head: u64 = lock_content[..len.min(1024)]
.iter()
.fold(0u64, |acc, &b| acc.wrapping_mul(31).wrapping_add(b as u64));
let tail: u64 = lock_content[len.saturating_sub(1024)..]
.iter()
.fold(0u64, |acc, &b| acc.wrapping_mul(31).wrapping_add(b as u64));
let current_hash = format!("{:x}{:x}{:x}", len, head, tail);
if let Ok(stored_hash) = std::fs::read_to_string(&hash_file) {
if stored_hash.trim() != current_hash {
println!(
"cargo:warning=Cargo.lock changed since last build. \
If you see strange compilation errors, run `cargo clean`."
);
}
}
// Always update the stored hash.
let _ = std::fs::write(&hash_file, &current_hash);
}
#[cfg(feature = "serve")]
fn build_frontend() {
use std::path::Path;
+2
View File
@@ -0,0 +1,2 @@
[toolchain]
channel = "stable"
+57 -54
View File
@@ -2,7 +2,7 @@
use anyhow::{bail, Result};
use clap::Args;
use std::path::{Path, PathBuf};
use std::path::PathBuf;
use crate::containers::{self, ContainerRuntimeInterface};
use crate::session::builder;
@@ -89,6 +89,11 @@ pub async fn run(profile: &str, args: AddArgs) -> Result<()> {
let config = repo_config::resolve_config_with_repo(profile, &path).unwrap_or_default();
// Preserve the original project path for hook trust checking.
// `path` gets reassigned to the worktree/workspace directory below,
// but hooks are defined in the original repo's `.agent-of-empires/config.toml`.
let original_project_path = path.clone();
let mut worktree_info_opt = None;
let mut workspace_info_opt = None;
@@ -296,50 +301,63 @@ pub async fn run(profile: &str, args: AddArgs) -> Result<()> {
}
}
// Check for repository hooks
// Check for repository hooks.
// Use the original project path for trust checking (not the worktree/workspace
// path, which won't contain `.agent-of-empires/config.toml`).
let hook_result: Result<()> = (|| {
match repo_config::check_hook_trust(&path) {
Ok(repo_config::HookTrustStatus::NeedsTrust { hooks, hooks_hash }) => {
let should_trust = if args.trust_hooks {
true
} else {
println!("\nRepository hooks detected in .agent-of-empires/config.toml:");
if !hooks.on_create.is_empty() {
println!(" on_create:");
for cmd in &hooks.on_create {
println!(" {}", cmd);
let resolved_hooks: Option<crate::session::HooksConfig> =
match repo_config::check_hook_trust(&original_project_path) {
Ok(repo_config::HookTrustStatus::NeedsTrust { hooks, hooks_hash }) => {
let should_trust = if args.trust_hooks {
true
} else {
println!("\nRepository hooks detected in .agent-of-empires/config.toml:");
if !hooks.on_create.is_empty() {
println!(" on_create:");
for cmd in &hooks.on_create {
println!(" {}", cmd);
}
}
}
if !hooks.on_launch.is_empty() {
println!(" on_launch:");
for cmd in &hooks.on_launch {
println!(" {}", cmd);
if !hooks.on_launch.is_empty() {
println!(" on_launch:");
for cmd in &hooks.on_launch {
println!(" {}", cmd);
}
}
}
print!("\nTrust and run these hooks? [y/N] ");
use std::io::Write;
std::io::stdout().flush()?;
let mut input = String::new();
std::io::stdin().read_line(&mut input)?;
input.trim().eq_ignore_ascii_case("y")
};
print!("\nTrust and run these hooks? [y/N] ");
use std::io::Write;
std::io::stdout().flush()?;
let mut input = String::new();
std::io::stdin().read_line(&mut input)?;
input.trim().eq_ignore_ascii_case("y")
};
if should_trust {
trust_and_run_on_create(&path, &hooks_hash, &hooks)?;
} else {
println!("Hooks skipped (session created without running hooks)");
if should_trust {
repo_config::trust_repo(&original_project_path, &hooks_hash)?;
println!("✓ Repository hooks trusted");
repo_config::merge_hooks_with_config(profile, hooks)
} else {
println!("Hooks skipped (session created without running hooks)");
None
}
}
}
Ok(repo_config::HookTrustStatus::Trusted(hooks)) => {
if !hooks.on_create.is_empty() {
println!("Running on_create hooks...");
repo_config::execute_hooks(&hooks.on_create, &path)?;
println!("✓ on_create hooks completed");
Ok(repo_config::HookTrustStatus::Trusted(repo_hooks)) => {
repo_config::merge_hooks_with_config(profile, repo_hooks)
}
}
Ok(repo_config::HookTrustStatus::NoHooks) => {}
Err(e) => {
tracing::warn!("Failed to check repo hooks: {}", e);
Ok(repo_config::HookTrustStatus::NoHooks) => {
repo_config::resolve_global_profile_hooks(profile)
}
Err(e) => {
tracing::warn!("Failed to check repo hooks: {}", e);
repo_config::resolve_global_profile_hooks(profile)
}
};
if let Some(hooks) = resolved_hooks {
if !hooks.on_create.is_empty() {
println!("Running on_create hooks...");
repo_config::execute_hooks(&hooks.on_create, &path)?;
println!("✓ on_create hooks completed");
}
}
Ok(())
@@ -437,21 +455,6 @@ pub fn is_duplicate_session(instances: &[Instance], title: &str, path: &str) ->
})
}
fn trust_and_run_on_create(
project_path: &Path,
hooks_hash: &str,
hooks: &crate::session::HooksConfig,
) -> Result<()> {
repo_config::trust_repo(project_path, hooks_hash)?;
println!("✓ Repository hooks trusted");
if !hooks.on_create.is_empty() {
println!("Running on_create hooks...");
repo_config::execute_hooks(&hooks.on_create, project_path)?;
println!("✓ on_create hooks completed");
}
Ok(())
}
fn detect_tool(cmd: &str) -> Result<String> {
crate::agents::resolve_tool_name(cmd)
.map(|name| name.to_string())
+37
View File
@@ -456,6 +456,43 @@ pub fn check_hook_trust(project_path: &Path) -> Result<HookTrustStatus> {
}
}
// ---------------------------------------------------------------------------
// Hook resolution helpers (shared by CLI and TUI)
// ---------------------------------------------------------------------------
/// Resolve hooks from global+profile config when no repo hooks are defined.
/// Returns `None` if no on_create or on_launch hooks are configured.
pub fn resolve_global_profile_hooks(profile: &str) -> Option<HooksConfig> {
let config = super::profile_config::resolve_config(profile).ok()?;
if config.hooks.on_create.is_empty() && config.hooks.on_launch.is_empty() {
None
} else {
Some(config.hooks)
}
}
/// Merge trusted repo hooks onto the global+profile base config.
/// Repo hooks override (not append) global hooks per-field.
/// Returns `None` if the merged result has no on_create or on_launch hooks.
pub fn merge_hooks_with_config(profile: &str, repo_hooks: HooksConfig) -> Option<HooksConfig> {
let mut base = super::profile_config::resolve_config(profile)
.map(|c| c.hooks)
.unwrap_or_default();
if !repo_hooks.on_create.is_empty() {
base.on_create = repo_hooks.on_create;
}
if !repo_hooks.on_launch.is_empty() {
base.on_launch = repo_hooks.on_launch;
}
if base.on_create.is_empty() && base.on_launch.is_empty() {
None
} else {
Some(base)
}
}
// ---------------------------------------------------------------------------
// Hook execution
// ---------------------------------------------------------------------------
+6 -40
View File
@@ -201,11 +201,12 @@ impl HomeView {
tracing::error!("Failed to trust repo: {}", e);
}
let merged =
self.merge_repo_hooks_onto_config_for(&data.profile, hooks);
repo_config::merge_hooks_with_config(&data.profile, hooks);
return self.create_session_with_hooks(data, merged);
}
HookTrustAction::Skip => {
let fallback = self.resolve_global_profile_hooks_for(&data.profile);
let fallback =
repo_config::resolve_global_profile_hooks(&data.profile);
return self.create_session_with_hooks(data, fallback);
}
}
@@ -1105,16 +1106,16 @@ impl HomeView {
None
}
Ok(repo_config::HookTrustStatus::Trusted(repo_hooks)) => {
let merged = self.merge_repo_hooks_onto_config_for(&data.profile, repo_hooks);
let merged = repo_config::merge_hooks_with_config(&data.profile, repo_hooks);
self.create_session_with_hooks(data, merged)
}
Ok(repo_config::HookTrustStatus::NoHooks) => {
let fallback = self.resolve_global_profile_hooks_for(&data.profile);
let fallback = repo_config::resolve_global_profile_hooks(&data.profile);
self.create_session_with_hooks(data, fallback)
}
Err(e) => {
tracing::warn!("Failed to check repo hooks: {}", e);
let fallback = self.resolve_global_profile_hooks_for(&data.profile);
let fallback = repo_config::resolve_global_profile_hooks(&data.profile);
self.create_session_with_hooks(data, fallback)
}
}
@@ -1171,39 +1172,4 @@ impl HomeView {
// No mouse handling for other views currently
None
}
/// Resolve hooks from global+profile config for the given profile.
fn resolve_global_profile_hooks_for(
&self,
profile: &str,
) -> Option<crate::session::HooksConfig> {
let config = resolve_config(profile).ok()?;
if config.hooks.on_create.is_empty() && config.hooks.on_launch.is_empty() {
None
} else {
Some(config.hooks)
}
}
/// Merge trusted repo hooks onto the resolved config for the given profile.
fn merge_repo_hooks_onto_config_for(
&self,
profile: &str,
repo_hooks: crate::session::HooksConfig,
) -> Option<crate::session::HooksConfig> {
let mut base = resolve_config(profile).map(|c| c.hooks).unwrap_or_default();
if !repo_hooks.on_create.is_empty() {
base.on_create = repo_hooks.on_create;
}
if !repo_hooks.on_launch.is_empty() {
base.on_launch = repo_hooks.on_launch;
}
if base.on_create.is_empty() && base.on_launch.is_empty() {
None
} else {
Some(base)
}
}
}
+142
View File
@@ -1,4 +1,5 @@
use serial_test::serial;
use std::path::Path;
use std::process::Command;
use crate::harness::{require_tmux, TuiTestHarness};
@@ -550,3 +551,144 @@ fn test_cli_rename_preserves_tmux_session() {
.args(["kill-session", "-t", &new_tmux_name])
.output();
}
/// Initialize a bare-minimum git repo at the given path so worktree operations work.
fn init_git_repo(path: &Path) {
std::fs::create_dir_all(path).expect("create repo dir");
let init = Command::new("git")
.args(["init"])
.current_dir(path)
.output()
.expect("git init");
assert!(init.status.success(), "git init failed");
// Need at least one commit for worktree creation.
let _ = Command::new("git")
.args(["commit", "--allow-empty", "-m", "init"])
.current_dir(path)
.env("GIT_AUTHOR_NAME", "test")
.env("GIT_AUTHOR_EMAIL", "test@test.com")
.env("GIT_COMMITTER_NAME", "test")
.env("GIT_COMMITTER_EMAIL", "test@test.com")
.output();
}
/// Regression test for #591: repo on_create hooks should execute for multi-repo
/// workspace sessions created via `aoe add --repo`.
#[test]
#[serial]
fn test_cli_add_workspace_repo_hooks_execute() {
let h = TuiTestHarness::new("cli_workspace_hooks");
let project_a = h.home_path().join("project-a");
let project_b = h.home_path().join("project-b");
init_git_repo(&project_a);
init_git_repo(&project_b);
// Set up repo-level hooks in project-a.
let hook_marker = h.home_path().join("hook-ran.marker");
let aoe_config_dir = project_a.join(".agent-of-empires");
std::fs::create_dir_all(&aoe_config_dir).expect("create .agent-of-empires dir");
let config = format!(
"[hooks]\non_create = [\"touch {}\"]\n",
hook_marker.display()
);
std::fs::write(aoe_config_dir.join("config.toml"), &config).expect("write repo config");
let add_output = h.run_cli(&[
"add",
project_a.to_str().unwrap(),
"--repo",
project_b.to_str().unwrap(),
"-w",
"feat/hook-test",
"-b",
"-t",
"HookTest",
"--trust-hooks",
]);
let stdout = String::from_utf8_lossy(&add_output.stdout);
let stderr = String::from_utf8_lossy(&add_output.stderr);
assert!(
add_output.status.success(),
"aoe add --repo failed:\nstdout: {}\nstderr: {}",
stdout,
stderr
);
assert!(
stdout.contains("on_create hooks completed"),
"should print hook completion message.\nstdout: {}",
stdout
);
assert!(
hook_marker.exists(),
"hook marker file should exist, proving on_create hooks ran"
);
}
/// Regression test for #591: global hooks should execute as fallback when no
/// repo hooks are defined, even for workspace sessions.
#[test]
#[serial]
fn test_cli_add_workspace_global_hook_fallback() {
let h = TuiTestHarness::new("cli_workspace_global_hooks");
let project_a = h.home_path().join("project-a");
let project_b = h.home_path().join("project-b");
init_git_repo(&project_a);
init_git_repo(&project_b);
// Set up global hooks (no repo config).
let hook_marker = h.home_path().join("global-hook-ran.marker");
let config_dir = if cfg!(target_os = "linux") {
h.home_path().join(".config/agent-of-empires")
} else {
h.home_path().join(".agent-of-empires")
};
let config_content = format!(
r#"[updates]
check_enabled = false
[app_state]
has_seen_welcome = true
last_seen_version = "{}"
[hooks]
on_create = ["touch {}"]
"#,
env!("CARGO_PKG_VERSION"),
hook_marker.display()
);
std::fs::write(config_dir.join("config.toml"), config_content).expect("write global config");
let add_output = h.run_cli(&[
"add",
project_a.to_str().unwrap(),
"--repo",
project_b.to_str().unwrap(),
"-w",
"feat/global-hook-test",
"-b",
"-t",
"GlobalHookTest",
]);
let stdout = String::from_utf8_lossy(&add_output.stdout);
let stderr = String::from_utf8_lossy(&add_output.stderr);
assert!(
add_output.status.success(),
"aoe add --repo failed:\nstdout: {}\nstderr: {}",
stdout,
stderr
);
assert!(
stdout.contains("on_create hooks completed"),
"should print hook completion message for global hooks.\nstdout: {}",
stdout
);
assert!(
hook_marker.exists(),
"global hook marker file should exist, proving global on_create hooks ran as fallback"
);
}