0958647 fix(subagents): address follow-up review findings · zut

M packages/agent/modes/interactive.go +7 -0


@@ -603,6 +603,10 @@ type Interactive struct {
603	603		subagentWatchMu sync.Mutex
604	604		subagentWatch []*subagentWatchEntry
605	605
	606	+	// subagentsWaitWatcherDone is an optional test hook invoked when a
	607	+	// /subagents wait watcher exits.
	608	+	subagentsWaitWatcherDone func()
	609	+
606	610		// pendingFork is true when the user ran /session fork: the next
607	611		// jump-picker selection should branch off that message instead
608	612		// of scrolling. Flag resets after the action fires or the dialog

@@ -7400,6 +7404,9 @@ func removeLastAutoSubagentsAddendum(system, addendum string) (string, bool) {
7400	7404		// The profile manifest and delegation guidance are managed together so
7401	7405		// toggling auto-subagents never leaves the model with names but no tool.
7402	7406		func (i *Interactive) applyAutoSubagentsSystemPrompt(active bool) {
	7407	+	// i.mu is sufficient here: this path updates the existing agent's system
	7408	+	// prompt and managed addenda in place; it does not replace the agent or
	7409	+	// its tool registry, so agentMu is not required.
7403	7410		i.mu.Lock()
7404	7411		defer i.mu.Unlock()
7405	7412		if i.agent == nil {


@@ -603,6 +603,10 @@ type Interactive struct {
603	subagentWatchMu sync.Mutex	603		subagentWatchMu sync.Mutex
604	subagentWatch []*subagentWatchEntry	604		subagentWatch []*subagentWatchEntry
605		605
		606	+	// subagentsWaitWatcherDone is an optional test hook invoked when a
		607	+	// /subagents wait watcher exits.
		608	+	subagentsWaitWatcherDone func()
		609	+
606	// pendingFork is true when the user ran /session fork: the next	610		// pendingFork is true when the user ran /session fork: the next
607	// jump-picker selection should branch off that message instead	611		// jump-picker selection should branch off that message instead
608	// of scrolling. Flag resets after the action fires or the dialog	612		// of scrolling. Flag resets after the action fires or the dialog

@@ -7400,6 +7404,9 @@ func removeLastAutoSubagentsAddendum(system, addendum string) (string, bool) {
7400	// The profile manifest and delegation guidance are managed together so	7404		// The profile manifest and delegation guidance are managed together so
7401	// toggling auto-subagents never leaves the model with names but no tool.	7405		// toggling auto-subagents never leaves the model with names but no tool.
7402	func (i *Interactive) applyAutoSubagentsSystemPrompt(active bool) {	7406		func (i *Interactive) applyAutoSubagentsSystemPrompt(active bool) {
		7407	+	// i.mu is sufficient here: this path updates the existing agent's system
		7408	+	// prompt and managed addenda in place; it does not replace the agent or
		7409	+	// its tool registry, so agentMu is not required.
7403	i.mu.Lock()	7410		i.mu.Lock()
7404	defer i.mu.Unlock()	7411		defer i.mu.Unlock()
7405	if i.agent == nil {	7412		if i.agent == nil {

M packages/agent/modes/subagents_slash.go +6 -0


@@ -243,7 +243,13 @@ func (i *Interactive) runSubagents(ctx context.Context, args []string) {
243	243		i.subagentsStatus("", "wait: no such agent "+rest)
244	244		return
245	245		}
	246	+	waitWatcherDone := i.subagentsWaitWatcherDone
246	247		go func() {
	248	+	defer func() {
	249	+	if waitWatcherDone != nil {
	250	+	waitWatcherDone()
	251	+	}
	252	+	}()
247	253		if err := a.WaitContext(ctx); err != nil {
248	254		return
249	255		}


@@ -243,7 +243,13 @@ func (i *Interactive) runSubagents(ctx context.Context, args []string) {
243	i.subagentsStatus("", "wait: no such agent "+rest)	243		i.subagentsStatus("", "wait: no such agent "+rest)
244	return	244		return
245	}	245		}
		246	+	waitWatcherDone := i.subagentsWaitWatcherDone
246	go func() {	247		go func() {
		248	+	defer func() {
		249	+	if waitWatcherDone != nil {
		250	+	waitWatcherDone()
		251	+	}
		252	+	}()
247	if err := a.WaitContext(ctx); err != nil {	253		if err := a.WaitContext(ctx); err != nil {
248	return	254		return
249	}	255		}

M packages/agent/modes/subagents_slash_test.go +14 -3


@@ -81,8 +81,11 @@ func TestRunSupervisorSubcommandsDoNotPanic(t *testing.T) {
81	81		iv.runSubagents(context.Background(), args)
82	82		}()
83	83		}
84		-	if !strings.Contains(iv.statusErr, "cancel") \|\| !strings.Contains(iv.statusErr, "wait") {
85		-	t.Fatalf("unknown-command hint = %q; want cancel and wait", iv.statusErr)
	84	+	iv.mu.Lock()
	85	+	statusErr := iv.statusErr
	86	+	iv.mu.Unlock()
	87	+	if !strings.Contains(statusErr, "cancel") \|\| !strings.Contains(statusErr, "wait") {
	88	+	t.Fatalf("unknown-command hint = %q; want cancel and wait", statusErr)
86	89		}
87	90		}
88	91

@@ -242,6 +245,8 @@ func TestRunSupervisorWaitHonorsRunContext(t *testing.T) {
242	245		t.Fatal(err)
243	246		}
244	247		ctx, cancel := context.WithCancel(context.Background())
	248	+	watcherDone := make(chan struct{})
	249	+	iv.subagentsWaitWatcherDone = func() { close(watcherDone) }
245	250		iv.runSubagents(ctx, []string{"wait", a.ID})
246	251		iv.mu.Lock()
247	252		status := iv.statusOK

@@ -253,7 +258,13 @@ func TestRunSupervisorWaitHonorsRunContext(t *testing.T) {
253	258
254	259		// The wait watcher must stop observing the run context and must not
255	260		// later overwrite the status with a false completion after cleanup.
256		-	time.Sleep(20 * time.Millisecond)
	261	+	deadline := time.NewTimer(time.Second)
	262	+	defer deadline.Stop()
	263	+	select {
	264	+	case <-watcherDone:
	265	+	case <-deadline.C:
	266	+	t.Fatal("timed out waiting for wait watcher to exit")
	267	+	}
257	268		iv.mu.Lock()
258	269		status = iv.statusOK
259	270		iv.mu.Unlock()

 			iv.runSubagents(context.Background(), args)
 		}()
 	}
-	if !strings.Contains(iv.statusErr, "cancel") || !strings.Contains(iv.statusErr, "wait") {
+	iv.mu.Lock()
-		t.Fatalf("unknown-command hint = %q; want cancel and wait", iv.statusErr)
+	statusErr := iv.statusErr
+	iv.mu.Unlock()
+	if !strings.Contains(statusErr, "cancel") || !strings.Contains(statusErr, "wait") {
+		t.Fatalf("unknown-command hint = %q; want cancel and wait", statusErr)
 	}
 }
 		t.Fatal(err)
 	}
 	ctx, cancel := context.WithCancel(context.Background())
+	watcherDone := make(chan struct{})
+	iv.subagentsWaitWatcherDone = func() { close(watcherDone) }
 	iv.runSubagents(ctx, []string{"wait", a.ID})
 	iv.mu.Lock()
 	status := iv.statusOK
 	// The wait watcher must stop observing the run context and must not
 	// later overwrite the status with a false completion after cleanup.
-	time.Sleep(20 * time.Millisecond)
+	deadline := time.NewTimer(time.Second)
+	defer deadline.Stop()
+	select {
+	case <-watcherDone:
+	case <-deadline.C:
+		t.Fatal("timed out waiting for wait watcher to exit")
+	}
 	iv.mu.Lock()
 	status = iv.statusOK
 	iv.mu.Unlock()

M packages/agent/subagents/event.go +12 -4


@@ -216,15 +216,23 @@ type eventRing struct {
216	216		}
217	217
218	218		func newEventRing() eventRing {
219		-	return eventRing{events: make([]Event, maxEventLogEvents)}
	219	+	return eventRing{events: make([]Event, 0)}
220	220		}
221	221
222	222		func (r *eventRing) append(ev Event) {
223		-	r.events[r.next] = ev
224		-	r.next = (r.next + 1) % len(r.events)
225		-	if r.count < len(r.events) {
	223	+	if r.count < maxEventLogEvents {
	224	+	r.events = append(r.events, ev)
226	225		r.count++
	226	+	if r.count == maxEventLogEvents {
	227	+	r.next = 0
	228	+	} else {
	229	+	r.next = r.count
	230	+	}
	231	+	return
227	232		}
	233	+
	234	+	r.events[r.next] = ev
	235	+	r.next = (r.next + 1) % len(r.events)
228	236		}
229	237
230	238		func (r *eventRing) last() (Event, bool) {

 }
 func newEventRing() eventRing {
-	return eventRing{events: make([]Event, maxEventLogEvents)}
+	return eventRing{events: make([]Event, 0)}
 }
 func (r *eventRing) append(ev Event) {
-	r.events[r.next] = ev
+	if r.count < maxEventLogEvents {
-	r.next = (r.next + 1) % len(r.events)
+		r.events = append(r.events, ev)
-	if r.count < len(r.events) {
 		r.count++
+		if r.count == maxEventLogEvents {
+			r.next = 0
+		} else {
+			r.next = r.count
+		}
+		return
 	}
+	r.events[r.next] = ev
+	r.next = (r.next + 1) % len(r.events)
 }
 func (r *eventRing) last() (Event, bool) {

M packages/agent/subagents/persist_test.go +8 -1


@@ -171,7 +171,14 @@ func TestResumePreservesPerAgentTimeout(t *testing.T) {
171	171		if err != nil {
172	172		t.Fatal(err)
173	173		}
174		-	observed := <-observedCh
	174	+	var observed time.Duration
	175	+	deadline := time.NewTimer(time.Second)
	176	+	defer deadline.Stop()
	177	+	select {
	178	+	case observed = <-observedCh:
	179	+	case <-deadline.C:
	180	+	t.Fatal("timed out waiting for resumed runner to observe timeout")
	181	+	}
175	182		if resumed.Timeout != wantTimeout \|\| observed != wantTimeout {
176	183		t.Fatalf("resumed timeout = %s, runner observed %s; want %s", resumed.Timeout, observed, wantTimeout)
177	184		}

 	if err != nil {
 		t.Fatal(err)
 	}
-	observed := <-observedCh
+	var observed time.Duration
+	deadline := time.NewTimer(time.Second)
+	defer deadline.Stop()
+	select {
+	case observed = <-observedCh:
+	case <-deadline.C:
+		t.Fatal("timed out waiting for resumed runner to observe timeout")
+	}
 	if resumed.Timeout != wantTimeout || observed != wantTimeout {
 		t.Fatalf("resumed timeout = %s, runner observed %s; want %s", resumed.Timeout, observed, wantTimeout)
 	}

M packages/agent/subagents/process_unix_test.go +3 -3


@@ -71,14 +71,14 @@ func TestWorkerCancellationClosesOutputPipes(t *testing.T) {
71	71		defer stdout.Close()
72	72		defer stderr.Close()
73	73
74		-	configureWorkerProcess(cmd)
	74	+	configureWorkerProcess(cmd, stdout, stderr)
75	75		if err := cmd.Cancel(); err != nil {
76	76		t.Fatal(err)
77	77		}
78	78		for name, pipe := range map[string]io.Reader{"stdout": stdout, "stderr": stderr} {
79	79		data := make([]byte, 1)
80		-	if _, err := pipe.Read(data); err != io.EOF {
81		-	t.Errorf("%s read error = %v, want EOF", name, err)
	80	+	if _, err := pipe.Read(data); !errors.Is(err, os.ErrClosed) {
	81	+	t.Errorf("%s read error = %v, want os.ErrClosed", name, err)
82	82		}
83	83		}
84	84		}


@@ -71,14 +71,14 @@ func TestWorkerCancellationClosesOutputPipes(t *testing.T) {
71		defer stdout.Close()	71		defer stdout.Close()
72		defer stderr.Close()	72		defer stderr.Close()
73			73
74	-	configureWorkerProcess(cmd)	74	+	configureWorkerProcess(cmd, stdout, stderr)
75		if err := cmd.Cancel(); err != nil {	75		if err := cmd.Cancel(); err != nil {
76		t.Fatal(err)	76		t.Fatal(err)
77		}	77		}
78		for name, pipe := range map[string]io.Reader{"stdout": stdout, "stderr": stderr} {	78		for name, pipe := range map[string]io.Reader{"stdout": stdout, "stderr": stderr} {
79		data := make([]byte, 1)	79		data := make([]byte, 1)
80	-	if _, err := pipe.Read(data); err != io.EOF {	80	+	if _, err := pipe.Read(data); !errors.Is(err, os.ErrClosed) {
81	-	t.Errorf("%s read error = %v, want EOF", name, err)	81	+	t.Errorf("%s read error = %v, want os.ErrClosed", name, err)
82		}	82		}
83		}	83		}
84		}	84		}

M packages/agent/subagents/process_windows.go +14 -2


@@ -10,6 +10,7 @@ import (
10	10		"os/exec"
11	11		"strconv"
12	12		"strings"
	13	+	"sync"
13	14		"syscall"
14	15		"time"
15	16		)

@@ -17,11 +18,22 @@ import (
17	18		// configureWorkerProcess makes CommandContext cancellation terminate the
18	19		// worker and its descendants on Windows. Windows does not provide a Unix-like
19	20		// process-group kill operation, so killProcessGroup uses taskkill's tree mode.
20		-	func configureWorkerProcess(cmd *exec.Cmd, _ ...io.Closer) {
	21	+	func configureWorkerProcess(cmd *exec.Cmd, pipes ...io.Closer) {
21	22		setProcessGroup(cmd)
22	23		cmd.WaitDelay = workerProcessWaitDelay
	24	+	var closePipes sync.Once
23	25		cmd.Cancel = func() error {
24		-	return killProcessGroup(cmd)
	26	+	err := killProcessGroup(cmd)
	27	+	closePipes.Do(func() { closeWorkerPipes(pipes) })
	28	+	return err
	29	+	}
	30	+	}
	31	+
	32	+	func closeWorkerPipes(pipes []io.Closer) {
	33	+	for _, pipe := range pipes {
	34	+	if pipe != nil {
	35	+	_ = pipe.Close()
	36	+	}
25	37		}
26	38		}
27	39

 	"os/exec"
 	"strconv"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 )
 // configureWorkerProcess makes CommandContext cancellation terminate the
 // worker and its descendants on Windows. Windows does not provide a Unix-like
 // process-group kill operation, so killProcessGroup uses taskkill's tree mode.
-func configureWorkerProcess(cmd *exec.Cmd, _ ...io.Closer) {
+func configureWorkerProcess(cmd *exec.Cmd, pipes ...io.Closer) {
 	setProcessGroup(cmd)
 	cmd.WaitDelay = workerProcessWaitDelay
+	var closePipes sync.Once
 	cmd.Cancel = func() error {
-		return killProcessGroup(cmd)
+		err := killProcessGroup(cmd)
+		closePipes.Do(func() { closeWorkerPipes(pipes) })
+		return err
+	}
+}
+func closeWorkerPipes(pipes []io.Closer) {
+	for _, pipe := range pipes {
+		if pipe != nil {
+			_ = pipe.Close()
+		}
 	}
 }

M packages/agent/subagents/supervisor.go +1 -8


@@ -313,7 +313,7 @@ type SpawnRequest struct {
313	313		// disables fast mode for this child; true still cannot enable fast mode
314	314		// when Config.FastMode is false. Nil inherits Config.FastMode.
315	315		FastMode *bool
316		-	Subagent string // optional named markdown profile
	316	+	Subagent string // optional named markdown profile
317	317
318	318		// ParentID is metadata used for per-parent scheduling limits. A
319	319		// non-empty RequesterAgentID identifies an untrusted child request;

@@ -847,13 +847,6 @@ func (f *Supervisor) stop(ctx context.Context, id string) error {
847	847		defer timer.Stop()
848	848		select {
849	849		case <-a.done:
850		-	case <-ctx.Done():
851		-	if a.cancel != nil {
852		-	a.cancel()
853		-	}
854		-	if a.inbox != nil {
855		-	_ = a.inbox.Close()
856		-	}
857	850		case <-timer.C:
858	851		if a.cancel != nil {
859	852		a.cancel()

 	// disables fast mode for this child; true still cannot enable fast mode
 	// when Config.FastMode is false. Nil inherits Config.FastMode.
 	FastMode *bool
-	Subagent  string // optional named markdown profile
+	Subagent string // optional named markdown profile
 	// ParentID is metadata used for per-parent scheduling limits. A
 	// non-empty RequesterAgentID identifies an untrusted child request;
 			defer timer.Stop()
 			select {
 			case <-a.done:
-			case <-ctx.Done():
-				if a.cancel != nil {
-					a.cancel()
-				}
-				if a.inbox != nil {
-					_ = a.inbox.Close()
-				}
 			case <-timer.C:
 				if a.cancel != nil {
 					a.cancel()

M packages/agent/subagents/supervisor_test.go +64 -0


@@ -141,6 +141,70 @@ func TestStopCancelsRunningAgent(t *testing.T) {
141	141		}
142	142		}
143	143
	144	+	func TestStopContextCallerCancellationDoesNotEndGracePeriod(t *testing.T) {
	145	+	const grace = 250 * time.Millisecond
	146	+	started := make(chan struct{})
	147	+	canceled := make(chan struct{})
	148	+	f := New(Config{
	149	+	Root: t.TempDir(),
	150	+	RepoRoot: t.TempDir(),
	151	+	Policy: SubagentPolicy{
	152	+	CancelGracePeriod: grace,
	153	+	},
	154	+	NewRunner: func(a *Agent) Runner {
	155	+	return RunnerFunc(func(ctx context.Context, sink Sink) error {
	156	+	listener, err := Listen(a.InboxPath)
	157	+	if err != nil {
	158	+	return err
	159	+	}
	160	+	defer listener.Close()
	161	+	close(started)
	162	+	select {
	163	+	case <-listener.Lines():
	164	+	case <-ctx.Done():
	165	+	close(canceled)
	166	+	return ctx.Err()
	167	+	}
	168	+	<-ctx.Done()
	169	+	close(canceled)
	170	+	return ctx.Err()
	171	+	})
	172	+	},
	173	+	})
	174	+	a, err := f.Spawn(context.Background(), "graceful stop")
	175	+	if err != nil {
	176	+	t.Fatal(err)
	177	+	}
	178	+	select {
	179	+	case <-started:
	180	+	case <-time.After(time.Second):
	181	+	t.Fatal("runner did not start")
	182	+	}
	183	+
	184	+	ctx, cancel := context.WithCancel(context.Background())
	185	+	if err := f.StopContext(ctx, a.ID); err != nil {
	186	+	t.Fatal(err)
	187	+	}
	188	+	cancel()
	189	+
	190	+	early := time.NewTimer(grace / 2)
	191	+	defer early.Stop()
	192	+	select {
	193	+	case <-canceled:
	194	+	t.Fatal("agent canceled when StopContext caller context was canceled")
	195	+	case <-early.C:
	196	+	}
	197	+
	198	+	deadline := time.NewTimer(grace * 2)
	199	+	defer deadline.Stop()
	200	+	select {
	201	+	case <-canceled:
	202	+	case <-deadline.C:
	203	+	t.Fatal("agent was not canceled after the grace period")
	204	+	}
	205	+	a.Wait()
	206	+	}
	207	+
144	208		func TestStopContextCancelsDetachedWaitWithoutHoldingOperationLock(t *testing.T) {
145	209		if runtime.GOOS == "windows" {
146	210		t.Skip("subagent inbox transport uses Unix-domain sockets")


@@ -141,6 +141,70 @@ func TestStopCancelsRunningAgent(t *testing.T) {
141	}	141		}
142	}	142		}
143		143
		144	+	func TestStopContextCallerCancellationDoesNotEndGracePeriod(t *testing.T) {
		145	+	const grace = 250 * time.Millisecond
		146	+	started := make(chan struct{})
		147	+	canceled := make(chan struct{})
		148	+	f := New(Config{
		149	+	Root: t.TempDir(),
		150	+	RepoRoot: t.TempDir(),
		151	+	Policy: SubagentPolicy{
		152	+	CancelGracePeriod: grace,
		153	+	},
		154	+	NewRunner: func(a *Agent) Runner {
		155	+	return RunnerFunc(func(ctx context.Context, sink Sink) error {
		156	+	listener, err := Listen(a.InboxPath)
		157	+	if err != nil {
		158	+	return err
		159	+	}
		160	+	defer listener.Close()
		161	+	close(started)
		162	+	select {
		163	+	case <-listener.Lines():
		164	+	case <-ctx.Done():
		165	+	close(canceled)
		166	+	return ctx.Err()
		167	+	}
		168	+	<-ctx.Done()
		169	+	close(canceled)
		170	+	return ctx.Err()
		171	+	})
		172	+	},
		173	+	})
		174	+	a, err := f.Spawn(context.Background(), "graceful stop")
		175	+	if err != nil {
		176	+	t.Fatal(err)
		177	+	}
		178	+	select {
		179	+	case <-started:
		180	+	case <-time.After(time.Second):
		181	+	t.Fatal("runner did not start")
		182	+	}
		183	+
		184	+	ctx, cancel := context.WithCancel(context.Background())
		185	+	if err := f.StopContext(ctx, a.ID); err != nil {
		186	+	t.Fatal(err)
		187	+	}
		188	+	cancel()
		189	+
		190	+	early := time.NewTimer(grace / 2)
		191	+	defer early.Stop()
		192	+	select {
		193	+	case <-canceled:
		194	+	t.Fatal("agent canceled when StopContext caller context was canceled")
		195	+	case <-early.C:
		196	+	}
		197	+
		198	+	deadline := time.NewTimer(grace * 2)
		199	+	defer deadline.Stop()
		200	+	select {
		201	+	case <-canceled:
		202	+	case <-deadline.C:
		203	+	t.Fatal("agent was not canceled after the grace period")
		204	+	}
		205	+	a.Wait()
		206	+	}
		207	+
144	func TestStopContextCancelsDetachedWaitWithoutHoldingOperationLock(t *testing.T) {	208		func TestStopContextCancelsDetachedWaitWithoutHoldingOperationLock(t *testing.T) {
145	if runtime.GOOS == "windows" {	209		if runtime.GOOS == "windows" {
146	t.Skip("subagent inbox transport uses Unix-domain sockets")	210		t.Skip("subagent inbox transport uses Unix-domain sockets")

M packages/agent/tools/subagent_spawn.go +1 -1


@@ -273,7 +273,7 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, pr
273	273		"reasoning": reasoning,
274	274		"fast_mode": agent.FastMode,
275	275		"isolation": string(workspaceMode),
276		-	"timeout": timeout.String(),
	276	+	"timeout": agent.Timeout.String(),
277	277		"max_turns": agent.MaxTurns,
278	278		"state": agent.Status(),
279	279		"process_state": string(agent.ProcessState()),


@@ -273,7 +273,7 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, pr
273		"reasoning": reasoning,	273		"reasoning": reasoning,
274		"fast_mode": agent.FastMode,	274		"fast_mode": agent.FastMode,
275		"isolation": string(workspaceMode),	275		"isolation": string(workspaceMode),
276	-	"timeout": timeout.String(),	276	+	"timeout": agent.Timeout.String(),
277		"max_turns": agent.MaxTurns,	277		"max_turns": agent.MaxTurns,
278		"state": agent.Status(),	278		"state": agent.Status(),
279		"process_state": string(agent.ProcessState()),	279		"process_state": string(agent.ProcessState()),

M packages/agent/tools/subagent_spawn_test.go +39 -0


@@ -6,6 +6,7 @@ import (
6	6		"path/filepath"
7	7		"strings"
8	8		"testing"
	9	+	"time"
9	10
10	11		"github.com/patriceckhart/zot/packages/agent/subagents"
11	12		"github.com/patriceckhart/zot/packages/provider"

@@ -75,6 +76,44 @@ func TestSubagentSpawnInheritsHostModelAndProviderWhenOmitted(t *testing.T) {
75	76		}
76	77		}
77	78
	79	+	func TestSubagentSpawnDetailsUseEffectiveTimeoutAndTurns(t *testing.T) {
	80	+	root := t.TempDir()
	81	+	manager := subagents.New(subagents.Config{
	82	+	Root: filepath.Join(root, "subagents"),
	83	+	RepoRoot: root,
	84	+	Policy: subagents.SubagentPolicy{
	85	+	DefaultTimeout: 37 * time.Minute,
	86	+	MaxTurns: 7,
	87	+	},
	88	+	NewRunner: func(*subagents.Agent) subagents.Runner {
	89	+	return noopSupervisorRunner{}
	90	+	},
	91	+	})
	92	+	t.Cleanup(manager.StopAll)
	93	+	tool := &SubagentSpawnTool{
	94	+	Supervisor: manager,
	95	+	Enabled: func() bool { return true },
	96	+	}
	97	+
	98	+	res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs"}`), nil)
	99	+	if err != nil {
	100	+	t.Fatal(err)
	101	+	}
	102	+	if res.IsError {
	103	+	t.Fatalf("unexpected tool error: %s", textResult(res.Content))
	104	+	}
	105	+	details, ok := res.Details.(map[string]any)
	106	+	if !ok {
	107	+	t.Fatalf("details type = %T, want map[string]any", res.Details)
	108	+	}
	109	+	if got := details["timeout"]; got != (37 * time.Minute).String() {
	110	+	t.Fatalf("omitted timeout detail = %v, want %s", got, (37 * time.Minute).String())
	111	+	}
	112	+	if got := details["max_turns"]; got != 7 {
	113	+	t.Fatalf("max_turns detail = %v, want 7", got)
	114	+	}
	115	+	}
	116	+
78	117		func TestSubagentSpawnRejectsExplicitZeroMaxTurns(t *testing.T) {
79	118		tool := &SubagentSpawnTool{
80	119		Supervisor: newTestSupervisor(t),


@@ -6,6 +6,7 @@ import (
6	"path/filepath"	6		"path/filepath"
7	"strings"	7		"strings"
8	"testing"	8		"testing"
		9	+	"time"
9		10
10	"github.com/patriceckhart/zot/packages/agent/subagents"	11		"github.com/patriceckhart/zot/packages/agent/subagents"
11	"github.com/patriceckhart/zot/packages/provider"	12		"github.com/patriceckhart/zot/packages/provider"

@@ -75,6 +76,44 @@ func TestSubagentSpawnInheritsHostModelAndProviderWhenOmitted(t *testing.T) {
75	}	76		}
76	}	77		}
77		78
		79	+	func TestSubagentSpawnDetailsUseEffectiveTimeoutAndTurns(t *testing.T) {
		80	+	root := t.TempDir()
		81	+	manager := subagents.New(subagents.Config{
		82	+	Root: filepath.Join(root, "subagents"),
		83	+	RepoRoot: root,
		84	+	Policy: subagents.SubagentPolicy{
		85	+	DefaultTimeout: 37 * time.Minute,
		86	+	MaxTurns: 7,
		87	+	},
		88	+	NewRunner: func(*subagents.Agent) subagents.Runner {
		89	+	return noopSupervisorRunner{}
		90	+	},
		91	+	})
		92	+	t.Cleanup(manager.StopAll)
		93	+	tool := &SubagentSpawnTool{
		94	+	Supervisor: manager,
		95	+	Enabled: func() bool { return true },
		96	+	}
		97	+
		98	+	res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs"}`), nil)
		99	+	if err != nil {
		100	+	t.Fatal(err)
		101	+	}
		102	+	if res.IsError {
		103	+	t.Fatalf("unexpected tool error: %s", textResult(res.Content))
		104	+	}
		105	+	details, ok := res.Details.(map[string]any)
		106	+	if !ok {
		107	+	t.Fatalf("details type = %T, want map[string]any", res.Details)
		108	+	}
		109	+	if got := details["timeout"]; got != (37 * time.Minute).String() {
		110	+	t.Fatalf("omitted timeout detail = %v, want %s", got, (37 * time.Minute).String())
		111	+	}
		112	+	if got := details["max_turns"]; got != 7 {
		113	+	t.Fatalf("max_turns detail = %v, want 7", got)
		114	+	}
		115	+	}
		116	+
78	func TestSubagentSpawnRejectsExplicitZeroMaxTurns(t *testing.T) {	117		func TestSubagentSpawnRejectsExplicitZeroMaxTurns(t *testing.T) {
79	tool := &SubagentSpawnTool{	118		tool := &SubagentSpawnTool{
80	Supervisor: newTestSupervisor(t),	119		Supervisor: newTestSupervisor(t),

M packages/core/agent.go +9 -6


@@ -16,9 +16,13 @@ import (
16	16		// Agent is a stateful conversation bound to a provider client, a model,
17	17		// and a set of tools.
18	18		type Agent struct {
19		-	Client provider.Client
20		-	Model string
21		-	System string
	19	+	Client provider.Client
	20	+	Model string
	21	+	// System is mutex-protected; use SetSystemPrompt to write it and
	22	+	// PromptConfig to read it.
	23	+	System string
	24	+	// Tools is mutex-protected; use SetTools to write it and PromptConfig
	25	+	// or ToolsSnapshot to read it.
22	26		Tools Registry
23	27		MaxSteps int
24	28		Reasoning string

@@ -760,7 +764,7 @@ func (a *Agent) executeTools(ctx context.Context, msg provider.Message, sink fun
760	764		if !ok {
761	765		continue
762	766		}
763		-	res := a.runOneTool(ctx, tc, sink)
	767	+	res := a.runOneTool(ctx, tc, tools, sink)
764	768		if res.IsError {
765	769		hadError = true
766	770		}

@@ -788,8 +792,7 @@ func (a *Agent) executeTools(ctx context.Context, msg provider.Message, sink fun
788	792		}, hadError
789	793		}
790	794
791		-	func (a *Agent) runOneTool(ctx context.Context, tc provider.ToolCallBlock, sink func(AgentEvent)) ToolResult {
792		-	tools := a.ToolsSnapshot()
	795	+	func (a *Agent) runOneTool(ctx context.Context, tc provider.ToolCallBlock, tools Registry, sink func(AgentEvent)) ToolResult {
793	796		tool, err := tools.Get(tc.Name)
794	797		if err != nil {
795	798		return ToolResult{

 // Agent is a stateful conversation bound to a provider client, a model,
 // and a set of tools.
 type Agent struct {
-	Client      provider.Client
+	Client provider.Client
-	Model       string
+	Model  string
-	System      string
+	// System is mutex-protected; use SetSystemPrompt to write it and
+	// PromptConfig to read it.
+	System string
+	// Tools is mutex-protected; use SetTools to write it and PromptConfig
+	// or ToolsSnapshot to read it.
 	Tools       Registry
 	MaxSteps    int
 	Reasoning   string
 		if !ok {
 			continue
 		}
-		res := a.runOneTool(ctx, tc, sink)
+		res := a.runOneTool(ctx, tc, tools, sink)
 		if res.IsError {
 			hadError = true
 		}
 	}, hadError
 }
-func (a *Agent) runOneTool(ctx context.Context, tc provider.ToolCallBlock, sink func(AgentEvent)) ToolResult {
+func (a *Agent) runOneTool(ctx context.Context, tc provider.ToolCallBlock, tools Registry, sink func(AgentEvent)) ToolResult {
-	tools := a.ToolsSnapshot()
 	tool, err := tools.Get(tc.Name)
 	if err != nil {
 		return ToolResult{

M packages/core/intercept_test.go +6 -3


@@ -39,11 +39,12 @@ func TestBeforeToolExecuteModifiesArgs(t *testing.T) {
39	39		}
40	40
41	41		ctx := context.Background()
	42	+	tools := a.ToolsSnapshot()
42	43		res := a.runOneTool(ctx, provider.ToolCallBlock{
43	44		ID: "T1",
44	45		Name: "echo",
45	46		Arguments: json.RawMessage(`{"command":"ls"}`),
46		-	}, func(AgentEvent) {})
	47	+	}, tools, func(AgentEvent) {})
47	48		if res.IsError {
48	49		t.Fatalf("unexpected error result: %v", res.Content)
49	50		}

@@ -65,12 +66,13 @@ func TestBeforeToolExecuteInvalidJSONIgnored(t *testing.T) {
65	66		}
66	67
67	68		ctx := context.Background()
	69	+	tools := a.ToolsSnapshot()
68	70		orig := json.RawMessage(`{"command":"ls"}`)
69	71		a.runOneTool(ctx, provider.ToolCallBlock{
70	72		ID: "T1",
71	73		Name: "echo",
72	74		Arguments: orig,
73		-	}, func(AgentEvent) {})
	75	+	}, tools, func(AgentEvent) {})
74	76		if string(rec.lastArgs) != string(orig) {
75	77		t.Errorf("tool saw %s, want original %s", string(rec.lastArgs), string(orig))
76	78		}

@@ -88,11 +90,12 @@ func TestBeforeToolExecuteBlockSurfacesReason(t *testing.T) {
88	90		}
89	91
90	92		ctx := context.Background()
	93	+	tools := a.ToolsSnapshot()
91	94		res := a.runOneTool(ctx, provider.ToolCallBlock{
92	95		ID: "T1",
93	96		Name: "echo",
94	97		Arguments: json.RawMessage(`{"command":"ls"}`),
95		-	}, func(AgentEvent) {})
	98	+	}, tools, func(AgentEvent) {})
96	99		if !res.IsError {
97	100		t.Fatal("want error result, got success")
98	101		}

 	}
 	ctx := context.Background()
+	tools := a.ToolsSnapshot()
 	res := a.runOneTool(ctx, provider.ToolCallBlock{
 		ID:        "T1",
 		Name:      "echo",
 		Arguments: json.RawMessage(`{"command":"ls"}`),
-	}, func(AgentEvent) {})
+	}, tools, func(AgentEvent) {})
 	if res.IsError {
 		t.Fatalf("unexpected error result: %v", res.Content)
 	}
 	}
 	ctx := context.Background()
+	tools := a.ToolsSnapshot()
 	orig := json.RawMessage(`{"command":"ls"}`)
 	a.runOneTool(ctx, provider.ToolCallBlock{
 		ID:        "T1",
 		Name:      "echo",
 		Arguments: orig,
-	}, func(AgentEvent) {})
+	}, tools, func(AgentEvent) {})
 	if string(rec.lastArgs) != string(orig) {
 		t.Errorf("tool saw %s, want original %s", string(rec.lastArgs), string(orig))
 	}
 	}
 	ctx := context.Background()
+	tools := a.ToolsSnapshot()
 	res := a.runOneTool(ctx, provider.ToolCallBlock{
 		ID:        "T1",
 		Name:      "echo",
 		Arguments: json.RawMessage(`{"command":"ls"}`),
-	}, func(AgentEvent) {})
+	}, tools, func(AgentEvent) {})
 	if !res.IsError {
 		t.Fatal("want error result, got success")
 	}