Skip to content

Commit 67d2116

Browse files
committed
feat(hooks): surface post_tool verification output to the model (verify-into-fix)
post_tool steering hooks ran but their output was discarded — `run_post_tool_hooks` only logged errors — and `run_hook_command` dropped stderr and returned `Ok(None)` on a non-zero exit. So a `cargo build` / `pytest` / lint hook configured to verify the agent's edits produced output the model never saw, leaving the documented verify-into-fix self-correction loop unbound. - `run_hook_command` gains `capture_failure`: for post_tool hooks it pipes stderr and returns the combined stdout+stderr **even on a non-zero exit** (prefixed with the exit status) — a failing build/test is exactly what must reach the model. pre_tool / user_prompt directive hooks pass `false` and keep the legacy contract (stderr dropped, non-zero exit -> `Ok(None)` -> proceed). Both pipes are now drained concurrently with `wait()` under one timeout, so a hook that fills a pipe buffer can't deadlock (previously stdout was read only after `wait()` returned). - `read_bounded` is generic over the pipe type, truncates (instead of erroring) on overflow so a verbose hook still yields a usable prefix, and decodes lossily so non-UTF-8 compiler/test output isn't dropped wholesale. - `run_post_tool_hooks` returns the combined hook output; the tool dispatch appends it to the tool result via `append_post_tool_output`, preserving Ok/Err polarity, so the model sees the verification output alongside the tool's own result. Tests: post_tool captures a failing hook's stdout+stderr with its exit code, captures success stdout, and returns None when a hook is silent; plus an Ok/Err-polarity test for the appender.
1 parent e99e8b3 commit 67d2116

2 files changed

Lines changed: 274 additions & 30 deletions

File tree

src/agent/mod.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,16 @@ fn extract_exec_command(args: &Value) -> Option<String> {
287287
.filter(|s| !s.is_empty())
288288
}
289289

290+
/// Append post_tool verification-hook output (build/test/lint results) to a tool result, preserving
291+
/// Ok/Err polarity so the model sees it alongside the tool's own output and can self-correct.
292+
fn append_post_tool_output(res: Result<String, String>, hook_out: &str) -> Result<String, String> {
293+
let note = format!("\n\n[post-tool hook]\n{hook_out}");
294+
match res {
295+
Ok(s) => Ok(format!("{s}{note}")),
296+
Err(s) => Err(format!("{s}{note}")),
297+
}
298+
}
299+
290300
fn should_require_shell_approval(command: &str, patterns: &[String]) -> bool {
291301
let lower = command.to_ascii_lowercase();
292302
patterns.iter().any(|p| lower.contains(p))
@@ -398,6 +408,22 @@ mod code_exec_gate_tests {
398408
// ...but a destructive one does.
399409
assert!(code_exec_requires_approval("exec", "rm -rf /tmp/x", &patterns));
400410
}
411+
412+
#[test]
413+
fn append_post_tool_output_preserves_polarity() {
414+
// Appends to an Ok result, staying Ok (verification output is informational).
415+
let ok = append_post_tool_output(Ok("applied".into()), "tests passed");
416+
assert_eq!(
417+
ok.as_deref(),
418+
Ok("applied\n\n[post-tool hook]\ntests passed")
419+
);
420+
// Appends to an Err result, staying Err (the tool itself failed).
421+
let err = append_post_tool_output(Err("boom".into()), "lint output");
422+
assert_eq!(
423+
err.as_ref().err().map(String::as_str),
424+
Some("boom\n\n[post-tool hook]\nlint output")
425+
);
426+
}
401427
}
402428

403429
fn hook_observe_telemetry(
@@ -697,6 +723,7 @@ async fn execute_tool_call_with_activity(
697723
}
698724
};
699725

726+
let mut post_tool_output: Option<String> = None;
700727
if let Some(ref hc) = runtime.hook_tool_ctx {
701728
if let Some(st) = &hc.steering {
702729
let res_for_hook = match &completed {
@@ -710,7 +737,7 @@ async fn execute_tool_call_with_activity(
710737
metadata: runtime.inbound_metadata.as_ref(),
711738
is_subagent: runtime.is_subagent,
712739
};
713-
run_post_tool_hooks(
740+
post_tool_output = run_post_tool_hooks(
714741
st.as_ref(),
715742
&tool_name,
716743
tool_call_id_for_hooks.as_deref(),
@@ -733,6 +760,12 @@ async fn execute_tool_call_with_activity(
733760
return ToolExecutionFinished::Waiting(ticket_id.to_string());
734761
}
735762
}
763+
// Append any post_tool verification-hook output so the model sees test/lint/build
764+
// results (including failures) and can self-correct. Ok/Err polarity is preserved.
765+
let res = match post_tool_output {
766+
Some(hook_out) => append_post_tool_output(res, &hook_out),
767+
None => res,
768+
};
736769
ToolExecutionFinished::Completed(res)
737770
}
738771
None => {

0 commit comments

Comments
 (0)