Skip to content

Commit a6e51ce

Browse files
committed
codemode: fix Start() fail-fast and use tools.As for wrapper unwrapping
Start() returned on the first error, so one broken toolset (e.g. an unreachable MCP server) prevented all remaining toolsets from starting. Collect all errors with errors.Join, matching the existing Stop() behavior. Both Start() and Stop() used a direct type assertion to find Startable, which cannot see through wrapper chains (e.g. StartableToolSet). Switch to tools.As so the capability is discovered at any depth. Extract the now-identical loops into a shared forEachStartable helper to remove the duplication. Assisted-By: docker-agent
1 parent c45c010 commit a6e51ce

File tree

2 files changed

+45
-17
lines changed

2 files changed

+45
-17
lines changed

pkg/tools/codemode/codemode.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -103,27 +103,23 @@ func (c *codeModeTool) Tools(ctx context.Context) ([]tools.Tool, error) {
103103
}
104104

105105
func (c *codeModeTool) Start(ctx context.Context) error {
106-
for _, t := range c.toolsets {
107-
if startable, ok := t.(tools.Startable); ok {
108-
if err := startable.Start(ctx); err != nil {
109-
return err
110-
}
111-
}
112-
}
113-
114-
return nil
106+
return c.forEachStartable(func(s tools.Startable) error { return s.Start(ctx) })
115107
}
116108

117109
func (c *codeModeTool) Stop(ctx context.Context) error {
118-
var errs []error
110+
return c.forEachStartable(func(s tools.Startable) error { return s.Stop(ctx) })
111+
}
119112

113+
// forEachStartable applies fn to every toolset that implements Startable,
114+
// collecting all errors so that one broken toolset doesn't block the others.
115+
func (c *codeModeTool) forEachStartable(fn func(tools.Startable) error) error {
116+
var errs []error
120117
for _, t := range c.toolsets {
121-
if startable, ok := t.(tools.Startable); ok {
122-
if err := startable.Stop(ctx); err != nil {
118+
if s, ok := tools.As[tools.Startable](t); ok {
119+
if err := fn(s); err != nil {
123120
errs = append(errs, err)
124121
}
125122
}
126123
}
127-
128124
return errors.Join(errs...)
129125
}

pkg/tools/codemode/codemode_test.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,42 @@ func TestCodeModeTool_CallEcho(t *testing.T) {
187187
require.Empty(t, scriptResult.StdOut)
188188
}
189189

190+
// TestCodeModeTool_StartContinuesOnError verifies that a failing Start on one
191+
// toolset does not prevent the remaining toolsets from being started.
192+
func TestCodeModeTool_StartContinuesOnError(t *testing.T) {
193+
failing := &testToolSet{startErr: assert.AnError}
194+
healthy := &testToolSet{}
195+
196+
tool := Wrap(failing, healthy).(tools.Startable)
197+
198+
err := tool.Start(t.Context())
199+
require.ErrorIs(t, err, assert.AnError)
200+
assert.Equal(t, 1, failing.start, "failing toolset should have been started")
201+
assert.Equal(t, 1, healthy.start, "healthy toolset should still be started")
202+
}
203+
204+
// TestCodeModeTool_StartStopWrappedToolSet verifies that Start/Stop find
205+
// Startable through a StartableToolSet wrapper via tools.As.
206+
func TestCodeModeTool_StartStopWrappedToolSet(t *testing.T) {
207+
inner := &testToolSet{}
208+
wrapped := tools.NewStartable(inner)
209+
210+
tool := Wrap(wrapped).(tools.Startable)
211+
212+
err := tool.Start(t.Context())
213+
require.NoError(t, err)
214+
assert.Equal(t, 1, inner.start)
215+
216+
err = tool.Stop(t.Context())
217+
require.NoError(t, err)
218+
assert.Equal(t, 1, inner.stop)
219+
}
220+
190221
type testToolSet struct {
191-
tools []tools.Tool
192-
start int
193-
stop int
222+
tools []tools.Tool
223+
start int
224+
stop int
225+
startErr error
194226
}
195227

196228
// Verify interface compliance
@@ -205,7 +237,7 @@ func (t *testToolSet) Tools(context.Context) ([]tools.Tool, error) {
205237

206238
func (t *testToolSet) Start(context.Context) error {
207239
t.start++
208-
return nil
240+
return t.startErr
209241
}
210242

211243
func (t *testToolSet) Stop(context.Context) error {

0 commit comments

Comments
 (0)