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
79 changes: 61 additions & 18 deletions crates/agent-runtime/src/claude/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,22 @@
//! configuration is never read, rewritten, or overridden.
//! 3. Before each tool call the agent runs that command, handing it the tool name
//! and arguments on stdin.
//! 4. The command asks the running Tervin, which consults Tervin Rules, and prints
//! the decision.
//! 4. The command asks the running Tervin, which consults Tervin Rules. A refusal is
//! written to stderr and exits 2. Anything else says nothing and exits 0.
//!
//! ## Tervin can only tighten, never loosen
//!
//! The protocol offers `allow`, which skips the runtime's *own* permission checks.
//! Tervin never returns it. When Rules do not object it returns `defer`, which
//! means "no opinion — carry on through the normal flow". So enabling this gate can
//! only ever add a refusal; it can never turn an action the runtime would have
//! asked about into one it performs silently. A safety feature that quietly
//! disables another safety feature is not one.
//! Tervin never returns it. When Rules do not object the hook says **nothing at all**
//! and exits 0, which is how this protocol spells "no opinion — carry on through the
//! normal flow". So enabling this gate can only ever add a refusal; it can never turn
//! an action the runtime would have asked about into one it performs silently. A
//! safety feature that quietly disables another safety feature is not one.
//!
//! Saying "no opinion" out loud is not equivalent to staying quiet. The runtime
//! accepts `allow`, `deny` and `ask` and nothing else; anything else ends the turn
//! immediately and reports success. Since this gate sees every tool call, that
//! landed on the first one and killed every Thread at its first action.
//!
//! ## What this gate cannot do
//!
Expand Down Expand Up @@ -83,7 +88,11 @@ pub enum HookDecision {
}

impl HookDecision {
/// The exact JSON the runtime expects on stdout.
/// The decision as it travels from Tervin to its own hook client.
///
/// Not what the client prints. `defer` is Tervin's word for "no objection" and
/// is meaningless to the runtime, which would end the turn on being handed it —
/// so only a denial reaches the runtime, as a reason on stderr with exit 2.
pub fn to_json(&self) -> Value {
let (decision, reason) = match self {
Self::Deny { reason } => ("deny", reason),
Expand Down Expand Up @@ -518,7 +527,18 @@ pub fn run_hook_client(socket: &Path) -> i32 {
return 2;
}

println!("{decision}");
// Silence is how this protocol spells "no opinion", and saying anything else
// here is not the harmless no-op it looks like.
//
// The runtime accepts `allow`, `deny` and `ask`. It does not accept `defer`, and
// on receiving one it **ends the turn on the spot** and reports success: no tool
// result, no continuation, the agent simply stops mid-task. Because Tervin gates
// every tool call, that fired on the *first* one, so every Thread died at its
// first action while the gate panel said it was protecting the session. Printing
// a decision that meant "carry on" was the thing stopping everything.
//
// Tervin only ever tightens. When Rules do not object there is nothing to say,
// and the exit code alone says it.
0
}

Expand Down Expand Up @@ -742,7 +762,7 @@ mod tests {
}))
.unwrap();

