Merge pull request #44 from patriceckhart/fix-swarm-tool-loop-turn-end
4 files · +89 -31
M packages/agent/modes/interactive.go +7 -6
| @@ -5103,12 +5103,13 @@ func (i *Interactive) TrackSwarmAgent(a *swarm.Agent, task string) { | |||
| 5103 | 5103 | } | |
| 5104 | 5104 | ||
| 5105 | 5105 | // trackSwarmAgent records a freshly-spawned auto-swarm agent and | |
| 5106 | - | // subscribes to its turn_end events. Sub-agents are long-lived | |
| 5107 | - | // daemons that keep running on the inbox after the initial task, | |
| 5108 | - | // so we can't wait on agent.Wait() — it never returns until the | |
| 5109 | - | // whole daemon dies. Instead we mark each entry done on its first | |
| 5110 | - | // turn_end (the initial task finishing), and when every tracked | |
| 5111 | - | // entry has reported in, flush a single summary into the main chat. | |
| 5106 | + | // subscribes to its prompt-level task completion events. Sub-agents | |
| 5107 | + | // are long-lived daemons that keep running on the inbox after the | |
| 5108 | + | // initial task, so we can't wait on agent.Wait() — it never returns | |
| 5109 | + | // until the whole daemon dies. Instead we mark each entry done when | |
| 5110 | + | // the swarm daemon reports the initial prompt has finished, and when | |
| 5111 | + | // every tracked entry has reported in, flush a single summary into | |
| 5112 | + | // the main chat. | |
| 5112 | 5113 | // | |
| 5113 | 5114 | // Wired in from cli.go via SwarmSpawnTool.OnSpawned only when auto- | |
| 5114 | 5115 | // swarm is enabled, so this is a no-op when the feature is off. | |
| @@ -5103,12 +5103,13 @@ func (i *Interactive) TrackSwarmAgent(a *swarm.Agent, task string) { | |||||
| 5103 | } | 5103 | } | ||
| 5104 | 5104 | ||||
| 5105 | // trackSwarmAgent records a freshly-spawned auto-swarm agent and | 5105 | // trackSwarmAgent records a freshly-spawned auto-swarm agent and | ||
| 5106 | - | // subscribes to its turn_end events. Sub-agents are long-lived | 5106 | + | // subscribes to its prompt-level task completion events. Sub-agents |
| 5107 | - | // daemons that keep running on the inbox after the initial task, | 5107 | + | // are long-lived daemons that keep running on the inbox after the |
| 5108 | - | // so we can't wait on agent.Wait() — it never returns until the | 5108 | + | // initial task, so we can't wait on agent.Wait() — it never returns |
| 5109 | - | // whole daemon dies. Instead we mark each entry done on its first | 5109 | + | // until the whole daemon dies. Instead we mark each entry done when |
| 5110 | - | // turn_end (the initial task finishing), and when every tracked | 5110 | + | // the swarm daemon reports the initial prompt has finished, and when |
| 5111 | - | // entry has reported in, flush a single summary into the main chat. | 5111 | + | // every tracked entry has reported in, flush a single summary into |
| 5112 | + | // the main chat. | |||
| 5112 | // | 5113 | // | ||
| 5113 | // Wired in from cli.go via SwarmSpawnTool.OnSpawned only when auto- | 5114 | // Wired in from cli.go via SwarmSpawnTool.OnSpawned only when auto- | ||
| 5114 | // swarm is enabled, so this is a no-op when the feature is off. | 5115 | // swarm is enabled, so this is a no-op when the feature is off. | ||
M packages/agent/swarm/agent.go +13 -10
| @@ -71,12 +71,15 @@ type Agent struct { | |||
| 71 | 71 | finished time.Time | |
| 72 | 72 | lastErr error | |
| 73 | 73 | ||
| 74 | - | // OnTurnEnd, if set, fires once per turn_end event the runner | |
| 75 | - | // observes from the child daemon. Used by auto-swarm watchers | |
| 76 | - | // to detect that a sub-agent's first (or n-th) task has | |
| 77 | - | // finished without waiting for the long-lived daemon itself to | |
| 78 | - | // exit — sub-agents keep running on the inbox even after the | |
| 79 | - | // initial task completes, so Wait() never unblocks for them. | |
| 74 | + | // OnTurnEnd, if set, fires once per prompt-level turn_end event | |
| 75 | + | // emitted by the swarm daemon wrapper. Provider/tool-loop | |
| 76 | + | // turn_end events (for example stop=tool_use) are ignored by the | |
| 77 | + | // runner because they do not mean the sub-agent task finished. | |
| 78 | + | // Used by auto-swarm watchers to detect that a sub-agent's first | |
| 79 | + | // (or n-th) task has finished without waiting for the long-lived | |
| 80 | + | // daemon itself to exit — sub-agents keep running on the inbox | |
| 81 | + | // even after the initial task completes, so Wait() never unblocks | |
| 82 | + | // for them. | |
| 80 | 83 | OnTurnEnd func(step int, errMsg string) | |
| 81 | 84 | ||
| 82 | 85 | ctx context.Context | |
| @@ -128,10 +131,10 @@ func (a *Agent) Err() error { | |||
| 128 | 131 | // and by /swarm wait <id>. | |
| 129 | 132 | func (a *Agent) Wait() { <-a.done } | |
| 130 | 133 | ||
| 131 | - | // SetOnTurnEnd installs (or clears, with nil) the per-turn callback | |
| 132 | - | // fired from the runner when the child daemon emits a turn_end | |
| 133 | - | // event. Safe to call from any goroutine: the runner reads the | |
| 134 | - | // callback under the same mutex. | |
| 134 | + | // SetOnTurnEnd installs (or clears, with nil) the callback fired | |
| 135 | + | // from the runner when the child daemon emits a prompt-level | |
| 136 | + | // turn_end event with a step field. Safe to call from any goroutine: | |
| 137 | + | // the runner reads the callback under the same mutex. | |
| 135 | 138 | func (a *Agent) SetOnTurnEnd(fn func(step int, errMsg string)) { | |
| 136 | 139 | a.mu.Lock() | |
| 137 | 140 | a.OnTurnEnd = fn | |
| @@ -71,12 +71,15 @@ type Agent struct { | |||||
| 71 | finished time.Time | 71 | finished time.Time | ||
| 72 | lastErr error | 72 | lastErr error | ||
| 73 | 73 | ||||
| 74 | - | // OnTurnEnd, if set, fires once per turn_end event the runner | 74 | + | // OnTurnEnd, if set, fires once per prompt-level turn_end event |
| 75 | - | // observes from the child daemon. Used by auto-swarm watchers | 75 | + | // emitted by the swarm daemon wrapper. Provider/tool-loop |
| 76 | - | // to detect that a sub-agent's first (or n-th) task has | 76 | + | // turn_end events (for example stop=tool_use) are ignored by the |
| 77 | - | // finished without waiting for the long-lived daemon itself to | 77 | + | // runner because they do not mean the sub-agent task finished. |
| 78 | - | // exit — sub-agents keep running on the inbox even after the | 78 | + | // Used by auto-swarm watchers to detect that a sub-agent's first |
| 79 | - | // initial task completes, so Wait() never unblocks for them. | 79 | + | // (or n-th) task has finished without waiting for the long-lived |
| 80 | + | // daemon itself to exit — sub-agents keep running on the inbox | |||
| 81 | + | // even after the initial task completes, so Wait() never unblocks | |||
| 82 | + | // for them. | |||
| 80 | OnTurnEnd func(step int, errMsg string) | 83 | OnTurnEnd func(step int, errMsg string) | ||
| 81 | 84 | ||||
| 82 | ctx context.Context | 85 | ctx context.Context | ||
| @@ -128,10 +131,10 @@ func (a *Agent) Err() error { | |||||
| 128 | // and by /swarm wait <id>. | 131 | // and by /swarm wait <id>. | ||
| 129 | func (a *Agent) Wait() { <-a.done } | 132 | func (a *Agent) Wait() { <-a.done } | ||
| 130 | 133 | ||||
| 131 | - | // SetOnTurnEnd installs (or clears, with nil) the per-turn callback | 134 | + | // SetOnTurnEnd installs (or clears, with nil) the callback fired |
| 132 | - | // fired from the runner when the child daemon emits a turn_end | 135 | + | // from the runner when the child daemon emits a prompt-level |
| 133 | - | // event. Safe to call from any goroutine: the runner reads the | 136 | + | // turn_end event with a step field. Safe to call from any goroutine: |
| 134 | - | // callback under the same mutex. | 137 | + | // the runner reads the callback under the same mutex. |
| 135 | func (a *Agent) SetOnTurnEnd(fn func(step int, errMsg string)) { | 138 | func (a *Agent) SetOnTurnEnd(fn func(step int, errMsg string)) { | ||
| 136 | a.mu.Lock() | 139 | a.mu.Lock() | ||
| 137 | a.OnTurnEnd = fn | 140 | a.OnTurnEnd = fn | ||
M packages/agent/swarm/runner.go +29 -15
| @@ -209,21 +209,13 @@ func (r *execRunner) Run(ctx context.Context, sink Sink) error { | |||
| 209 | 209 | if ev, ok := parseEventLine(trimmed); ok { | |
| 210 | 210 | _ = log.Append(ev) | |
| 211 | 211 | applyEventToSink(ev, sink) | |
| 212 | - | // Fan turn_end up to any subscriber on the | |
| 213 | - | // supervised Agent. Daemons stay alive across | |
| 214 | - | // many turns, so Wait()-style hooks would | |
| 215 | - | // never fire; per-turn callbacks let auto- | |
| 216 | - | // swarm summarise as each task completes. | |
| 217 | - | if ev.Type == "turn_end" && r.agent != nil { | |
| 218 | - | r.agent.mu.Lock() | |
| 219 | - | fn := r.agent.OnTurnEnd | |
| 220 | - | r.agent.mu.Unlock() | |
| 221 | - | if fn != nil { | |
| 222 | - | step, _ := ev.Data["step"].(float64) | |
| 223 | - | errMsg, _ := ev.Data["error"].(string) | |
| 224 | - | go fn(int(step), errMsg) | |
| 225 | - | } | |
| 226 | - | } | |
| 212 | + | // Fan prompt-level task completions up to any | |
| 213 | + | // subscriber on the supervised Agent. The child | |
| 214 | + | // also forwards provider/tool-loop turn_end | |
| 215 | + | // events (for example stop=tool_use); those do | |
| 216 | + | // not contain step and must not be treated as | |
| 217 | + | // swarm task completion. | |
| 218 | + | notifyPromptTurnEnd(r.agent, ev) | |
| 227 | 219 | } else { | |
| 228 | 220 | // Non-JSON output. Keep it as transcript so an | |
| 229 | 221 | // accidental fmt.Println in the child still | |
| @@ -301,6 +293,28 @@ func parseEventLine(line string) (Event, bool) { | |||
| 301 | 293 | return ev, true | |
| 302 | 294 | } | |
| 303 | 295 | ||
| 296 | + | // notifyPromptTurnEnd calls Agent.OnTurnEnd only for the swarm | |
| 297 | + | // daemon's prompt-level completion event. Provider/tool-loop | |
| 298 | + | // turn_end events (such as stop=tool_use) do not include step and | |
| 299 | + | // are not terminal for the delegated task. | |
| 300 | + | func notifyPromptTurnEnd(a *Agent, ev Event) { | |
| 301 | + | if a == nil || ev.Type != "turn_end" { | |
| 302 | + | return | |
| 303 | + | } | |
| 304 | + | step, ok := ev.Data["step"].(float64) | |
| 305 | + | if !ok { | |
| 306 | + | return | |
| 307 | + | } | |
| 308 | + | ||
| 309 | + | a.mu.Lock() | |
| 310 | + | fn := a.OnTurnEnd | |
| 311 | + | a.mu.Unlock() | |
| 312 | + | if fn != nil { | |
| 313 | + | errMsg, _ := ev.Data["error"].(string) | |
| 314 | + | go fn(int(step), errMsg) | |
| 315 | + | } | |
| 316 | + | } | |
| 317 | + | ||
| 304 | 318 | // applyEventToSink translates an Event into Sink updates. Only a | |
| 305 | 319 | // few event types are interpreted; the rest still land in the | |
| 306 | 320 | // durable log via the caller. | |
| @@ -209,21 +209,13 @@ func (r *execRunner) Run(ctx context.Context, sink Sink) error { | |||||
| 209 | if ev, ok := parseEventLine(trimmed); ok { | 209 | if ev, ok := parseEventLine(trimmed); ok { | ||
| 210 | _ = log.Append(ev) | 210 | _ = log.Append(ev) | ||
| 211 | applyEventToSink(ev, sink) | 211 | applyEventToSink(ev, sink) | ||
| 212 | - | // Fan turn_end up to any subscriber on the | 212 | + | // Fan prompt-level task completions up to any |
| 213 | - | // supervised Agent. Daemons stay alive across | 213 | + | // subscriber on the supervised Agent. The child |
| 214 | - | // many turns, so Wait()-style hooks would | 214 | + | // also forwards provider/tool-loop turn_end |
| 215 | - | // never fire; per-turn callbacks let auto- | 215 | + | // events (for example stop=tool_use); those do |
| 216 | - | // swarm summarise as each task completes. | 216 | + | // not contain step and must not be treated as |
| 217 | - | if ev.Type == "turn_end" && r.agent != nil { | 217 | + | // swarm task completion. |
| 218 | - | r.agent.mu.Lock() | 218 | + | notifyPromptTurnEnd(r.agent, ev) |
| 219 | - | fn := r.agent.OnTurnEnd | |||
| 220 | - | r.agent.mu.Unlock() | |||
| 221 | - | if fn != nil { | |||
| 222 | - | step, _ := ev.Data["step"].(float64) | |||
| 223 | - | errMsg, _ := ev.Data["error"].(string) | |||
| 224 | - | go fn(int(step), errMsg) | |||
| 225 | - | } | |||
| 226 | - | } | |||
| 227 | } else { | 219 | } else { | ||
| 228 | // Non-JSON output. Keep it as transcript so an | 220 | // Non-JSON output. Keep it as transcript so an | ||
| 229 | // accidental fmt.Println in the child still | 221 | // accidental fmt.Println in the child still | ||
| @@ -301,6 +293,28 @@ func parseEventLine(line string) (Event, bool) { | |||||
| 301 | return ev, true | 293 | return ev, true | ||
| 302 | } | 294 | } | ||
| 303 | 295 | ||||
| 296 | + | // notifyPromptTurnEnd calls Agent.OnTurnEnd only for the swarm | |||
| 297 | + | // daemon's prompt-level completion event. Provider/tool-loop | |||
| 298 | + | // turn_end events (such as stop=tool_use) do not include step and | |||
| 299 | + | // are not terminal for the delegated task. | |||
| 300 | + | func notifyPromptTurnEnd(a *Agent, ev Event) { | |||
| 301 | + | if a == nil || ev.Type != "turn_end" { | |||
| 302 | + | return | |||
| 303 | + | } | |||
| 304 | + | step, ok := ev.Data["step"].(float64) | |||
| 305 | + | if !ok { | |||
| 306 | + | return | |||
| 307 | + | } | |||
| 308 | + | ||||
| 309 | + | a.mu.Lock() | |||
| 310 | + | fn := a.OnTurnEnd | |||
| 311 | + | a.mu.Unlock() | |||
| 312 | + | if fn != nil { | |||
| 313 | + | errMsg, _ := ev.Data["error"].(string) | |||
| 314 | + | go fn(int(step), errMsg) | |||
| 315 | + | } | |||
| 316 | + | } | |||
| 317 | + | ||||
| 304 | // applyEventToSink translates an Event into Sink updates. Only a | 318 | // applyEventToSink translates an Event into Sink updates. Only a | ||
| 305 | // few event types are interpreted; the rest still land in the | 319 | // few event types are interpreted; the rest still land in the | ||
| 306 | // durable log via the caller. | 320 | // durable log via the caller. | ||
M packages/agent/swarm/runner_test.go +40 -0
| @@ -3,6 +3,7 @@ package swarm | |||
| 3 | 3 | import ( | |
| 4 | 4 | "strings" | |
| 5 | 5 | "testing" | |
| 6 | + | "time" | |
| 6 | 7 | ) | |
| 7 | 8 | ||
| 8 | 9 | // TestSwarmAgentArgs locks in the exact flag set the subprocess | |
| @@ -123,6 +124,45 @@ func TestDefaultChildArgsResumeOmitsTask(t *testing.T) { | |||
| 123 | 124 | } | |
| 124 | 125 | } | |
| 125 | 126 | ||
| 127 | + | func TestNotifyPromptTurnEndIgnoresProviderToolLoopTurnEnd(t *testing.T) { | |
| 128 | + | called := make(chan struct{}, 1) | |
| 129 | + | a := &Agent{} | |
| 130 | + | a.SetOnTurnEnd(func(step int, errMsg string) { | |
| 131 | + | called <- struct{}{} | |
| 132 | + | }) | |
| 133 | + | ||
| 134 | + | notifyPromptTurnEnd(a, NewEvent("turn_end", map[string]any{"stop": "tool_use"})) | |
| 135 | + | ||
| 136 | + | select { | |
| 137 | + | case <-called: | |
| 138 | + | t.Fatal("OnTurnEnd fired for provider/tool-loop turn_end without step") | |
| 139 | + | case <-time.After(50 * time.Millisecond): | |
| 140 | + | } | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | func TestNotifyPromptTurnEndFiresForDaemonPromptCompletion(t *testing.T) { | |
| 144 | + | type got struct { | |
| 145 | + | step int | |
| 146 | + | err string | |
| 147 | + | } | |
| 148 | + | called := make(chan got, 1) | |
| 149 | + | a := &Agent{} | |
| 150 | + | a.SetOnTurnEnd(func(step int, errMsg string) { | |
| 151 | + | called <- got{step: step, err: errMsg} | |
| 152 | + | }) | |
| 153 | + | ||
| 154 | + | notifyPromptTurnEnd(a, NewEvent("turn_end", map[string]any{"step": float64(2), "error": "boom"})) | |
| 155 | + | ||
| 156 | + | select { | |
| 157 | + | case g := <-called: | |
| 158 | + | if g.step != 2 || g.err != "boom" { | |
| 159 | + | t.Fatalf("callback = (%d, %q); want (2, boom)", g.step, g.err) | |
| 160 | + | } | |
| 161 | + | case <-time.After(time.Second): | |
| 162 | + | t.Fatal("OnTurnEnd did not fire for prompt-level turn_end with step") | |
| 163 | + | } | |
| 164 | + | } | |
| 165 | + | ||
| 126 | 166 | func indexOf(xs []string, want string) int { | |
| 127 | 167 | for i, x := range xs { | |
| 128 | 168 | if x == want { | |
| @@ -3,6 +3,7 @@ package swarm | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "strings" | 4 | "strings" | ||
| 5 | "testing" | 5 | "testing" | ||
| 6 | + | "time" | |||
| 6 | ) | 7 | ) | ||
| 7 | 8 | ||||
| 8 | // TestSwarmAgentArgs locks in the exact flag set the subprocess | 9 | // TestSwarmAgentArgs locks in the exact flag set the subprocess | ||
| @@ -123,6 +124,45 @@ func TestDefaultChildArgsResumeOmitsTask(t *testing.T) { | |||||
| 123 | } | 124 | } | ||
| 124 | } | 125 | } | ||
| 125 | 126 | ||||
| 127 | + | func TestNotifyPromptTurnEndIgnoresProviderToolLoopTurnEnd(t *testing.T) { | |||
| 128 | + | called := make(chan struct{}, 1) | |||
| 129 | + | a := &Agent{} | |||
| 130 | + | a.SetOnTurnEnd(func(step int, errMsg string) { | |||
| 131 | + | called <- struct{}{} | |||
| 132 | + | }) | |||
| 133 | + | ||||
| 134 | + | notifyPromptTurnEnd(a, NewEvent("turn_end", map[string]any{"stop": "tool_use"})) | |||
| 135 | + | ||||
| 136 | + | select { | |||
| 137 | + | case <-called: | |||
| 138 | + | t.Fatal("OnTurnEnd fired for provider/tool-loop turn_end without step") | |||
| 139 | + | case <-time.After(50 * time.Millisecond): | |||
| 140 | + | } | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | func TestNotifyPromptTurnEndFiresForDaemonPromptCompletion(t *testing.T) { | |||
| 144 | + | type got struct { | |||
| 145 | + | step int | |||
| 146 | + | err string | |||
| 147 | + | } | |||
| 148 | + | called := make(chan got, 1) | |||
| 149 | + | a := &Agent{} | |||
| 150 | + | a.SetOnTurnEnd(func(step int, errMsg string) { | |||
| 151 | + | called <- got{step: step, err: errMsg} | |||
| 152 | + | }) | |||
| 153 | + | ||||
| 154 | + | notifyPromptTurnEnd(a, NewEvent("turn_end", map[string]any{"step": float64(2), "error": "boom"})) | |||
| 155 | + | ||||
| 156 | + | select { | |||
| 157 | + | case g := <-called: | |||
| 158 | + | if g.step != 2 || g.err != "boom" { | |||
| 159 | + | t.Fatalf("callback = (%d, %q); want (2, boom)", g.step, g.err) | |||
| 160 | + | } | |||
| 161 | + | case <-time.After(time.Second): | |||
| 162 | + | t.Fatal("OnTurnEnd did not fire for prompt-level turn_end with step") | |||
| 163 | + | } | |||
| 164 | + | } | |||
| 165 | + | ||||
| 126 | func indexOf(xs []string, want string) int { | 166 | func indexOf(xs []string, want string) int { | ||
| 127 | for i, x := range xs { | 167 | for i, x := range xs { | ||
| 128 | if x == want { | 168 | if x == want { | ||