Keep Claude commands out of subagents
This commit is contained in:
1 parent
8262ceb786
commit
1c60e78b55
2 files changed
+81
-49
No files matched your search
+18
-13
@@ -66,8 +66,12 @@ transcript is still being written to and its process is the session's to stop.
|
|||||||
|
|
||||||
## Lifecycle, as events in the subagent's transcript
|
## Lifecycle, as events in the subagent's transcript
|
||||||
|
|
||||||
1. Created on the first child line for an unseen parent id (or, when the
|
1. Created when the parent Task/Agent call is seen. A current Claude CLI's
|
||||||
parent Task call was seen, at that call). First lines written:
|
`task_started` with `task_type: local_agent` is a recovery source when an
|
||||||
|
adopted stream begins after that call. A bare `parent_tool_use_id` is not
|
||||||
|
enough: other operations can also parent nested lines, and treating one as
|
||||||
|
proof created false subagents named after their first subcommand.
|
||||||
|
First lines written:
|
||||||
`Status Running`, then `UserMessage { text: <the Task's prompt> }` when
|
`Status Running`, then `UserMessage { text: <the Task's prompt> }` when
|
||||||
the prompt is known -- it genuinely is the subagent's first user turn.
|
the prompt is known -- it genuinely is the subagent's first user turn.
|
||||||
2. Every child line is translated by that subagent's own `Translator`
|
2. Every child line is translated by that subagent's own `Translator`
|
||||||
@@ -75,12 +79,13 @@ transcript is still being written to and its process is the session's to stop.
|
|||||||
content-block index, and parallel subagents interleave).
|
content-block index, and parallel subagents interleave).
|
||||||
3. **What ends a subagent is the CLI's own task lifecycle**, on top-level
|
3. **What ends a subagent is the CLI's own task lifecycle**, on top-level
|
||||||
`system` lines that carry no `parent_tool_use_id`: `task_started`
|
`system` lines that carry no `parent_tool_use_id`: `task_started`
|
||||||
(`task_id`, `tool_use_id`, `is_backgrounded`, the prompt), `task_progress`
|
(`task_id`, `tool_use_id`, `task_type`, `is_backgrounded`, the prompt),
|
||||||
repeatedly, then `task_updated` (`patch.status`, naming the *task* only)
|
`task_progress` repeatedly, then `task_updated` (`patch.status`, naming the
|
||||||
and `task_notification` (`tool_use_id`, `status`, and `summary` -- the
|
*task* only) and `task_notification` (`tool_use_id`, `status`, and `summary`
|
||||||
agent's own report). `translate_task` keeps the `task_id -> tool_use_id`
|
-- the agent's own report). `translate_task` keeps the
|
||||||
mapping from the first so the update can be attributed, records the
|
`task_id -> tool_use_id` mapping from the first so the update can be
|
||||||
summary as the subagent's closing text, and writes `Status Exited`. A
|
attributed, records the summary as the subagent's closing text, and writes
|
||||||
|
`Status Exited`. A
|
||||||
`completed` update is deliberately not the end: its notification carries
|
`completed` update is deliberately not the end: its notification carries
|
||||||
the summary and would otherwise land after the ending. Any other terminal
|
the summary and would otherwise land after the ending. Any other terminal
|
||||||
status ends it from the update, since the failure to avoid is a subagent
|
status ends it from the update, since the failure to avoid is a subagent
|
||||||
@@ -175,11 +180,11 @@ its last status stays `Running`, which the list reports as **unknown**
|
|||||||
rather than as running (see the wire shape) until then.
|
rather than as running (see the wire shape) until then.
|
||||||
|
|
||||||
Title: for Claude, the Task call's `description` input, then
|
Title: for Claude, the Task call's `description` input, then
|
||||||
` (<subagent_type>)` when one is given; falling back to the tool's name when
|
` (<subagent_type>)` when one is given; falling back to `Task` when the
|
||||||
the child arrives before (or without) the parent call being seen. For Codex,
|
description is absent. An adopted current CLI can recover the same fields from
|
||||||
the first lifecycle record uses the spawned thread's name or the last segment
|
its `local_agent` lifecycle record. For Codex, the first lifecycle record uses
|
||||||
of `agentPath`, with underscores shown as spaces, then falls back to
|
the spawned thread's name or the last segment of `agentPath`, with underscores
|
||||||
`subagent`.
|
shown as spaces, then falls back to `subagent`.
|
||||||
|
|
||||||
## Server layout
|
## Server layout
|
||||||
|
|
||||||
|
|||||||
@@ -214,13 +214,15 @@ impl Translator {
|
|||||||
self.interrupting = true;
|
self.interrupting = true;
|
||||||
}
|
}
|
||||||
pub(super) fn translate(&mut self, message: &Value) -> Vec<Event> {
|
pub(super) fn translate(&mut self, message: &Value) -> Vec<Event> {
|
||||||
// Events from subagents (Task tool internals) carry a
|
// A parent id says that a line is nested, not what it is nested in.
|
||||||
// parent_tool_use_id; the transcript shows the Task tool's own
|
// Only a Task/Agent call or an agent lifecycle edge may create the
|
||||||
// start/end instead of every nested step. Routed into that
|
// subagent; the id alone is not proof of either.
|
||||||
// subagent's own transcript rather than dropped -- see
|
|
||||||
// `SUBAGENTS.md`.
|
|
||||||
if let Some(parent_id) = message.get("parent_tool_use_id").and_then(Value::as_str) {
|
if let Some(parent_id) = message.get("parent_tool_use_id").and_then(Value::as_str) {
|
||||||
return self.translate_child(parent_id, message);
|
return if self.subagents.get(parent_id).is_some() {
|
||||||
|
self.translate_child(parent_id, message)
|
||||||
|
} else {
|
||||||
|
Vec::new()
|
||||||
|
};
|
||||||
}
|
}
|
||||||
self.dispatch(message)
|
self.dispatch(message)
|
||||||
}
|
}
|
||||||
@@ -239,14 +241,7 @@ impl Translator {
|
|||||||
self.subagents.reopen(id);
|
self.subagents.reopen(id);
|
||||||
}
|
}
|
||||||
Some(_) => {}
|
Some(_) => {}
|
||||||
None => {
|
None => return Vec::new(),
|
||||||
// Nobody has heard of this id yet: the Task call itself
|
|
||||||
// either has not been seen or never will be. Started here
|
|
||||||
// with the best title available -- the tool name of this
|
|
||||||
// first line -- since SUBAGENTS.md's real title only
|
|
||||||
// arrives with the Task call.
|
|
||||||
self.subagents.start(id, &fallback_title(message), None);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
let child = self
|
let child = self
|
||||||
.children
|
.children
|
||||||
@@ -625,10 +620,16 @@ impl Translator {
|
|||||||
}
|
}
|
||||||
Some("task_started") => {
|
Some("task_started") => {
|
||||||
// What makes the session `Waiting` when its turn ends. The
|
// What makes the session `Waiting` when its turn ends. The
|
||||||
// subagent itself is created by the Task `tool_use` in the
|
// Task/Agent tool call ordinarily created the subagent first.
|
||||||
// parent's own message, which arrives first and carries the
|
// The lifecycle edge is the recovery path when an adopted
|
||||||
// title this side shows.
|
// current CLI's earlier tool call was not observed. Commands
|
||||||
|
// use `local_bash`, so they must never enter the registry.
|
||||||
if let Some(about) = about {
|
if let Some(about) = about {
|
||||||
|
if message.get("task_type").and_then(Value::as_str) == Some("local_agent")
|
||||||
|
&& self.subagents.get(&about).is_none()
|
||||||
|
{
|
||||||
|
self.start_subagent_from_task(&about, message);
|
||||||
|
}
|
||||||
self.awaiting_task_summaries.remove(&about);
|
self.awaiting_task_summaries.remove(&about);
|
||||||
self.open_tasks.insert(about);
|
self.open_tasks.insert(about);
|
||||||
}
|
}
|
||||||
@@ -1180,23 +1181,6 @@ fn prefixed_lines(prefix: char, text: &str) -> String {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The title to start a subagent under when its own first line arrives
|
|
||||||
/// before (or without) its Task call ever being seen: the tool name of that
|
|
||||||
/// first line, which is the only thing known about it yet. `"subagent"` for
|
|
||||||
/// a line this cannot even find a tool name in, such as one that opens with
|
|
||||||
/// something other than a tool call.
|
|
||||||
fn fallback_title(message: &Value) -> String {
|
|
||||||
message["message"]["content"]
|
|
||||||
.as_array()
|
|
||||||
.into_iter()
|
|
||||||
.flatten()
|
|
||||||
.find(|block| block.get("type").and_then(Value::as_str) == Some("tool_use"))
|
|
||||||
.and_then(|block| block.get("name"))
|
|
||||||
.and_then(Value::as_str)
|
|
||||||
.unwrap_or("subagent")
|
|
||||||
.to_string()
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Whether this line is a subagent's *own* turn ending -- the only thing
|
/// Whether this line is a subagent's *own* turn ending -- the only thing
|
||||||
/// that does, per `SUBAGENTS.md`: not the parent's `tool_result`, which for
|
/// that does, per `SUBAGENTS.md`: not the parent's `tool_result`, which for
|
||||||
/// a background Task arrives at launch rather than at completion.
|
/// a background Task arrives at launch rather than at completion.
|
||||||
@@ -1578,16 +1562,22 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn subagent_events_are_not_duplicated_into_the_transcript() {
|
fn a_background_commands_nested_line_does_not_create_a_subagent() {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
let mut translator = Translator::new(dir.path().to_path_buf(), test_subagents(&dir));
|
let subagents = test_subagents(&dir);
|
||||||
|
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||||
let events = translate_lines(
|
let events = translate_lines(
|
||||||
&mut translator,
|
&mut translator,
|
||||||
&[
|
&[
|
||||||
|
r#"{"type":"system","subtype":"task_started","task_id":"command1","tool_use_id":"toolu_parent","description":"cargo test","is_backgrounded":true,"task_type":"local_bash"}"#,
|
||||||
r#"{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"toolu_02","name":"Bash","input":{}}]},"parent_tool_use_id":"toolu_parent"}"#,
|
r#"{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"toolu_02","name":"Bash","input":{}}]},"parent_tool_use_id":"toolu_parent"}"#,
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
assert!(events.is_empty());
|
assert!(events.is_empty());
|
||||||
|
assert!(
|
||||||
|
subagents.get("toolu_parent").is_none(),
|
||||||
|
"a nested command is not a subagent"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A child line does not just vanish from the parent -- it lands in its
|
/// A child line does not just vanish from the parent -- it lands in its
|
||||||
@@ -1599,11 +1589,18 @@ mod tests {
|
|||||||
let subagents = test_subagents(&dir);
|
let subagents = test_subagents(&dir);
|
||||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||||
translate_lines(
|
translate_lines(
|
||||||
|
&mut translator,
|
||||||
|
&[
|
||||||
|
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_parent","name":"Agent","input":{"description":"helper"}}]},"parent_tool_use_id":null}"#,
|
||||||
|
],
|
||||||
|
);
|
||||||
|
let events = translate_lines(
|
||||||
&mut translator,
|
&mut translator,
|
||||||
&[
|
&[
|
||||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_c1","name":"Bash","input":{"command":"echo hi"}}]},"parent_tool_use_id":"toolu_parent"}"#,
|
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_c1","name":"Bash","input":{"command":"echo hi"}}]},"parent_tool_use_id":"toolu_parent"}"#,
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
|
assert!(events.is_empty());
|
||||||
let subagent = subagents.get("toolu_parent").expect("subagent started");
|
let subagent = subagents.get("toolu_parent").expect("subagent started");
|
||||||
let lines = crate::session::transcript::read_after(&subagent.transcript_path(), 0)
|
let lines = crate::session::transcript::read_after(&subagent.transcript_path(), 0)
|
||||||
.expect("read subagent transcript");
|
.expect("read subagent transcript");
|
||||||
@@ -1645,6 +1642,35 @@ mod tests {
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_agent_lifecycle_edge_recovers_an_unseen_subagent() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let subagents = test_subagents(&dir);
|
||||||
|
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||||
|
translate_lines(
|
||||||
|
&mut translator,
|
||||||
|
&[
|
||||||
|
r#"{"type":"system","subtype":"task_started","task_id":"agent1","tool_use_id":"toolu_agent","description":"Investigate","subagent_type":"general-purpose","prompt":"Find it","is_backgrounded":true,"task_type":"local_agent"}"#,
|
||||||
|
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_read","name":"Read","input":{}}]},"parent_tool_use_id":"toolu_agent"}"#,
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
let rows = subagents.list(true);
|
||||||
|
assert_eq!(rows.len(), 1);
|
||||||
|
assert_eq!(rows[0].title, "Investigate (general-purpose)");
|
||||||
|
let subagent = subagents.get("toolu_agent").expect("subagent recovered");
|
||||||
|
let lines = crate::session::transcript::read_after(&subagent.transcript_path(), 0)
|
||||||
|
.expect("read subagent transcript");
|
||||||
|
assert!(lines.iter().any(
|
||||||
|
|entry| matches!(&entry.event, Event::UserMessage { text, .. } if text == "Find it")
|
||||||
|
));
|
||||||
|
assert!(
|
||||||
|
lines.iter().any(
|
||||||
|
|entry| matches!(&entry.event, Event::ToolStart { tool, .. } if tool == "Read")
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// The parent's `tool_result` for the Task id is what ends the
|
/// The parent's `tool_result` for the Task id is what ends the
|
||||||
/// subagent -- SUBAGENTS.md's lifecycle #3 -- and nothing else does.
|
/// subagent -- SUBAGENTS.md's lifecycle #3 -- and nothing else does.
|
||||||
#[test]
|
#[test]
|
||||||
@@ -2288,6 +2314,7 @@ mod tests {
|
|||||||
translate_lines(
|
translate_lines(
|
||||||
&mut translator,
|
&mut translator,
|
||||||
&[
|
&[
|
||||||
|
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_task_a","name":"Agent","input":{"description":"helper a"}},{"type":"tool_use","id":"toolu_task_b","name":"Agent","input":{"description":"helper b"}}]},"parent_tool_use_id":null}"#,
|
||||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_a","name":"Bash","input":{}}]},"parent_tool_use_id":"toolu_task_a"}"#,
|
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_a","name":"Bash","input":{}}]},"parent_tool_use_id":"toolu_task_a"}"#,
|
||||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_b","name":"Read","input":{}}]},"parent_tool_use_id":"toolu_task_b"}"#,
|
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_b","name":"Read","input":{}}]},"parent_tool_use_id":"toolu_task_b"}"#,
|
||||||
],
|
],
|
||||||
|
|||||||
Reference in new issue
Block a user