let (code, stderr) = run_client_capturing(&socket, &payload).await;
let (code, _out, stderr) = run_client_capturing(&socket, &payload).await;
assert_eq!(code, 2, "a denial must exit 2 or the tool runs anyway");
assert!(
stderr.contains("test policy"),
Expand All @@ -768,7 +788,7 @@ mod tests {

let socket = gate.socket_path().to_path_buf();
let payload = r#"{"hook_event_name":"PreToolUse","tool_name":"Read","tool_input":{"file_path":"/p/a.rs"},"cwd":"/tmp"}"#;
let (code, _) = run_client_capturing(&socket, payload).await;
let (code, _out, _err) = run_client_capturing(&socket, payload).await;
assert_eq!(code, 0);
assert!(gate.denials().is_empty());
}
Expand All @@ -787,7 +807,7 @@ mod tests {
.unwrap();

let socket = gate.socket_path().to_path_buf();
let (code, _) = run_client_capturing(&socket, "this is not json").await;
let (code, _out, _err) = run_client_capturing(&socket, "this is not json").await;
assert_eq!(code, 2, "input Tervin cannot read must not be allowed");
}

Expand All @@ -797,7 +817,7 @@ mod tests {
// believes actions are being checked.
let dir = tempfile::tempdir().unwrap();
let missing = dir.path().join("nothing.sock");
let (code, stderr) = run_client_capturing(&missing, "{}").await;
let (code, _out, stderr) = run_client_capturing(&missing, "{}").await;
// Not 2: blocking on Tervin being down would make an unrelated crash stop
// the user's work.
assert_eq!(code, 1);
Expand Down Expand Up @@ -1000,7 +1020,7 @@ mod tests {
let socket = gate.socket_path().to_path_buf();

let deny = r#"{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"rm -rf /"},"cwd":"/tmp"}"#;
let (code, err) = run_client_capturing(&socket, deny).await;
let (code, _out, err) = run_client_capturing(&socket, deny).await;
assert_ne!(
code, 1,
"exit 1 means the client never got an answer: {err}"
Expand All @@ -1011,10 +1031,23 @@ mod tests {
);

let allow = r#"{"hook_event_name":"PreToolUse","tool_name":"Read","tool_input":{"file_path":"/p/a.rs"},"cwd":"/tmp"}"#;
let (code, err) = run_client_capturing(&socket, allow).await;
let (code, out, err) = run_client_capturing(&socket, allow).await;
assert_ne!(code, 1, "exit 1 means no answer: {err}");
assert_eq!(code, 0, "a permitted action defers, which exits 0: {err}");

// Nothing on stdout, and this is the assertion the gate most needs.
//
// The runtime accepts `allow`, `deny` and `ask`. Handed anything else — such
// as Tervin's own word for "no objection" — it ends the turn immediately and
// reports success, so the agent stops at its first tool call having done
// nothing. Every Thread died that way while the exit code, the stderr and
// the audit trail all said the gate was working correctly, which is exactly
// why stdout is captured here at all.
assert!(
out.trim().is_empty(),
"the gate must say nothing when it does not object, got: {out:?}"
);

// The denial is recorded, because a refusal that is not inspectable
// afterwards is not an audit trail.
assert_eq!(gate.denials().len(), 1, "the deny should be recorded once");
Expand Down Expand Up @@ -1053,8 +1086,12 @@ mod tests {

let mut codes = Vec::new();
for task in tasks {
let (code, err) = task.await.unwrap();
let (code, out, err) = task.await.unwrap();
assert_ne!(code, 1, "a concurrent call went unanswered: {err}");
assert!(
out.trim().is_empty(),
"no call may print a decision the runtime would choke on: {out:?}"
);
codes.push(code);
}
codes.sort_unstable();
Expand All @@ -1079,12 +1116,17 @@ mod tests {
}
}

/// Run the real client against a socket, capturing its exit code and stderr.
/// Run the real client against a socket, capturing exit code, stdout and stderr.
///
/// The client is deliberately a blocking, process-shaped function, so it is
/// driven here the same way the runtime drives it: a child process with the
/// payload on stdin.
async fn run_client_capturing(socket: &Path, payload: &str) -> (i32, String) {
///
/// Stdout is captured and returned because it was once discarded here, and the
/// gate's worst bug lived in it: the client printed a decision the runtime does
/// not accept, which ended every Thread at its first tool call. Exit codes and
/// stderr were asserted throughout and all of them were correct.
async fn run_client_capturing(socket: &Path, payload: &str) -> (i32, String, String) {
let socket = socket.to_path_buf();
let payload = payload.to_string();
tokio::task::spawn_blocking(move || {
Expand All @@ -1111,6 +1153,7 @@ mod tests {
let out = child.wait_with_output().expect("client did not finish");
(
out.status.code().unwrap_or(-1),
String::from_utf8_lossy(&out.stdout).to_string(),
String::from_utf8_lossy(&out.stderr).to_string(),
)
})
Expand Down
16 changes: 10 additions & 6 deletions crates/agent-runtime/src/claude/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,12 +613,6 @@ pub struct ClaudeSession {
}

impl ClaudeSession {
/// Take the raw-payload stream, so the store can persist what the runtime
/// actually said behind each event.
pub fn take_raw_stream(&self) -> Option<mpsc::UnboundedReceiver<(String, String)>> {
self.raw_rx.lock().take()
}

async fn write_line(&self, value: &Value) -> Result<()> {
if !self.shared.running.load(Ordering::SeqCst) {
return Err(RuntimeError::SessionEnded);
Expand Down Expand Up @@ -819,6 +813,10 @@ impl AgentSession for ClaudeSession {
self.shared.running.load(Ordering::SeqCst)
}

fn take_raw_stream(&self) -> Option<mpsc::UnboundedReceiver<(String, String)>> {
self.raw_rx.lock().take()
}

async fn shutdown(&self) -> Result<()> {
self.shared.running.store(false, Ordering::SeqCst);
// Closing stdin asks the runtime to finish; `kill_on_drop` is the backstop.
Expand Down Expand Up @@ -868,6 +866,12 @@ async fn read_stream(
continue;
}

// Every protocol line, verbatim, behind `TERVIN_LOG=trace`. When a Thread
// behaves in a way the timeline cannot explain, this is the only account of
// what the runtime actually said, and needing a code change to obtain it is
// how a one-off oddity becomes unreproducible.
tracing::trace!(target: "tervin::claude::protocol", line = %trimmed);

let Ok(value) = serde_json::from_str::<Value>(trimmed) else {
// Not JSON. Some builds print banners or warnings on stdout; record
// it rather than treating the stream as broken.
Expand Down
13 changes: 13 additions & 0 deletions crates/agent-runtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,19 @@ pub trait AgentSession: Send + Sync {
/// True while the underlying process is alive.
fn is_running(&self) -> bool;

/// The raw payloads behind this session's events, if the runtime keeps them.
///
/// Handed out once, like the event stream, because there is one consumer. Events
/// already carry a pointer to what the runtime actually said; without someone
/// draining this, that pointer names a body nobody saved, and the question "what
/// did the runtime actually send?" has no answer at the moment it is asked.
///
/// A runtime with nothing to offer returns `None` and its events simply carry no
/// pointers.
fn take_raw_stream(&self) -> Option<mpsc::UnboundedReceiver<(String, String)>> {
None
}

/// End the session and reap the process.
async fn shutdown(&self) -> Result<()>;
}
Expand Down
55 changes: 52 additions & 3 deletions crates/tervin-app/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,24 @@ pub async fn block_get(

/// Full raw output, including anything that spilled to disk.
#[tauri::command]
pub async fn block_output(state: State<'_, Arc<AppState>>, block_id: String) -> Result<Vec<u8>> {
/// A Block's full output, as a reader would see it.
///
/// Escape sequences come off here rather than in the interface, with the same
/// routine the preview and the search index already use — three views of one
/// output that have to agree, and previously did not. Handing the bytes over
/// untouched put the terminal's own bookkeeping in front of the user: zsh's
/// partial-line marker and a window-title sequence rendered as `[1m[7m%[27m`,
/// which is not output they produced and not something they can act on.
pub async fn block_output(state: State<'_, Arc<AppState>>, block_id: String) -> Result<String> {
let store = state.store.clone();
let id = BlockId::from_external(block_id);
blocking(move || store.read_full_output(&id).map_err(CommandError::from)).await
blocking(move || {
let bytes = store.read_full_output(&id).map_err(CommandError::from)?;
Ok(block_engine::parse::strip_ansi(&String::from_utf8_lossy(
&bytes,
)))
})
.await
}

#[tauri::command]
Expand Down Expand Up @@ -1170,6 +1184,10 @@ pub async fn thread_start(
let capabilities = launched.session.capabilities();
let permissions = launched.session.permissions();

// The bodies behind each event's raw pointer. Taken before the session is
// handed to the registry, because it can only be taken once.
let mut raw_rx = launched.session.take_raw_stream();

// Drain the event stream into the store and on to the UI.
{
let store = state.store.clone();
Expand All @@ -1181,7 +1199,19 @@ pub async fn thread_start(
let thread_id = thread_id.clone();
let mut events = launched.events;
tokio::spawn(async move {
// Pointer to body, for events not yet seen. The runtime sends a payload
// before the event that references it, so this is normally empty or
// holds one entry; it is drained on the way past rather than by its own
// task, which would race the event it belongs to.
let mut bodies: std::collections::HashMap<String, String> =
std::collections::HashMap::new();

while let Some(event) = events.recv().await {
if let Some(rx) = raw_rx.as_mut() {
while let Ok((pointer, body)) = rx.try_recv() {
bodies.insert(pointer, body);
}
}
let _ = app.emit("thread://event", &event);

// A command an agent ran is the same kind of thing as one you ran, so it
Expand All @@ -1202,10 +1232,29 @@ pub async fn thread_start(
}),
);
}
// Save what the runtime actually said alongside the event. Without
// this the pointer on the event names nothing, and the first
// question asked of a surprising event — what did the runtime
// really send? — cannot be answered after the fact.
let body = event
.raw
.as_ref()
.and_then(|raw| bodies.remove(&raw.pointer));

// Anything still held belongs to an event that never arrived. Rare,
// but this map outlives every event in the session, so it is capped
// rather than left to grow.
if bodies.len() > 64 {
bodies.clear();
}

let store = store.clone();
let event = event.clone();
// Persisting is blocking; keep it off the event loop.
let _ = tokio::task::spawn_blocking(move || store.append_event(&event, None)).await;
let _ = tokio::task::spawn_blocking(move || {
store.append_event(&event, body.as_deref())
})
.await;
}
});
}
Expand Down
7 changes: 2 additions & 5 deletions ui/src/components/BlocksPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,8 @@ export function BlocksPanel({ failuresOnly = false }: Props) {
setExpanded(block.id);
if (!fullOutput[block.id]) {
try {
const bytes = await api.blockOutput(block.id);
setFullOutput((prev) => ({
...prev,
[block.id]: new TextDecoder().decode(bytes),
}));
const text = await api.blockOutput(block.id);
setFullOutput((prev) => ({ ...prev, [block.id]: text }));
} catch {
// The spill file may have been cleaned up; the preview still shows.
}
Expand Down
4 changes: 2 additions & 2 deletions ui/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -626,8 +626,8 @@ export const blocksQuery = (filter: BlockFilter) =>
export const blockGet = (blockId: string) =>
invoke<Block | null>("block_get", { blockId });

export const blockOutput = (blockId: string) =>
invoke<number[]>("block_output", { blockId }).then((b) => Uint8Array.from(b));
/** Full output, already stripped of escape sequences by the same routine as the preview. */
export const blockOutput = (blockId: string) => invoke<string>("block_output", { blockId });

export const blockSetBookmark = (blockId: string, bookmarked: boolean) =>
invoke<void>("block_set_bookmark", { blockId, bookmarked });
Expand Down