mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 01:09:46 -10:00
Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
279c31d9bc | ||
|
|
aff56441f8 | ||
|
|
e46e24e76e | ||
|
|
a7ab28e553 | ||
|
|
9e6e6f3353 | ||
|
|
ce3c90b5cc | ||
|
|
f055c30470 |
No files matched your search
@@ -280,6 +280,25 @@ func isContextLimitError(message string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// isStreamTimeoutError reports whether a streamed error is a provider stream
|
||||
// idle/stall timeout (providerio surfaces these as "idle timeout after …" /
|
||||
// "no output for …" / "stream stalled"). Such a turn produced nothing, so when
|
||||
// no content was forwarded yet it can be safely re-issued on a fresh connection
|
||||
// (see the stall-retry in the agent loop). Substring + case-insensitive to
|
||||
// tolerate the prefixed "provider stream error: …" wrapping.
|
||||
func isStreamTimeoutError(message string) bool {
|
||||
lowered := strings.ToLower(strings.TrimSpace(message))
|
||||
if lowered == "" {
|
||||
return false
|
||||
}
|
||||
for _, needle := range []string{"idle timeout after", "no output for", "stream stalled"} {
|
||||
if strings.Contains(lowered, needle) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// compactionState carries the per-run state the agent loop needs to compact a
|
||||
// conversation safely. It is created once per Run and is a no-op whenever
|
||||
// options.ContextWindow <= 0.
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
package agent
|
||||
|
||||
import "testing"
|
||||
|
||||
// decodeToolArguments must (a) decode a normal single-object payload unchanged,
|
||||
// (b) tolerate a weak model packing multiple concatenated top-level JSON objects
|
||||
// into one arguments string by decoding the FIRST object and ignoring the rest
|
||||
// (the minimax-m3 "invalid character '{' after top-level value" failure), and
|
||||
// (c) still reject a genuinely malformed/truncated first object.
|
||||
func TestDecodeToolArguments(t *testing.T) {
|
||||
t.Run("single object decodes normally", func(t *testing.T) {
|
||||
var a map[string]any
|
||||
if err := decodeToolArguments(`{"path":"x"}`, &a); err != nil {
|
||||
t.Fatalf("err = %v, want nil", err)
|
||||
}
|
||||
if a["path"] != "x" {
|
||||
t.Fatalf("decoded %v, want path=x", a)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("concatenated objects decode the first, ignore trailing", func(t *testing.T) {
|
||||
var b map[string]any
|
||||
if err := decodeToolArguments(`{"path":"first"}{"path":"second"}`, &b); err != nil {
|
||||
t.Fatalf("multi-object should decode the first, got err = %v", err)
|
||||
}
|
||||
if b["path"] != "first" {
|
||||
t.Fatalf("decoded %v, want the first object (path=first)", b)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("distinct tools' args concatenated (real minimax case): first object used", func(t *testing.T) {
|
||||
var c map[string]any
|
||||
if err := decodeToolArguments(`{"plan":[1,2]}{"cmd":"go version"}`, &c); err != nil {
|
||||
t.Fatalf("err = %v, want nil", err)
|
||||
}
|
||||
if _, ok := c["plan"]; !ok || c["cmd"] != nil {
|
||||
t.Fatalf("decoded %v, want only the first object's keys", c)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("truncated first object still errors", func(t *testing.T) {
|
||||
var d map[string]any
|
||||
if err := decodeToolArguments(`{"path":`, &d); err == nil {
|
||||
t.Fatal("truncated JSON must still error")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("non-JSON trailing garbage still errors (not masked)", func(t *testing.T) {
|
||||
var g map[string]any
|
||||
if err := decodeToolArguments(`{"x":1}xyz`, &g); err == nil {
|
||||
t.Fatal("a valid first object followed by non-JSON garbage must still error")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("recoverableToolArguments: only whole-JSON trailing is tolerated", func(t *testing.T) {
|
||||
cases := []struct {
|
||||
in string
|
||||
ok bool
|
||||
first string
|
||||
}{
|
||||
{`{"a":1}`, true, `{"a":1}`},
|
||||
{`{"a":1}{"b":2}`, true, `{"a":1}`},
|
||||
{`{"a":1} {"b":2} `, true, `{"a":1}`},
|
||||
{`{"a":1}xyz`, false, ""},
|
||||
{`{"a":`, false, ""},
|
||||
{` `, false, ""},
|
||||
}
|
||||
for _, c := range cases {
|
||||
first, ok := recoverableToolArguments(c.in)
|
||||
if ok != c.ok || (ok && first != c.first) {
|
||||
t.Fatalf("recoverableToolArguments(%q) = (%q,%v), want (%q,%v)", c.in, first, ok, c.first, c.ok)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("historySafeToolCalls recovers the first object, not {}", func(t *testing.T) {
|
||||
out := historySafeToolCalls([]ToolCall{
|
||||
{ID: "c1", Name: "read_file", Arguments: `{"path":"README.md"}{"path":"AGENTS.md"}`}, // concatenated -> first
|
||||
{ID: "c2", Name: "read_file", Arguments: `{"path":"x"}xyz`}, // garbage -> {}
|
||||
{ID: "c3", Name: "read_file", Arguments: `{"path":"ok"}`}, // valid -> unchanged
|
||||
})
|
||||
if out[0].Arguments != `{"path":"README.md"}` {
|
||||
t.Fatalf("concatenated args should record the first object, got %q", out[0].Arguments)
|
||||
}
|
||||
if out[1].Arguments != "{}" {
|
||||
t.Fatalf("garbage-trailing args should record {}, got %q", out[1].Arguments)
|
||||
}
|
||||
if out[2].Arguments != `{"path":"ok"}` {
|
||||
t.Fatalf("valid args should be unchanged, got %q", out[2].Arguments)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("whitespace-only errors (caller guards empty)", func(t *testing.T) {
|
||||
var e map[string]any
|
||||
if err := decodeToolArguments(" ", &e); err == nil {
|
||||
t.Fatal("whitespace-only must error")
|
||||
}
|
||||
})
|
||||
}
|
||||
+151
-36
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -20,6 +21,12 @@ import (
|
||||
const maxTurnsAnswer = "Agent reached maximum number of turns without a final answer."
|
||||
const maxTurnsFinalAnswerPrompt = "You have reached the tool-turn limit. Do not call tools. Give a concise final answer now: summarize what you completed, what you found, and any remaining blockers."
|
||||
|
||||
// maxStreamStallRetries bounds how many times a turn that timed out (idle/stall)
|
||||
// WITH NO OUTPUT yet is re-issued on a fresh connection before giving up. Only
|
||||
// the no-output case is retried (a partial turn would duplicate), so this is a
|
||||
// safe recovery for a stalled/dead pooled connection.
|
||||
const maxStreamStallRetries = 2
|
||||
|
||||
const (
|
||||
toolResultMetaControl = "control"
|
||||
toolResultControlSpecReview = "spec_review_required"
|
||||
@@ -203,55 +210,72 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
}
|
||||
}
|
||||
|
||||
collected := zeroruntime.CollectStreamWithOptions(ctx, stream, zeroruntime.CollectOptions{
|
||||
OnText: options.OnText,
|
||||
OnReasoning: options.OnReasoning,
|
||||
OnUsage: options.OnUsage,
|
||||
OnToolCallStart: options.OnToolCallStart,
|
||||
OnToolCallDelta: options.OnToolCallDelta,
|
||||
})
|
||||
if collected.Error != "" {
|
||||
collectedErr := errors.New(collected.Error)
|
||||
if isImageRejectionError(collectedErr) {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, fmt.Errorf("model %s rejected the image: %s. The model may not support image input — try switching to a vision-capable model (claude, gpt-4o, gemini)", options.Model, collected.Error)
|
||||
// Wrapped callbacks flag ANY output forwarded this turn, so a stall retry
|
||||
// never re-streams on top of output the user already saw — even across the
|
||||
// reactive-compaction retry below, which overwrites `collected`.
|
||||
forwardedAnything := false
|
||||
mark := func() { forwardedAnything = true }
|
||||
// OnUsage is telemetry, not visible output, so it does NOT mark
|
||||
// forwardedAnything: a provider that emits an early usage event (e.g. prompt
|
||||
// tokens) then stalls with no content must still be safely retried. The gate
|
||||
// only guards against re-streaming output the user actually SAW.
|
||||
forwardingOpts := zeroruntime.CollectOptions{OnUsage: options.OnUsage}
|
||||
if options.OnText != nil {
|
||||
forwardingOpts.OnText = func(s string) { mark(); options.OnText(s) }
|
||||
}
|
||||
if options.OnReasoning != nil {
|
||||
forwardingOpts.OnReasoning = func(s string) { mark(); options.OnReasoning(s) }
|
||||
}
|
||||
if options.OnToolCallStart != nil {
|
||||
forwardingOpts.OnToolCallStart = func(id, name string) { mark(); options.OnToolCallStart(id, name) }
|
||||
}
|
||||
if options.OnToolCallDelta != nil {
|
||||
forwardingOpts.OnToolCallDelta = func(id, fragment string) { mark(); options.OnToolCallDelta(id, fragment) }
|
||||
}
|
||||
// recoverStreamError applies the same non-stall recovery the initial stream
|
||||
// gets to ANY collected error — including one from a reissued (stall-retry)
|
||||
// stream: an image rejection gets the friendly wrapping, and a context limit
|
||||
// gets one compaction + reactive reissue (omitting visible callbacks, since
|
||||
// any pre-error output was already forwarded). It returns the possibly-updated
|
||||
// collected and a non-nil stop error when the run must end now.
|
||||
recoverStreamError := func(collected zeroruntime.CollectedStream) (zeroruntime.CollectedStream, error) {
|
||||
if isImageRejectionError(errors.New(collected.Error)) {
|
||||
return collected, fmt.Errorf("model %s rejected the image: %s. The model may not support image input — try switching to a vision-capable model (claude, gpt-4o, gemini)", options.Model, collected.Error)
|
||||
}
|
||||
// REACTIVE compaction: the streamed error may also be a context
|
||||
// limit (some providers surface it mid-stream). Compact and retry
|
||||
// the same turn once before giving up.
|
||||
// REACTIVE compaction: the streamed error may also be a context limit
|
||||
// (some providers surface it mid-stream). Compact and retry once.
|
||||
if compacted, retried, retryErr := compactor.recover(ctx, provider, messages, request.Tools, collected.Error); retried {
|
||||
messages = compacted
|
||||
if retryErr != nil {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, retryErr
|
||||
return collected, retryErr
|
||||
}
|
||||
// Reuse the SAME active-mode partition (exposed) from this
|
||||
// turn rather than the bare toolDefinitions: exposed depends on
|
||||
// registry+loaded (not the messages), so they stay valid after compaction.
|
||||
// Routing through an empty-loaded partition here would re-hide every
|
||||
// already-loaded deferred tool.
|
||||
// Reuse the SAME active-mode partition (exposed) from this turn rather
|
||||
// than the bare toolDefinitions: exposed depends on registry+loaded (not
|
||||
// the messages), so it stays valid after compaction.
|
||||
retryRequest := zeroruntime.CompletionRequest{
|
||||
Messages: copyMessages(messages),
|
||||
Tools: exposed,
|
||||
ReasoningEffort: options.ReasoningEffort,
|
||||
}
|
||||
// Pre-content reconnect of a mid-stream disconnect: route the connect
|
||||
// through the reconnect helper too (AUDIT-L1).
|
||||
retryStream, retryStreamErr := streamWithReconnect(ctx, provider, retryRequest, reconnectNoticeFor(options))
|
||||
if retryStreamErr != nil {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, retryStreamErr
|
||||
return collected, retryStreamErr
|
||||
}
|
||||
// Omit OnText on the reactive retry: when the original error
|
||||
// surfaced MID-stream, partial text was already forwarded to the
|
||||
// user. Re-streaming the retried response on top of it would
|
||||
// duplicate output. OnUsage IS kept so token telemetry/budgeting
|
||||
// still counts the successful retry. The retried text is captured
|
||||
// in collected.Text and becomes the turn's assistant message.
|
||||
collected = zeroruntime.CollectStreamWithOptions(ctx, retryStream, zeroruntime.CollectOptions{
|
||||
OnUsage: options.OnUsage,
|
||||
})
|
||||
}
|
||||
return collected, nil
|
||||
}
|
||||
|
||||
collected := zeroruntime.CollectStreamWithOptions(ctx, stream, forwardingOpts)
|
||||
if collected.Error != "" {
|
||||
updated, stop := recoverStreamError(collected)
|
||||
collected = updated
|
||||
if stop != nil {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, stop
|
||||
}
|
||||
}
|
||||
// Check ctx first: on cancellation helpers.go sets collected.Error to
|
||||
// ctx.Err().Error(), so returning errors.New(collected.Error) would lose
|
||||
@@ -260,9 +284,45 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, ctx.Err()
|
||||
}
|
||||
// A stream idle/stall timeout with NO output forwarded yet this turn can be
|
||||
// safely re-issued on a fresh connection — nothing was shown, so there's no
|
||||
// duplication (this recovers the macOS stale-pooled-connection hang when it
|
||||
// slips past the transport's response-header timeout). A turn that already
|
||||
// streamed partial text/tool-calls is NOT retried (it would duplicate) and
|
||||
// falls through to the error return below. Capped + exponential backoff.
|
||||
for attempt := 1; attempt <= maxStreamStallRetries &&
|
||||
isStreamTimeoutError(collected.Error) && !forwardedAnything &&
|
||||
collected.Text == "" && len(collected.ToolCalls) == 0; attempt++ {
|
||||
if err := sleepWithContext(ctx, backoffFor(attempt)); err != nil {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, err
|
||||
}
|
||||
retryRequest := zeroruntime.CompletionRequest{
|
||||
Messages: copyMessages(messages),
|
||||
Tools: exposed,
|
||||
ReasoningEffort: options.ReasoningEffort,
|
||||
}
|
||||
retryStream, retryErr := streamWithReconnect(ctx, provider, retryRequest, reconnectNoticeFor(options))
|
||||
if retryErr != nil {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, retryErr
|
||||
}
|
||||
collected = zeroruntime.CollectStreamWithOptions(ctx, retryStream, forwardingOpts)
|
||||
}
|
||||
if collected.Error != "" {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, errors.New(collected.Error)
|
||||
// Route a reissued stream's non-stall error through the SAME recovery as
|
||||
// the initial stream (image-rejection wrapping / context-limit compaction)
|
||||
// rather than returning it raw.
|
||||
updated, stop := recoverStreamError(collected)
|
||||
collected = updated
|
||||
if stop != nil {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, stop
|
||||
}
|
||||
if collected.Error != "" {
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, errors.New(collected.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// Carry the turn's terminal stop reason so a final answer cut off at the
|
||||
@@ -615,8 +675,20 @@ func historySafeToolCalls(calls []ToolCall) []ToolCall {
|
||||
for i, call := range calls {
|
||||
safe[i] = call
|
||||
args := strings.TrimSpace(call.Arguments)
|
||||
if args == "" || !json.Valid([]byte(args)) {
|
||||
switch {
|
||||
case args == "":
|
||||
safe[i].Arguments = "{}"
|
||||
case json.Valid([]byte(args)):
|
||||
// already a single valid JSON value — keep as-is
|
||||
default:
|
||||
// Recover the first object for the concatenated case so the replayed
|
||||
// transcript matches what executeToolCall actually ran; otherwise (real
|
||||
// corruption) fall back to an empty object.
|
||||
if first, ok := recoverableToolArguments(args); ok {
|
||||
safe[i].Arguments = first
|
||||
} else {
|
||||
safe[i].Arguments = "{}"
|
||||
}
|
||||
}
|
||||
}
|
||||
return safe
|
||||
@@ -641,10 +713,53 @@ func toolSchemaJSON(registry *tools.Registry, name string) string {
|
||||
// is a non-nil ABORT error only when the call demands the whole run stop (a
|
||||
// canceled/timed-out ask_user prompt) rather than continuing — every ordinary
|
||||
// success or tool error returns a nil abort error.
|
||||
// recoverableToolArguments inspects a tool call's raw JSON arguments. It returns
|
||||
// the raw text of the FIRST JSON value when the payload is one value optionally
|
||||
// followed by additional WHOLE JSON values (the weak-model concatenated case, e.g.
|
||||
// `{A}{B}`). ok is false when the payload is genuinely malformed: a bad/truncated
|
||||
// first value, OR trailing NON-JSON garbage after a valid first value (e.g.
|
||||
// `{"x":1}xyz`) — that case must still error, not be silently accepted. Sharing
|
||||
// this between dispatch and replay-history keeps them consistent: only a cleanly
|
||||
// recoverable first object is ever used.
|
||||
func recoverableToolArguments(arguments string) (first string, ok bool) {
|
||||
dec := json.NewDecoder(strings.NewReader(arguments))
|
||||
var head json.RawMessage
|
||||
if err := dec.Decode(&head); err != nil {
|
||||
return "", false
|
||||
}
|
||||
// The remainder must be only whole JSON values (or nothing); any decode error
|
||||
// other than EOF means trailing garbage — reject so corruption still surfaces.
|
||||
for {
|
||||
var rest json.RawMessage
|
||||
if err := dec.Decode(&rest); err != nil {
|
||||
if errors.Is(err, io.EOF) {
|
||||
return strings.TrimSpace(string(head)), true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// decodeToolArguments decodes a tool call's JSON arguments into v. It tolerates a
|
||||
// weaker model that packs MULTIPLE concatenated top-level JSON objects into one
|
||||
// arguments string (`{A}{B}`) — the cause of "invalid character '{' after top-level
|
||||
// value" failures with small models like minimax-m3 — by decoding the FIRST object
|
||||
// and ignoring the trailing WHOLE JSON values, so the primary intended call runs.
|
||||
// A genuinely malformed payload (bad/truncated first object, or non-JSON trailing
|
||||
// garbage) still returns the real parse error; a standard single-object payload
|
||||
// decodes exactly as before.
|
||||
func decodeToolArguments(arguments string, v any) error {
|
||||
if first, ok := recoverableToolArguments(arguments); ok {
|
||||
return json.Unmarshal([]byte(first), v)
|
||||
}
|
||||
// Not cleanly recoverable — surface the genuine parse error.
|
||||
return json.Unmarshal([]byte(arguments), v)
|
||||
}
|
||||
|
||||
func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCall, permissionMode PermissionMode, options Options) (ToolResult, error) {
|
||||
args := map[string]any{}
|
||||
if call.Arguments != "" {
|
||||
if err := json.Unmarshal([]byte(call.Arguments), &args); err != nil {
|
||||
if err := decodeToolArguments(call.Arguments, &args); err != nil {
|
||||
return ToolResult{
|
||||
ToolCallID: call.ID,
|
||||
Name: call.Name,
|
||||
|
||||
@@ -1053,7 +1053,10 @@ func TestRunSanitizesMalformedToolCallArgumentsBeforeRetry(t *testing.T) {
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "read_file"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":"README.md"}{"path":"AGENTS.md"}`},
|
||||
// Genuinely malformed (truncated) args: still error -> sanitize -> retry.
|
||||
// (Concatenated multi-object args are now tolerated; see
|
||||
// TestRunRecoversFirstObjectFromConcatenatedToolArgs.)
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":"README.md"`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
@@ -1103,6 +1106,53 @@ func TestRunSanitizesMalformedToolCallArgumentsBeforeRetry(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A weak model (e.g. minimax-m3) that packs MULTIPLE concatenated JSON objects into
|
||||
// one tool-call slot must RECOVER the first object and run the call, not fail with
|
||||
// "invalid character '{' after top-level value". The trailing object(s) are dropped
|
||||
// (the model re-issues them on a later turn if still needed).
|
||||
func TestRunRecoversFirstObjectFromConcatenatedToolArgs(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "README.md"), []byte("hello readme"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(tools.NewReadFileTool(root))
|
||||
provider := &mockProvider{
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "read_file"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":"README.md"}{"path":"AGENTS.md"}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "done"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
result, err := Run(context.Background(), "read", provider, Options{Registry: registry, MaxTurns: 2})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.FinalAnswer != "done" {
|
||||
t.Fatalf("final answer = %q, want done", result.FinalAnswer)
|
||||
}
|
||||
var toolResult string
|
||||
for _, message := range provider.requests[1].Messages {
|
||||
if message.Role == zeroruntime.MessageRoleTool {
|
||||
toolResult = message.Content
|
||||
}
|
||||
}
|
||||
if strings.Contains(toolResult, "Failed to parse arguments") {
|
||||
t.Fatalf("concatenated args should recover the first object, not fail to parse: %q", toolResult)
|
||||
}
|
||||
if !strings.Contains(toolResult, "hello readme") {
|
||||
t.Fatalf("expected README.md contents (the first object's path), got %q", toolResult)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunDefersSelfCorrectFeedbackUntilAfterToolBatch(t *testing.T) {
|
||||
// A single assistant turn with two tool calls where the first mutates and
|
||||
// self-correct fails must keep both tool_results contiguous, with the feedback
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// stallProvider connects successfully (HTTP 200) but the stream emits a stall/idle
|
||||
// timeout error for the first stallBefore calls, then succeeds with "done". When
|
||||
// partialText is set it emits that text BEFORE the stall, simulating a stall after
|
||||
// partial output (which must NOT be retried — re-issuing would duplicate it).
|
||||
type stallProvider struct {
|
||||
calls int32
|
||||
stallBefore int32
|
||||
partialText string
|
||||
reasoningText string
|
||||
}
|
||||
|
||||
func (p *stallProvider) StreamCompletion(_ context.Context, _ zeroruntime.CompletionRequest) (<-chan zeroruntime.StreamEvent, error) {
|
||||
n := atomic.AddInt32(&p.calls, 1)
|
||||
ch := make(chan zeroruntime.StreamEvent, 3)
|
||||
if n <= p.stallBefore {
|
||||
if p.partialText != "" {
|
||||
ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventText, Content: p.partialText}
|
||||
}
|
||||
if p.reasoningText != "" {
|
||||
ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventReasoning, Content: p.reasoningText}
|
||||
}
|
||||
ch <- zeroruntime.StreamEvent{
|
||||
Type: zeroruntime.StreamEventError,
|
||||
Error: "provider stream error: no output for 10m (the model produced nothing)",
|
||||
}
|
||||
close(ch)
|
||||
return ch, nil
|
||||
}
|
||||
ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventText, Content: "done"}
|
||||
ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventDone}
|
||||
close(ch)
|
||||
return ch, nil
|
||||
}
|
||||
|
||||
// A no-output stall is re-issued on a fresh connection and recovers — this is the
|
||||
// macOS stale-pooled-connection hang turned into an automatic recovery.
|
||||
func TestRunRetriesStreamStallWithNoOutput(t *testing.T) {
|
||||
p := &stallProvider{stallBefore: 1}
|
||||
result, err := Run(context.Background(), "go", p, Options{Registry: tools.NewRegistry()})
|
||||
if err != nil {
|
||||
t.Fatalf("a no-output stall should retry to success, got %v", err)
|
||||
}
|
||||
if result.FinalAnswer != "done" {
|
||||
t.Fatalf("final answer = %q, want %q", result.FinalAnswer, "done")
|
||||
}
|
||||
if got := atomic.LoadInt32(&p.calls); got != 2 {
|
||||
t.Fatalf("want 2 calls (1 stall + 1 retry), got %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A stall AFTER partial output must NOT be retried — re-issuing would duplicate
|
||||
// the already-streamed text. It surfaces the error instead.
|
||||
func TestRunDoesNotRetryStallAfterPartialOutput(t *testing.T) {
|
||||
p := &stallProvider{stallBefore: 1, partialText: "partial"}
|
||||
_, err := Run(context.Background(), "go", p, Options{Registry: tools.NewRegistry()})
|
||||
if err == nil {
|
||||
t.Fatal("a stall after partial output must NOT be retried; want an error")
|
||||
}
|
||||
if got := atomic.LoadInt32(&p.calls); got != 1 {
|
||||
t.Fatalf("partial-then-stall must not retry, got %d calls", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A stall after only REASONING was forwarded (no text/tool-calls) must NOT be
|
||||
// retried: the user already saw the reasoning, so re-issuing with callbacks
|
||||
// re-enabled would duplicate it. collected.Text is empty here, so this exercises
|
||||
// the turn-level forwarded-output gate specifically, not the text check.
|
||||
func TestRunDoesNotRetryStallAfterForwardedReasoning(t *testing.T) {
|
||||
p := &stallProvider{stallBefore: 1, reasoningText: "thinking hard"}
|
||||
_, err := Run(context.Background(), "go", p, Options{
|
||||
Registry: tools.NewRegistry(),
|
||||
OnReasoning: func(string) {},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("a stall after forwarded reasoning must NOT be retried; want an error")
|
||||
}
|
||||
if got := atomic.LoadInt32(&p.calls); got != 1 {
|
||||
t.Fatalf("reasoning-then-stall must not retry, got %d calls", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A persistent stall surfaces an error after exhausting the capped retries.
|
||||
func TestRunGivesUpAfterMaxStallRetries(t *testing.T) {
|
||||
p := &stallProvider{stallBefore: 99}
|
||||
_, err := Run(context.Background(), "go", p, Options{Registry: tools.NewRegistry()})
|
||||
if err == nil {
|
||||
t.Fatal("a persistent stall must surface an error after exhausting retries")
|
||||
}
|
||||
if got := atomic.LoadInt32(&p.calls); got != int32(1+maxStreamStallRetries) {
|
||||
t.Fatalf("want %d calls (1 + %d retries), got %d", 1+maxStreamStallRetries, maxStreamStallRetries, got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsStreamTimeoutError(t *testing.T) {
|
||||
timeouts := []string{
|
||||
"provider stream error: no output for 10m (the model produced nothing)",
|
||||
"provider stream error: idle timeout after 5m0s (upstream stopped sending data)",
|
||||
"stream stalled (upstream kept the connection alive but produced no output)",
|
||||
}
|
||||
for _, m := range timeouts {
|
||||
if !isStreamTimeoutError(m) {
|
||||
t.Fatalf("want timeout-classified: %q", m)
|
||||
}
|
||||
}
|
||||
notTimeouts := []string{"", "context length exceeded", "rate limit error: slow down", "model not found"}
|
||||
for _, m := range notTimeouts {
|
||||
if isStreamTimeoutError(m) {
|
||||
t.Fatalf("must NOT classify as timeout: %q", m)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,56 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"os"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// SetMaxTurnsEnv must export the per-run turn budget so a child process (whose
|
||||
// resolution reads the inherited environment via the nil-map -> os.Getenv path in
|
||||
// applyEnv) runs with the SAME budget the user set via /turns. n <= 0 is a no-op,
|
||||
// and a non-numeric/zero env value must not clobber the configured budget.
|
||||
func TestSetMaxTurnsEnvRoundTrips(t *testing.T) {
|
||||
t.Setenv(MaxTurnsEnv, "") // register cleanup; restores the original after
|
||||
|
||||
if err := os.Unsetenv(MaxTurnsEnv); err != nil {
|
||||
t.Fatalf("unset: %v", err)
|
||||
}
|
||||
SetMaxTurnsEnv(0) // non-positive must not set anything
|
||||
if got := os.Getenv(MaxTurnsEnv); got != "" {
|
||||
t.Fatalf("n<=0 set the env to %q, want unset", got)
|
||||
}
|
||||
|
||||
SetMaxTurnsEnv(150)
|
||||
if got := os.Getenv(MaxTurnsEnv); got != "150" {
|
||||
t.Fatalf("env = %q, want 150", got)
|
||||
}
|
||||
|
||||
// applyEnv with a nil map is the exact path a child uses: it falls back to
|
||||
// os.Getenv, so the inherited value sets the budget.
|
||||
cfg := &FileConfig{MaxTurns: defaultMaxTurns}
|
||||
applyEnv(cfg, nil)
|
||||
if cfg.MaxTurns != 150 {
|
||||
t.Fatalf("applyEnv MaxTurns = %d, want 150 (inherited env not read)", cfg.MaxTurns)
|
||||
}
|
||||
|
||||
// A garbage env value must leave the configured budget untouched.
|
||||
if err := os.Setenv(MaxTurnsEnv, "not-a-number"); err != nil {
|
||||
t.Fatalf("set: %v", err)
|
||||
}
|
||||
cfg2 := &FileConfig{MaxTurns: defaultMaxTurns}
|
||||
applyEnv(cfg2, nil)
|
||||
if cfg2.MaxTurns != defaultMaxTurns {
|
||||
t.Fatalf("garbage env clobbered MaxTurns to %d, want default %d", cfg2.MaxTurns, defaultMaxTurns)
|
||||
}
|
||||
|
||||
// An over-ceiling env value (e.g. a raw shell export that bypasses the /turns
|
||||
// clamp) must be clamped to MaxTurnsCeiling at the read site.
|
||||
if err := os.Setenv(MaxTurnsEnv, "999999"); err != nil {
|
||||
t.Fatalf("set: %v", err)
|
||||
}
|
||||
cfg3 := &FileConfig{MaxTurns: defaultMaxTurns}
|
||||
applyEnv(cfg3, nil)
|
||||
if cfg3.MaxTurns != MaxTurnsCeiling {
|
||||
t.Fatalf("applyEnv MaxTurns = %d, want clamped to ceiling %d", cfg3.MaxTurns, MaxTurnsCeiling)
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"net/url"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/modelregistry"
|
||||
@@ -13,7 +14,15 @@ import (
|
||||
"github.com/Gitlawb/zero/internal/sandbox"
|
||||
)
|
||||
|
||||
const defaultMaxTurns = 30
|
||||
// defaultMaxTurns is the per-run tool-turn budget when none is configured. 30 was
|
||||
// too low for real multi-step agentic work (agents ran out mid-task before reaching
|
||||
// later steps); 50 matches the old "deep" preset. Raise per-session with /turns.
|
||||
const defaultMaxTurns = 50
|
||||
|
||||
// MaxTurnsCeiling caps the per-run tool-turn budget so a stray env value or typo
|
||||
// can't set an absurd ceiling. Shared between applyEnv (read site) and the /turns
|
||||
// command (write site) so the bound holds even if the env is set by a raw shell.
|
||||
const MaxTurnsCeiling = 500
|
||||
|
||||
// defaultDeferThreshold is the number of deferred-eligible (MCP) tools at which
|
||||
// Zero collapses their full JSON schemas into compact `tool_search` reminder
|
||||
@@ -451,11 +460,33 @@ func SetActiveProviderEnv(name string) {
|
||||
_ = os.Setenv(ActiveProviderEnv, name)
|
||||
}
|
||||
|
||||
// MaxTurnsEnv overrides the per-run tool-turn budget by name (read in applyEnv).
|
||||
const MaxTurnsEnv = "ZERO_MAX_TURNS"
|
||||
|
||||
// SetMaxTurnsEnv exports the per-run tool-turn budget to the process environment so
|
||||
// a spawned child (sub-agent / swarm member, which inherits the environment) runs
|
||||
// with the SAME budget the user set via /turns. Without it a child re-resolves
|
||||
// config.json's default and a large delegated task can exhaust its turns mid-run
|
||||
// (exit 4 / max-turns). No-op for n <= 0.
|
||||
func SetMaxTurnsEnv(n int) {
|
||||
if n > 0 {
|
||||
_ = os.Setenv(MaxTurnsEnv, strconv.Itoa(n))
|
||||
}
|
||||
}
|
||||
|
||||
func applyEnv(cfg *FileConfig, env map[string]string) {
|
||||
activeProvider := strings.TrimSpace(envValue(env, ActiveProviderEnv))
|
||||
if activeProvider != "" {
|
||||
cfg.ActiveProvider = activeProvider
|
||||
}
|
||||
if maxTurns := strings.TrimSpace(envValue(env, MaxTurnsEnv)); maxTurns != "" {
|
||||
if n, err := strconv.Atoi(maxTurns); err == nil && n > 0 {
|
||||
if n > MaxTurnsCeiling {
|
||||
n = MaxTurnsCeiling
|
||||
}
|
||||
cfg.MaxTurns = n
|
||||
}
|
||||
}
|
||||
|
||||
applyProviderEnv(cfg, ProviderKindOpenAI, envProfile{
|
||||
Name: string(ProviderKindOpenAI),
|
||||
|
||||
@@ -883,8 +883,8 @@ func TestResolveAllowsNoConfiguredProviders(t *testing.T) {
|
||||
if HasProviderProfile(resolved.Provider) {
|
||||
t.Fatalf("Provider = %#v, want zero value", resolved.Provider)
|
||||
}
|
||||
if resolved.MaxTurns != 30 {
|
||||
t.Fatalf("MaxTurns = %d, want 30", resolved.MaxTurns)
|
||||
if resolved.MaxTurns != defaultMaxTurns {
|
||||
t.Fatalf("MaxTurns = %d, want default %d", resolved.MaxTurns, defaultMaxTurns)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -102,10 +102,11 @@ func New(options Options) (*Provider, error) {
|
||||
endpoint = baseURL + "/chat/completions"
|
||||
}
|
||||
|
||||
httpClient := options.HTTPClient
|
||||
if httpClient == nil {
|
||||
httpClient = http.DefaultClient
|
||||
}
|
||||
// Route through providerio.HTTPClient so a missing client gets the shared,
|
||||
// stall-hardened transport (bounded response-header wait + shorter idle-conn
|
||||
// reuse) rather than the raw http.DefaultClient, which hangs on a reused-dead
|
||||
// pooled connection (the macOS-only "both providers stall" bug).
|
||||
httpClient := providerio.HTTPClient(options.HTTPClient)
|
||||
|
||||
maxTokens := options.MaxTokens
|
||||
if maxTokens < 0 {
|
||||
|
||||
@@ -105,12 +105,42 @@ func NormalizeBaseURL(baseURL string, defaultBaseURL string, label string) (stri
|
||||
return baseURL, nil
|
||||
}
|
||||
|
||||
// HTTPClient returns the configured client or the process default.
|
||||
// sharedHTTPClient is the process-wide client used when a provider supplies none.
|
||||
// It tunes the default transport to defeat the stale-pooled-connection hang: Go
|
||||
// keeps idle keep-alive connections in a pool, and a later request can reuse one
|
||||
// the server/NAT has silently dropped. Because the model call is a POST (non-
|
||||
// idempotent), Go will NOT auto-retry it on a fresh connection — so it blocks
|
||||
// forever waiting for a response that never arrives. This is provider-agnostic
|
||||
// (every provider shares the default transport), which is why it reproduced on
|
||||
// BOTH chatgpt and ollama, and it surfaces on macOS far more than Linux because
|
||||
// macOS keeps dead pooled connections around longer.
|
||||
//
|
||||
// - ResponseHeaderTimeout bounds the gap between finishing the request and the
|
||||
// first response header, so a reused-dead connection fails fast (then the
|
||||
// reconnect path re-dials) instead of hanging. It does NOT bound streaming
|
||||
// after the 200 header arrives, so slow first tokens / long reasoning are
|
||||
// unaffected (the stream-idle + content watchdogs cover that).
|
||||
// - IdleConnTimeout is shortened so a connection that went idle across a pause
|
||||
// is closed (and re-dialed fresh) rather than reused after it has gone stale.
|
||||
var sharedHTTPClient = func() *http.Client {
|
||||
transport := http.DefaultTransport.(*http.Transport).Clone()
|
||||
// 120s, not 60s: a slow cloud proxy (e.g. ollama `*:cloud`) can withhold its
|
||||
// 200 response header until the upstream model emits a first token, so a 60s cap
|
||||
// risked aborting a legitimately-slow-but-alive request. 120s still bounds a
|
||||
// truly dead reused connection (which never responds) while tolerating slow
|
||||
// header delivery; slow first tokens after the header are covered by the idle +
|
||||
// content-stall watchdogs.
|
||||
transport.ResponseHeaderTimeout = 120 * time.Second
|
||||
transport.IdleConnTimeout = 30 * time.Second
|
||||
return &http.Client{Transport: transport}
|
||||
}()
|
||||
|
||||
// HTTPClient returns the configured client or the shared, stall-hardened default.
|
||||
func HTTPClient(client *http.Client) *http.Client {
|
||||
if client != nil {
|
||||
return client
|
||||
}
|
||||
return http.DefaultClient
|
||||
return sharedHTTPClient
|
||||
}
|
||||
|
||||
// SendEvent writes a provider event without blocking cancellation cleanup.
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -304,3 +305,30 @@ func TestStreamTimeoutMessage(t *testing.T) {
|
||||
t.Fatalf("stalled message must not claim the upstream stopped sending data: %q", stalled)
|
||||
}
|
||||
}
|
||||
|
||||
// HTTPClient(nil) must return the shared, stall-hardened transport (bounded
|
||||
// response-header wait + shorter idle-conn reuse) that defeats the macOS stale-
|
||||
// pooled-connection hang; an explicit client is returned untouched.
|
||||
func TestHTTPClientReturnsStallHardenedSharedClient(t *testing.T) {
|
||||
got := HTTPClient(nil)
|
||||
if got == nil {
|
||||
t.Fatal("HTTPClient(nil) returned nil")
|
||||
}
|
||||
tr, ok := got.Transport.(*http.Transport)
|
||||
if !ok {
|
||||
t.Fatalf("transport type = %T, want *http.Transport", got.Transport)
|
||||
}
|
||||
if tr.ResponseHeaderTimeout != 120*time.Second {
|
||||
t.Fatalf("ResponseHeaderTimeout = %v, want 120s", tr.ResponseHeaderTimeout)
|
||||
}
|
||||
if tr.IdleConnTimeout != 30*time.Second {
|
||||
t.Fatalf("IdleConnTimeout = %v, want 30s", tr.IdleConnTimeout)
|
||||
}
|
||||
if HTTPClient(nil) != got {
|
||||
t.Fatal("HTTPClient(nil) must return a shared instance so the conn pool is reused")
|
||||
}
|
||||
custom := &http.Client{}
|
||||
if HTTPClient(custom) != custom {
|
||||
t.Fatal("an explicit client must be returned unchanged")
|
||||
}
|
||||
}
|
||||
@@ -204,7 +204,11 @@ type ChildRunResult struct {
|
||||
Events []streamjson.Event
|
||||
Stderr string
|
||||
ExitCode int
|
||||
Started bool
|
||||
// Signal is a human-readable description (e.g. "signal: killed") when the child
|
||||
// was terminated by a signal rather than exiting normally; empty otherwise. It
|
||||
// turns an opaque exit -1 into an actionable reason (SIGKILL ~ out of memory).
|
||||
Signal string
|
||||
Started bool
|
||||
}
|
||||
|
||||
func (executor Executor) Run(ctx context.Context, params TaskParameters, options TaskRunOptions) (ExecResult, error) {
|
||||
@@ -566,17 +570,35 @@ func (executor Executor) runBuiltArgs(ctx context.Context, built BuildArgsResult
|
||||
exitCode := run.exitCodeOr(-1)
|
||||
summary := SummarizeStream(run.Events, exitCode)
|
||||
executor.recordSpecialistStop(accounting, summary, "error", summary.ExitCode, err, false)
|
||||
return ExecResult{}, err
|
||||
// Carry the child session id even on a post-start failure so a caller (the
|
||||
// swarm launcher -> FailWithSession) can still make the failed member
|
||||
// drillable; the session exists once the child has started.
|
||||
return ExecResult{SessionID: built.SessionID}, err
|
||||
}
|
||||
summary := SummarizeStream(run.Events, run.ExitCode)
|
||||
rolledUp := executor.rollUpSpecialistUsage(accounting, summary)
|
||||
executor.recordSpecialistStop(accounting, summary, summary.Status, summary.ExitCode, nil, rolledUp)
|
||||
return ExecResult{
|
||||
Result: BuildFinalResult(run.Events, run.Stderr, run.ExitCode),
|
||||
Result: BuildFinalResult(run.Events, run.Stderr, run.ExitCode, run.Signal),
|
||||
SessionID: built.SessionID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// availableSpecialistList renders the registered specialist names for a corrective
|
||||
// "not found" error, so a model that guessed a wrong name learns the real options.
|
||||
func availableSpecialistList(result LoadResult) string {
|
||||
names := make([]string, 0, len(result.Specialists))
|
||||
for _, manifest := range result.Specialists {
|
||||
if n := strings.TrimSpace(manifest.Metadata.Name); n != "" {
|
||||
names = append(names, n)
|
||||
}
|
||||
}
|
||||
if len(names) == 0 {
|
||||
return "(none registered)"
|
||||
}
|
||||
return strings.Join(names, ", ")
|
||||
}
|
||||
|
||||
func (executor Executor) loadManifest(name string) (Manifest, error) {
|
||||
name = strings.TrimSpace(name)
|
||||
if name == "" {
|
||||
@@ -592,7 +614,12 @@ func (executor Executor) loadManifest(name string) (Manifest, error) {
|
||||
}
|
||||
manifest, ok := Find(result, name)
|
||||
if !ok {
|
||||
return Manifest{}, fmt.Errorf("specialist %q not found", name)
|
||||
// Corrective error: a model that invents a specialist name (e.g.
|
||||
// "validator-runner", "file-writer") otherwise gets an opaque "not found"
|
||||
// and keeps retrying made-up names. List the ACTUALLY-registered ones so it
|
||||
// self-corrects — no hardcoded names, since a custom registry may not have
|
||||
// worker/explorer/code-review.
|
||||
return Manifest{}, fmt.Errorf("specialist %q not found. Available: %s. Pick one of these whose tools fit the task, or omit the name to use the default", name, availableSpecialistList(result))
|
||||
}
|
||||
return manifest, nil
|
||||
}
|
||||
@@ -783,15 +810,22 @@ func runChildProcess(ctx context.Context, binaryPath string, args []string, prog
|
||||
}
|
||||
exitCode := 0
|
||||
started := true
|
||||
signalDesc := ""
|
||||
if err := command.Wait(); err != nil {
|
||||
var exitErr *osexec.ExitError
|
||||
if errors.As(err, &exitErr) {
|
||||
exitCode = exitErr.ExitCode()
|
||||
if exitCode < 0 && exitErr.ProcessState != nil {
|
||||
// ExitCode() is -1 when the child was terminated by a signal rather
|
||||
// than exiting. ProcessState.String() is the portable description
|
||||
// (e.g. "signal: killed") — capture it so the failure isn't opaque.
|
||||
signalDesc = exitErr.ProcessState.String()
|
||||
}
|
||||
} else {
|
||||
return ChildRunResult{Events: events, Stderr: stderr.String(), ExitCode: -1, Started: started}, fmt.Errorf("run specialist child: %w", err)
|
||||
}
|
||||
}
|
||||
return ChildRunResult{Events: events, Stderr: stderr.String(), ExitCode: exitCode, Started: started}, nil
|
||||
return ChildRunResult{Events: events, Stderr: stderr.String(), ExitCode: exitCode, Signal: signalDesc, Started: started}, nil
|
||||
}
|
||||
|
||||
func (run ChildRunResult) exitCodeOr(defaultExitCode int) int {
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
package specialist
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// A model that invents a specialist name (e.g. "validator-runner", "file-writer")
|
||||
// must get a corrective error listing the real specialists, so it self-corrects to a
|
||||
// capable one instead of looping on made-up names that spawn doomed sub-agents.
|
||||
func TestLoadManifestUnknownNameListsAvailable(t *testing.T) {
|
||||
executor := Executor{
|
||||
Load: func(LoadOptions) (LoadResult, error) {
|
||||
return LoadResult{Specialists: []Manifest{
|
||||
{Metadata: Metadata{Name: "worker"}},
|
||||
{Metadata: Metadata{Name: "explorer"}},
|
||||
{Metadata: Metadata{Name: "code-review"}},
|
||||
}}, nil
|
||||
},
|
||||
}
|
||||
|
||||
_, err := executor.loadManifest("validator-runner")
|
||||
if err == nil {
|
||||
t.Fatal("an unknown specialist name must error")
|
||||
}
|
||||
msg := err.Error()
|
||||
for _, want := range []string{"validator-runner", "not found", "worker", "explorer", "code-review"} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Fatalf("error %q missing %q (must steer the model to a real specialist)", msg, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -260,6 +260,12 @@ func formatTaskOutputSummary(task background.Task, summary StreamResult, rawLine
|
||||
if !task.CompletedAt.IsZero() {
|
||||
fmt.Fprintf(&builder, "completed_at: %s\n", task.CompletedAt.Format(time.RFC3339))
|
||||
fmt.Fprintf(&builder, "exit_code: %d\n", task.ExitCode)
|
||||
if task.ExitCode < 0 {
|
||||
// A negative exit code means the background child was terminated by a
|
||||
// signal rather than exiting (the int-only launcher can't carry the
|
||||
// signal name). Give the same actionable hint the foreground path does.
|
||||
builder.WriteString("note: terminated by a signal — killed before it finished. Likely causes: the OS out-of-memory killer (common when many sub-agents run in parallel — try fewer), a timeout, or cancellation.\n")
|
||||
}
|
||||
}
|
||||
|
||||
if summary.Text != "" {
|
||||
|
||||
@@ -134,12 +134,17 @@ func SummarizeStream(events []streamjson.Event, processExitCode int) StreamResul
|
||||
return result
|
||||
}
|
||||
|
||||
func BuildFinalResult(events []streamjson.Event, stderrOutput string, processExitCode int) tools.Result {
|
||||
func BuildFinalResult(events []streamjson.Event, stderrOutput string, processExitCode int, signalDesc string) tools.Result {
|
||||
summary := SummarizeStream(events, processExitCode)
|
||||
hasErrors := len(summary.Errors) > 0 || summary.ExitCode != 0
|
||||
if summary.Status != "" && summary.Status != "success" && summary.Status != "ok" {
|
||||
hasErrors = true
|
||||
}
|
||||
// A captured kill signal must always surface, even if a late run_end reported a
|
||||
// clean exit just before the child was killed during teardown.
|
||||
if strings.TrimSpace(signalDesc) != "" {
|
||||
hasErrors = true
|
||||
}
|
||||
if !hasErrors {
|
||||
output := summary.Text
|
||||
if summary.SessionID != "" {
|
||||
@@ -149,7 +154,15 @@ func BuildFinalResult(events []streamjson.Event, stderrOutput string, processExi
|
||||
}
|
||||
|
||||
var builder strings.Builder
|
||||
fmt.Fprintf(&builder, "Subagent failed (exit %d)\n", summary.ExitCode)
|
||||
if signal := strings.TrimSpace(signalDesc); signal != "" {
|
||||
// The child was terminated by a signal (exit code -1) rather than exiting.
|
||||
// Surface the signal instead of an opaque "exit -1", and list the common
|
||||
// causes evenhandedly — this branch also covers timeouts and cancellations,
|
||||
// so don't assert OOM.
|
||||
fmt.Fprintf(&builder, "Subagent terminated by a signal (%s) — it was killed before it finished. Common causes: an out-of-memory kill, a timeout, or cancellation; check the signal to tell which. If you were running many sub-agents in parallel, reduce the concurrency.\n", signal)
|
||||
} else {
|
||||
fmt.Fprintf(&builder, "Subagent failed (exit %d)\n", summary.ExitCode)
|
||||
}
|
||||
if len(summary.Errors) > 0 {
|
||||
fmt.Fprintf(&builder, "errors: %s\n", strings.Join(summary.Errors, "; "))
|
||||
}
|
||||
|
||||
@@ -25,7 +25,7 @@ func TestParseStreamAndBuildFinalResultSuccess(t *testing.T) {
|
||||
if summary.SessionID != "child" || summary.Text != "done" || summary.ExitCode != 0 || len(summary.Tools) != 1 || summary.Tools[0] != "grep" {
|
||||
t.Fatalf("unexpected summary: %#v", summary)
|
||||
}
|
||||
result := BuildFinalResult(events, "", 0)
|
||||
result := BuildFinalResult(events, "", 0, "")
|
||||
if result.Status != tools.StatusOK || result.Output != "session_id: child\ndone" {
|
||||
t.Fatalf("unexpected final result: %#v", result)
|
||||
}
|
||||
@@ -41,7 +41,7 @@ func TestBuildFinalResultUsesTextDeltasWhenFinalMissing(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("ParseStream returned error: %v", err)
|
||||
}
|
||||
result := BuildFinalResult(events, "", 0)
|
||||
result := BuildFinalResult(events, "", 0, "")
|
||||
if result.Status != tools.StatusOK || result.Output != "session_id: child\nhello world" {
|
||||
t.Fatalf("unexpected final result: %#v", result)
|
||||
}
|
||||
@@ -82,7 +82,7 @@ func TestBuildFinalResultErrorIncludesDiagnostics(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("ParseStream returned error: %v", err)
|
||||
}
|
||||
result := BuildFinalResult(events, "stderr text", 0)
|
||||
result := BuildFinalResult(events, "stderr text", 0, "")
|
||||
if result.Status != tools.StatusError {
|
||||
t.Fatalf("Status = %s, want error", result.Status)
|
||||
}
|
||||
@@ -93,6 +93,33 @@ func TestBuildFinalResultErrorIncludesDiagnostics(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A child terminated by a signal (exit code -1) must surface the signal + an
|
||||
// actionable hint, not an opaque "Subagent failed (exit -1)".
|
||||
func TestBuildFinalResultSurfacesKillSignal(t *testing.T) {
|
||||
events, err := ParseStream(strings.NewReader(strings.Join([]string{
|
||||
`{"schemaVersion":2,"type":"run_start","runId":"run_1","sessionId":"child"}`,
|
||||
`{"schemaVersion":2,"type":"tool_call","runId":"run_1","id":"call_1","name":"read_file"}`,
|
||||
"",
|
||||
}, "\n")))
|
||||
if err != nil {
|
||||
t.Fatalf("ParseStream returned error: %v", err)
|
||||
}
|
||||
result := BuildFinalResult(events, "", -1, "signal: killed")
|
||||
if result.Status != tools.StatusError {
|
||||
t.Fatalf("Status = %s, want error", result.Status)
|
||||
}
|
||||
// Surfaces the signal + lists causes (OOM/timeout/cancellation) without asserting
|
||||
// OOM as the sole cause.
|
||||
for _, want := range []string{"terminated by a signal", "signal: killed", "out-of-memory", "timeout", "cancellation"} {
|
||||
if !strings.Contains(result.Output, want) {
|
||||
t.Fatalf("kill output missing %q:\n%s", want, result.Output)
|
||||
}
|
||||
}
|
||||
if strings.Contains(result.Output, "Subagent failed (exit") {
|
||||
t.Fatalf("signaled child should not show the opaque exit line:\n%s", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseStreamRejectsInvalidLines(t *testing.T) {
|
||||
_, err := ParseStream(strings.NewReader(`{"schemaVersion":2,"runId":"run_1"}` + "\n"))
|
||||
if err == nil || !strings.Contains(err.Error(), "type is required") {
|
||||
|
||||
@@ -166,6 +166,12 @@ func (c *Coordinator) Fail(id, errMsg string) error {
|
||||
return c.finish(id, StatusFailed, "", errMsg, "")
|
||||
}
|
||||
|
||||
// FailWithSession marks a task failed but keeps its child session id, so a member
|
||||
// that ran and failed (e.g. exited non-zero / hit max-turns) is still drillable.
|
||||
func (c *Coordinator) FailWithSession(id, errMsg, sessionID string) error {
|
||||
return c.finish(id, StatusFailed, "", errMsg, sessionID)
|
||||
}
|
||||
|
||||
func (c *Coordinator) finish(id string, status TaskStatus, result, errMsg, sessionID string) error {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
|
||||
@@ -2,8 +2,10 @@ package swarm
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/specialist"
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
)
|
||||
|
||||
// NewSpecialistLauncher adapts internal/specialist.Executor into a
|
||||
@@ -54,7 +56,18 @@ func NewSpecialistLauncher(executor specialist.Executor) MemberLauncher {
|
||||
MemberAutonomy: true,
|
||||
})
|
||||
if err != nil {
|
||||
return MemberResult{}, err
|
||||
// Preserve the child session id on a post-start failure too (exec.go
|
||||
// returns it on the error path) so lifecycle's FailWithSession can keep
|
||||
// the failed member drillable, not just the StatusError case below.
|
||||
return MemberResult{SessionID: res.SessionID}, err
|
||||
}
|
||||
if res.Result.Status == tools.StatusError {
|
||||
// The child ran but its task FAILED (e.g. non-zero exit / max-turns).
|
||||
// Surface it as a member failure so the swarm marks it [failed], not
|
||||
// [done] — otherwise the orchestrator (which can't see the AGENTS panel)
|
||||
// trusts incomplete work. Keep the session id so the failed member is
|
||||
// still drillable, and carry its report as the failure message.
|
||||
return MemberResult{SessionID: res.SessionID}, errors.New(res.Result.Output)
|
||||
}
|
||||
return MemberResult{Result: res.Result.Output, SessionID: res.SessionID}, nil
|
||||
}}
|
||||
|
||||
@@ -67,3 +67,50 @@ func TestSpecialistLauncherRunsUnregisteredSwarmAgent(t *testing.T) {
|
||||
t.Fatalf("member args missing agent type: %#v", gotArgs)
|
||||
}
|
||||
}
|
||||
|
||||
// A member whose child exits non-zero (e.g. exit 4 / max-turns) must be reported as
|
||||
// a FAILURE — otherwise the swarm marks it [done] and the orchestrator trusts
|
||||
// incomplete work. The failed member keeps its session id (drill-in) and the child
|
||||
// report rides along as the failure message.
|
||||
func TestSpecialistLauncherMarksNonZeroExitAsFailed(t *testing.T) {
|
||||
four := 4
|
||||
executor := specialist.Executor{
|
||||
BinaryPath: "/usr/local/bin/zero",
|
||||
NewSessionID: func() (string, error) { return "member_task", nil },
|
||||
Load: func(specialist.LoadOptions) (specialist.LoadResult, error) {
|
||||
return specialist.LoadResult{}, nil
|
||||
},
|
||||
RunChild: func(ctx context.Context, binaryPath string, args []string, progress func(streamjson.Event)) (specialist.ChildRunResult, error) {
|
||||
return specialist.ChildRunResult{
|
||||
Events: []streamjson.Event{
|
||||
{Type: streamjson.EventRunStart, SessionID: "member_task"},
|
||||
{Type: streamjson.EventFinal, Text: "i could not finish the objective"},
|
||||
{Type: streamjson.EventRunEnd, Status: "error", ExitCode: &four},
|
||||
},
|
||||
ExitCode: 4,
|
||||
}, nil
|
||||
},
|
||||
}
|
||||
|
||||
handle, err := NewSpecialistLauncher(executor).Launch(context.Background(), MemberSpec{
|
||||
ID: "m1",
|
||||
TaskID: "m1",
|
||||
AgentType: "subagent",
|
||||
Team: "probe",
|
||||
Task: "a task too big for the budget",
|
||||
SystemPrompt: "You are a subagent spawned to complete a specific task.",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("launch: %v", err)
|
||||
}
|
||||
res, err := handle.Wait()
|
||||
if err == nil {
|
||||
t.Fatal("a member that exited non-zero must be reported as FAILED, got nil error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "exit 4") {
|
||||
t.Fatalf("failure should carry the child report (exit 4), got %q", err.Error())
|
||||
}
|
||||
if res.SessionID != "member_task" {
|
||||
t.Fatalf("a failed member must keep its session id for drill-in, got %q", res.SessionID)
|
||||
}
|
||||
}
|
||||
@@ -71,7 +71,9 @@ func (s *Swarm) watch(t *Team, m *Member, spec MemberSpec) {
|
||||
}
|
||||
// fall through to record the original error if relaunch fails
|
||||
}
|
||||
_ = s.coord.Fail(m.TaskID, memberError(err))
|
||||
// res.SessionID is preserved by the handle even on error, so a member that
|
||||
// ran then failed stays drillable; a pure launch error carries an empty id.
|
||||
_ = s.coord.FailWithSession(m.TaskID, memberError(err), res.SessionID)
|
||||
} else {
|
||||
_ = s.coord.CompleteWithSession(m.TaskID, res.Result, res.SessionID)
|
||||
}
|
||||
|
||||
@@ -40,6 +40,7 @@ const (
|
||||
commandImage
|
||||
commandAddDir
|
||||
commandSelfCorrect
|
||||
commandTurns
|
||||
commandUnknown
|
||||
)
|
||||
|
||||
@@ -241,6 +242,13 @@ var commandDefinitions = []commandDefinition{
|
||||
description: "Show or set post-edit self-correction depth (LSP-only default; on/tests/full add the project test plan; off/lsp disable tests, LSP-only).",
|
||||
kind: commandSelfCorrect,
|
||||
},
|
||||
{
|
||||
name: "/turns",
|
||||
usage: "/turns [n]",
|
||||
group: commandGroupSession,
|
||||
description: "Show or set the per-run tool-turn budget for this session (raise it for long multi-step tasks).",
|
||||
kind: commandTurns,
|
||||
},
|
||||
{
|
||||
name: "/help",
|
||||
usage: "/help",
|
||||
|
||||
@@ -3465,6 +3465,18 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) {
|
||||
m, text = m.handleSelfCorrectCommand(command.text)
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text})
|
||||
return m, nil
|
||||
case commandTurns:
|
||||
// Changing the budget mid-run would mutate the inherited ZERO_MAX_TURNS env
|
||||
// that sub-agents spawned later in THIS run read, making the run's budget
|
||||
// inconsistent. Require an idle session (the new budget applies next run).
|
||||
if m.pending && strings.TrimSpace(command.text) != "" {
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: "Turns\nFinish or stop the current run before changing the tool-turn budget."})
|
||||
return m, nil
|
||||
}
|
||||
text := ""
|
||||
m, text = m.handleTurnsCommand(command.text)
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text})
|
||||
return m, nil
|
||||
case commandTheme:
|
||||
text := ""
|
||||
m, text = m.handleThemeCommand(command.text)
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
tea "charm.land/bubbletea/v2"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/agent"
|
||||
"github.com/Gitlawb/zero/internal/config"
|
||||
"github.com/Gitlawb/zero/internal/modelregistry"
|
||||
"github.com/Gitlawb/zero/internal/sessions"
|
||||
"github.com/Gitlawb/zero/internal/usage"
|
||||
@@ -256,6 +257,42 @@ func (m model) handleSelfCorrectCommand(args string) (model, string) {
|
||||
return m, m.selfCorrectText()
|
||||
}
|
||||
|
||||
// maxTurnsCeiling caps the per-session /turns budget so a typo (e.g. /turns 99999)
|
||||
// can't set an absurd ceiling; real multi-step tasks fit comfortably under it. Shared
|
||||
// with config so applyEnv enforces the same bound on an inherited ZERO_MAX_TURNS.
|
||||
const maxTurnsCeiling = config.MaxTurnsCeiling
|
||||
|
||||
func (m model) handleTurnsCommand(args string) (model, string) {
|
||||
args = strings.TrimSpace(args)
|
||||
if args == "" || strings.EqualFold(args, "status") {
|
||||
return m, m.turnsText()
|
||||
}
|
||||
n, err := strconv.Atoi(args)
|
||||
if err != nil || n < 1 {
|
||||
return m, "Turns\nUsage: /turns <n> — set the per-run tool-turn budget for this session (a positive number, e.g. /turns 150)."
|
||||
}
|
||||
if n > maxTurnsCeiling {
|
||||
n = maxTurnsCeiling
|
||||
}
|
||||
m.agentOptions.MaxTurns = n
|
||||
// Propagate the budget to spawned sub-agents / swarm members (which inherit the
|
||||
// environment) so a delegated task gets the same budget, not config.json's default.
|
||||
config.SetMaxTurnsEnv(n)
|
||||
return m, m.turnsText()
|
||||
}
|
||||
|
||||
func (m model) turnsText() string {
|
||||
return renderCommandOutput(commandOutput{
|
||||
Title: "Turns",
|
||||
Status: commandStatusOK,
|
||||
Sections: []commandSection{{
|
||||
Title: "State",
|
||||
Lines: []string{fmt.Sprintf("max tool-turns per run: %d", m.agentOptions.MaxTurns)},
|
||||
}},
|
||||
Hints: []string{fmt.Sprintf("/turns <n> sets this session's tool-turn budget (max %d); raise it for long multi-step tasks like delegation/swarm runs", maxTurnsCeiling)},
|
||||
})
|
||||
}
|
||||
|
||||
func (m model) selfCorrectText() string {
|
||||
depth := "LSP diagnostics only — fast, scoped to changed files"
|
||||
if m.selfCorrectTests {
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestHandleTurnsCommand(t *testing.T) {
|
||||
// Isolate the inherited-by-children budget env this command exports.
|
||||
t.Setenv("ZERO_MAX_TURNS", "")
|
||||
|
||||
t.Run("sets a valid budget and exports it for children", func(t *testing.T) {
|
||||
got, out := model{}.handleTurnsCommand("150")
|
||||
if got.agentOptions.MaxTurns != 150 {
|
||||
t.Fatalf("MaxTurns = %d, want 150", got.agentOptions.MaxTurns)
|
||||
}
|
||||
if !strings.Contains(out, "150") {
|
||||
t.Fatalf("output should report 150, got %q", out)
|
||||
}
|
||||
if env := os.Getenv("ZERO_MAX_TURNS"); env != "150" {
|
||||
t.Fatalf("ZERO_MAX_TURNS = %q, want 150 (so sub-agents inherit it)", env)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("clamps above the ceiling (env too)", func(t *testing.T) {
|
||||
got, _ := model{}.handleTurnsCommand("99999")
|
||||
if got.agentOptions.MaxTurns != maxTurnsCeiling {
|
||||
t.Fatalf("MaxTurns = %d, want clamp to %d", got.agentOptions.MaxTurns, maxTurnsCeiling)
|
||||
}
|
||||
if env := os.Getenv("ZERO_MAX_TURNS"); env != strconv.Itoa(maxTurnsCeiling) {
|
||||
t.Fatalf("ZERO_MAX_TURNS = %q, want clamped %d", env, maxTurnsCeiling)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("invalid / non-positive input is rejected without changing the budget", func(t *testing.T) {
|
||||
for _, in := range []string{"abc", "0", "-5", "3.5"} {
|
||||
m := model{}
|
||||
m.agentOptions.MaxTurns = 42
|
||||
got, out := m.handleTurnsCommand(in)
|
||||
if got.agentOptions.MaxTurns != 42 {
|
||||
t.Fatalf("input %q changed MaxTurns to %d, want unchanged (42)", in, got.agentOptions.MaxTurns)
|
||||
}
|
||||
if !strings.Contains(out, "Usage") {
|
||||
t.Fatalf("input %q should show usage, got %q", in, out)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("empty / status shows the current budget without changing it", func(t *testing.T) {
|
||||
m := model{}
|
||||
m.agentOptions.MaxTurns = 42
|
||||
got, out := m.handleTurnsCommand("")
|
||||
if got.agentOptions.MaxTurns != 42 {
|
||||
t.Fatalf("status changed MaxTurns to %d, want 42", got.agentOptions.MaxTurns)
|
||||
}
|
||||
if !strings.Contains(out, "42") {
|
||||
t.Fatalf("status should report current 42, got %q", out)
|
||||
}
|
||||
})
|
||||
}
|
||||
Reference in new issue
Block a user