Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions crates/agent-runtime/src/claude/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ pub fn run_hook_client(socket: &Path) -> i32 {
if std::io::stdin().read_to_string(&mut input).is_err() {
// Nothing to decide about. Exiting 1 is a non-blocking error, which leaves
// the runtime's own flow intact rather than blocking a call Tervin never saw.
eprintln!("Tervin hook: could not read the tool call from stdin.");
eprintln!("{HOOK_STDERR_PREFIX}could not read the tool call from stdin.");
return 1;
}

Expand All @@ -470,8 +470,8 @@ pub fn run_hook_client(socket: &Path) -> i32 {
// Tervin is gone. Say so loudly rather than failing silently: the user
// believes actions are being gated, and right now they are not.
eprintln!(
"Tervin hook: could not reach Tervin at {} ({e}). This tool call was \
NOT checked against Tervin Rules.",
"{HOOK_STDERR_PREFIX}could not reach Tervin at {} ({e}). This tool call \
was NOT checked against Tervin Rules.",
socket.display()
);
return 1;
Expand All @@ -485,7 +485,7 @@ pub fn run_hook_client(socket: &Path) -> i32 {
let mut payload = input.trim().replace('\n', " ");
payload.push('\n');
if stream.write_all(payload.as_bytes()).is_err() || stream.flush().is_err() {
eprintln!("Tervin hook: could not send the tool call to Tervin.");
eprintln!("{HOOK_STDERR_PREFIX}could not send the tool call to Tervin.");
return 1;
}

Expand All @@ -494,12 +494,12 @@ pub fn run_hook_client(socket: &Path) -> i32 {
.is_err()
|| response.trim().is_empty()
{
eprintln!("Tervin hook: Tervin did not answer within {HOOK_TIMEOUT_SECS}s.");
eprintln!("{HOOK_STDERR_PREFIX}Tervin did not answer within {HOOK_TIMEOUT_SECS}s.");
return 1;
}

let Ok(value) = serde_json::from_str::<Value>(response.trim()) else {
eprintln!("Tervin hook: Tervin's answer could not be read.");
eprintln!("{HOOK_STDERR_PREFIX}Tervin's answer could not be read.");
return 1;
};

Expand All @@ -525,6 +525,18 @@ pub fn run_hook_client(socket: &Path) -> i32 {
/// The flag that turns Tervin's own executable into the hook.
pub const HOOK_FLAG: &str = "--tervin-hook";

/// The prefix every message the hook client prints to stderr carries.
///
/// This is how the normalizer recognises the gate's own failures. The runtime echoes
/// the hook's command line back only when the hook *blocked*; a hook that merely
/// failed carries nothing but its own stderr, so without a marker of its own every
/// gate failure was reported as the user's configuration.
///
/// It is deliberately the same string a reader sees, rather than a hidden token: the
/// text is already user-facing, and a sentinel nobody can read is one nobody
/// maintains. Denials are exempt — their stderr is fed back to the agent verbatim.
pub const HOOK_STDERR_PREFIX: &str = "Tervin hook: ";

/// Recognise a hook invocation before any UI starts.
///
/// The workspace binary doubles as the hook command so the path is exact and no
Expand Down
71 changes: 68 additions & 3 deletions crates/agent-runtime/src/claude/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,15 @@ impl Normalizer {
.filter(|s| !s.is_empty())
.map(String::from);

// Tervin's gate identifies itself by the flag in its own command line, which
// the runtime echoes back in the hook's error text.
let is_tervin = value.to_string().contains(crate::claude::hooks::HOOK_FLAG);
// Tervin's gate identifies itself two ways, and it needs both. The runtime
// echoes the hook's command line back only when the hook *blocked*; a hook that
// merely failed carries nothing but its own stderr. Matching on the command line
// alone therefore recognised the gate exactly when it worked and missed it every
// time it broke — so Tervin reported its own failures as the user's hooks.
let is_tervin = value.to_string().contains(crate::claude::hooks::HOOK_FLAG)
|| stderr
.as_deref()
.is_some_and(|s| s.contains(crate::claude::hooks::HOOK_STDERR_PREFIX));

self.hook_runs.push(crate::runtime::HookRun {
name: name.clone(),
Expand Down Expand Up @@ -1829,6 +1835,65 @@ mod tests {
assert!(n.denials.is_empty(), "the gate records its own denials");
}

#[test]
fn a_gate_that_failed_is_still_recognised_as_tervins_own() {
// The case that was wrong. When the gate blocks, the runtime echoes its command
// line back and the flag is there to find. When the gate merely fails there is
// no command line — only the hook's own stderr — so matching on the flag alone
// recognised the gate exactly when it worked and missed it every time it broke.
// Tervin then showed its own dead socket as a hook the user had configured.
let mut n = normalizer();
let events = n.ingest(&serde_json::json!({
"type": "system", "subtype": "hook_response",
"hook_name": "PreToolUse:Bash", "hook_event": "PreToolUse",
"stderr": "Tervin hook: Tervin did not answer within 5s.",
"exit_code": 1, "outcome": "error"
}));

assert!(
events.is_empty(),
"Tervin's own failure is not a diagnostic about the user's setup: {events:?}"
);
assert_eq!(n.hook_runs.len(), 1);
assert!(
n.hook_runs[0].is_tervin,
"a gate failure carries no command line, so the stderr prefix has to carry it"
);
}

#[test]
fn the_hook_client_and_the_normalizer_agree_on_the_prefix() {
// The two ends drifting apart is what caused the misattribution, and nothing
// else would catch it: each side compiles perfectly well on its own.
let mut n = normalizer();
n.ingest(&serde_json::json!({
"type": "system", "subtype": "hook_response",
"hook_name": "PreToolUse:Bash", "hook_event": "PreToolUse",
"stderr": format!(
"{}could not reach Tervin at /run/h.sock (No such file or directory). \
This tool call was NOT checked against Tervin Rules.",
crate::claude::hooks::HOOK_STDERR_PREFIX
),
"exit_code": 1, "outcome": "error"
}));
assert!(n.hook_runs[0].is_tervin);
}

#[test]
fn a_user_hook_that_fails_is_still_reported_as_theirs() {
// The other half of the fix: widening the match must not swallow the user's own
// broken hooks, which are the reason the diagnostic exists at all.
let mut n = normalizer();
let events = n.ingest(&serde_json::json!({
"type": "system", "subtype": "hook_response",
"hook_name": "PreToolUse:Bash", "hook_event": "PreToolUse",
"stderr": "my-gate.sh: line 3: jq: command not found",
"exit_code": 127, "outcome": "error"
}));
assert!(!n.hook_runs[0].is_tervin);
assert!(events.iter().any(|e| e.kind() == "diagnostic.detected"));
}

#[test]
fn an_error_result_fails_the_thread() {
let mut n = normalizer();
Expand Down
105 changes: 98 additions & 7 deletions crates/agent-runtime/src/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,53 @@ fn candidates_from_acp_agents() -> Vec<ImportCandidate> {
.collect()
}

/// How long the user's shell gets to list its aliases before Tervin stops caring.
const ALIAS_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);

/// Run a command, collect its standard output, and give up after `limit`.
///
/// `std::process::Command` offers no timeout, and the one thing worse than a
/// discovery that finds nothing is a discovery that never returns. Returns `None`
/// if the command could not start, ran out of time, or was killed — all of which
/// mean the same thing to every caller here: no answer.
fn run_briefly(command: &mut std::process::Command, limit: std::time::Duration) -> Option<Vec<u8>> {
let mut child = command
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::null())
.spawn()
.ok()?;

// Drain the pipe on another thread. A child that fills the pipe buffer blocks
// until someone reads it, so waiting for exit without draining would be waiting
// for a child that is itself waiting for us.
let mut pipe = child.stdout.take()?;
let (tx, rx) = std::sync::mpsc::channel();
std::thread::spawn(move || {
let mut buf = Vec::new();
let _ = std::io::Read::read_to_end(&mut pipe, &mut buf);
let _ = tx.send(buf);
});

let deadline = std::time::Instant::now() + limit;
loop {
match child.try_wait() {
Ok(Some(_)) => break,
Ok(None) if std::time::Instant::now() < deadline => {
std::thread::sleep(std::time::Duration::from_millis(25));
}
// Out of time, or a child that cannot be waited on at all. Kill it either
// way: whatever the rc files are still doing, nobody is waiting for it now.
_ => {
let _ = child.kill();
let _ = child.wait();
return None;
}
}
}

rx.recv_timeout(limit).ok()
}

/// Ask the user's shell for its aliases and parse agent-launching ones.
///
/// Runs `$SHELL -ic alias`, which sources the user's rc files. That is the only
Expand All @@ -344,15 +391,19 @@ fn candidates_from_aliases() -> Vec<ImportCandidate> {
_ => return Vec::new(),
};

let output = std::process::Command::new(&shell)
.args(["-ic", "alias"])
.stdin(std::process::Stdio::null())
.output();

let Ok(output) = output else {
// An interactive shell runs whatever the user's rc files contain, and some of
// that waits: version managers that hit the network, prompt frameworks, a stray
// `read`. Aliases are a convenience, so a shell that will not answer promptly is
// simply one that offers nothing — never a reason to keep the caller waiting.
let Some(stdout) = run_briefly(
std::process::Command::new(&shell)
.args(["-ic", "alias"])
.stdin(std::process::Stdio::null()),
ALIAS_TIMEOUT,
) else {
return Vec::new();
};
let text = String::from_utf8_lossy(&output.stdout);
let text = String::from_utf8_lossy(&stdout);

text.lines()
.filter_map(parse_alias_line)
Expand Down Expand Up @@ -569,6 +620,46 @@ fn shell_words_split(input: &str) -> Option<Vec<String>> {
mod tests {
use super::*;

#[test]
fn a_command_that_answers_is_read_in_full() {
let out = run_briefly(
std::process::Command::new("sh").args(["-c", "echo alias-one; echo alias-two"]),
std::time::Duration::from_secs(10),
)
.expect("a prompt command should produce output");
let text = String::from_utf8_lossy(&out);
assert!(
text.contains("alias-one") && text.contains("alias-two"),
"{text}"
);
}

#[test]
fn a_command_that_hangs_is_abandoned_rather_than_waited_on() {
// The bug this exists to prevent: `$SHELL -ic alias` sources rc files, and an
// rc file that blocks used to block the whole agents view behind it.
let started = std::time::Instant::now();
let out = run_briefly(
std::process::Command::new("sh").args(["-c", "sleep 30"]),
std::time::Duration::from_millis(300),
);
assert!(out.is_none(), "a hung command has no answer to give");
assert!(
started.elapsed() < std::time::Duration::from_secs(5),
"gave up after {:?}, which is not giving up",
started.elapsed()
);
}

#[test]
fn a_command_that_does_not_exist_is_not_an_answer() {
assert!(run_briefly(
&mut std::process::Command::new("tervin-no-such-binary-anywhere"),
std::time::Duration::from_secs(5),
)
.is_none());
}

#[test]
fn parses_a_config_dir_alias() {
// The exact shape people actually use for multiple accounts.
Expand Down
48 changes: 38 additions & 10 deletions crates/tervin-app/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,13 +782,19 @@ pub async fn audit_recent(

// ============================================================ agents

/// What the user configured, read from disk and nothing else.
///
/// Deliberately separate from [`AgentsDiscovery`]. These two answer different
/// questions — "what did I set up?" and "what is on this machine?" — and only the
/// second one has to leave the process to find out. Serving them together meant a
/// profile the user had written by hand could not be shown until every agent binary
/// on the machine had been probed, and could not be shown *at all* if one of those
/// probes failed: the whole command returned an error, the UI had no profiles, and
/// it said "No agent profile configured" to a user with five of them.
#[derive(Debug, Serialize)]
pub struct AgentsOverview {
pub profiles: Vec<AgentProfile>,
pub default_profile: Option<String>,
pub discovered: Vec<agent_runtime::Discovery>,
/// Profiles Tervin found but has not adopted.
pub import_candidates: Vec<ImportCandidate>,
/// Where the files the UI mentions actually are.
///
/// Resolved rather than written into the interface, because the location differs by
Expand All @@ -798,8 +804,29 @@ pub struct AgentsOverview {
pub mcp_path: String,
}

/// What Tervin found on the machine. Every field here costs a subprocess.
#[derive(Debug, Serialize)]
pub struct AgentsDiscovery {
pub discovered: Vec<agent_runtime::Discovery>,
/// Profiles Tervin found but has not adopted.
pub import_candidates: Vec<ImportCandidate>,
}

/// The user's own configuration. Cheap, local, and cannot fail on a missing binary.
#[tauri::command]
pub async fn agents_overview(state: State<'_, Arc<AppState>>) -> Result<AgentsOverview> {
let profiles = state.profiles.read().clone();
Ok(AgentsOverview {
default_profile: profiles.default_profile,
profiles: profiles.profiles,
profiles_path: tervin_core::paths::abbreviate(&ProfileConfig::path()),
mcp_path: tervin_core::paths::abbreviate(&agent_runtime::McpConfig::path()),
})
}

/// What is installed. Slow and failure-prone by nature, which is why it stands alone.
#[tauri::command]
pub async fn agents_discovery(state: State<'_, Arc<AppState>>) -> Result<AgentsDiscovery> {
// Snapshot under the lock, then release it: discovery spawns processes and
// must not hold the registry lock while it awaits them.
let adapters = state.agents.read().snapshot();
Expand All @@ -811,8 +838,13 @@ pub async fn agents_overview(state: State<'_, Arc<AppState>>) -> Result<AgentsOv
discovered.push(agent_runtime::registry::discover_generic(agent).await);
}

let profiles = state.profiles.read().clone();
let existing: Vec<String> = profiles.profiles.iter().map(|p| p.id.clone()).collect();
let existing: Vec<String> = state
.profiles
.read()
.profiles
.iter()
.map(|p| p.id.clone())
.collect();

let import_candidates = blocking(move || {
Ok(agent_runtime::profile::import_candidates()
Expand All @@ -822,13 +854,9 @@ pub async fn agents_overview(state: State<'_, Arc<AppState>>) -> Result<AgentsOv
})
.await?;

Ok(AgentsOverview {
default_profile: profiles.default_profile.clone(),
profiles: profiles.profiles.clone(),
Ok(AgentsDiscovery {
discovered,
import_candidates,
profiles_path: tervin_core::paths::abbreviate(&ProfileConfig::path()),
mcp_path: tervin_core::paths::abbreviate(&agent_runtime::McpConfig::path()),
})
}

Expand Down
1 change: 1 addition & 0 deletions crates/tervin-app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub fn run() -> anyhow::Result<()> {
commands::audit_recent,
// agents
commands::agents_overview,
commands::agents_discovery,
commands::project_instructions,
commands::agents_add_acp,
commands::agents_add_local_model,
Expand Down
Loading