fix(ai): a pi turn finished on a tool call is a completion, not a failure
pi reports `stop_reason = "toolUse"` when a turn ends right after a tool result — which is exactly how the plan approval panel and a completed goal close out. The bridge mapped that to `Incomplete`, so every such turn raised the red "needs attention" banner and marked the pane as a failed command, even though nothing went wrong. `toolUse` now maps to `Succeeded`; `length`, `error` and `aborted` keep their meanings. Nothing changes on the pi side. Verified: unit tests for the outcome table and the notification path now cover the host-finished case (completion notification, non-failure); full `cargo test -p nebula --bin pebrel` — 1754 passed.
This commit is contained in:
1 parent
7b0cedb5f9
commit
d2aa6030b7
3 files changed
+23
-6
No files matched your search
@@ -2,7 +2,7 @@ use super::*;
|
||||
|
||||
#[test]
|
||||
fn non_successful_turns_do_not_emit_a_completion_notification() {
|
||||
for reason in ["error", "aborted", "length", "toolUse"] {
|
||||
for reason in ["error", "aborted", "length"] {
|
||||
let payload = serde_json::json!({"kind":"done", "stop_reason":reason});
|
||||
let event = parse_remote_envelope(
|
||||
format!("nebula-hook/1 source=pi\n{payload}").as_bytes(),
|
||||
@@ -25,6 +25,17 @@ fn non_successful_turns_do_not_emit_a_completion_notification() {
|
||||
)
|
||||
.unwrap();
|
||||
assert!(crate::notify::Notification::from_ai_hook(&event, None, false,).is_some());
|
||||
// Host-finished turns (plan menu, completed goal) end on a tool call; they
|
||||
// must notify like an ordinary completion, not as a failure.
|
||||
let event = parse_remote_envelope(
|
||||
b"nebula-hook/1 source=pi
|
||||
{\"kind\":\"done\",\"stop_reason\":\"toolUse\"}",
|
||||
Some(1),
|
||||
)
|
||||
.unwrap();
|
||||
let notification = crate::notify::Notification::from_ai_hook(&event, None, false)
|
||||
.expect("host-finished turns notify like ordinary completions");
|
||||
assert!(!notification.is_failure(), "a toolUse finish is not a failure");
|
||||
}
|
||||
#[test]
|
||||
fn pi_result_metadata_distinguishes_legacy_unknown_and_background_work() {
|
||||
@@ -36,7 +47,7 @@ fn pi_result_metadata_distinguishes_legacy_unknown_and_background_work() {
|
||||
(",\"stop_reason\":\"error\"", AiTurnOutcome::Failed, AgentStatus::Idle, true),
|
||||
(",\"stop_reason\":\"aborted\"", AiTurnOutcome::Cancelled, AgentStatus::Idle, false),
|
||||
(",\"stop_reason\":\"length\"", AiTurnOutcome::Incomplete, AgentStatus::Idle, true),
|
||||
(",\"stop_reason\":\"toolUse\"", AiTurnOutcome::Incomplete, AgentStatus::Idle, true),
|
||||
(",\"stop_reason\":\"toolUse\"", AiTurnOutcome::Succeeded, AgentStatus::Done, true),
|
||||
(",\"stop_reason\":\"future\"", AiTurnOutcome::Unknown, AgentStatus::Idle, true),
|
||||
(",\"stop_reason\":null", AiTurnOutcome::Unknown, AgentStatus::Idle, true),
|
||||
(",\"stop_reason\":42", AiTurnOutcome::Unknown, AgentStatus::Idle, true),
|
||||
|
||||
@@ -235,7 +235,11 @@ pub(super) fn parse_envelope(bytes: &[u8]) -> Option<AiHookEvent> {
|
||||
Some("stop") => AiTurnOutcome::Succeeded,
|
||||
Some("error") => AiTurnOutcome::Failed,
|
||||
Some("aborted") => AiTurnOutcome::Cancelled,
|
||||
Some("length" | "toolUse") => AiTurnOutcome::Incomplete,
|
||||
Some("length") => AiTurnOutcome::Incomplete,
|
||||
// pi ends a turn with `toolUse` when the host or an extension closes it
|
||||
// right after a tool result (the plan menu, a completed goal). That is a
|
||||
// finished turn, not a truncated one, so it must not raise a failure.
|
||||
Some("toolUse") => AiTurnOutcome::Succeeded,
|
||||
None if !payload.as_object()?.contains_key("stop_reason") => AiTurnOutcome::Unspecified,
|
||||
_ => AiTurnOutcome::Unknown,
|
||||
}
|
||||
|
||||
@@ -696,7 +696,6 @@ mod delivery_tests {
|
||||
("unknown", Some(AiTurnOutcome::Unknown)),
|
||||
("future", Some(AiTurnOutcome::Unknown)),
|
||||
("length", Some(AiTurnOutcome::Incomplete)),
|
||||
("toolUse", Some(AiTurnOutcome::Incomplete)),
|
||||
] {
|
||||
let event = pi_result(Some(reason), None);
|
||||
let notification = Notification::from_ai_hook(&event, None, false);
|
||||
@@ -708,7 +707,7 @@ mod delivery_tests {
|
||||
result => panic!("unexpected {reason} result: {result:?}"),
|
||||
}
|
||||
}
|
||||
for reason in [None, Some("stop")] {
|
||||
for reason in [None, Some("stop"), Some("toolUse")] {
|
||||
let mut payload = serde_json::json!({"kind": "done"});
|
||||
if let Some(reason) = reason {
|
||||
payload["stop_reason"] = reason.into();
|
||||
@@ -716,7 +715,10 @@ mod delivery_tests {
|
||||
let wire = format!("nebula-hook/1 source=pi\n{payload}");
|
||||
let event = crate::ai_hook::parse_remote_envelope(wire.as_bytes(), Some(1)).unwrap();
|
||||
let note = Notification::from_ai_hook(&event, None, false).unwrap();
|
||||
assert_eq!(matches!(note, Notification::AiTurn { .. }), reason == Some("stop"));
|
||||
assert_eq!(
|
||||
matches!(note, Notification::AiTurn { .. }),
|
||||
matches!(reason, Some("stop") | Some("toolUse"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user