Skip to content

Commit a73424e

Browse files
committed
fix: decouple elicitation channel from ToolsApproved flag
The elicitation events channel was guarded by ToolsApproved, which meant sessions with ToolsApproved=true (serve mcp, serve a2a, background agents, delegated sub-sessions) could never handle elicitation requests, always failing with 'no events channel available for elicitation'. Replace the set/clear pattern with a save/restore pattern: every RunStream call now unconditionally sets its events channel as the elicitation target, saving the previous one. On teardown it restores the previous channel instead of clearing to nil. This correctly handles nested RunStream calls (sub-sessions, background agents) without losing the parent's channel. Assisted-By: docker-agent
1 parent ebd216e commit a73424e

2 files changed

Lines changed: 22 additions & 30 deletions

File tree

pkg/runtime/loop.go

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,13 @@ func (r *LocalRuntime) registerDefaultTools() {
4242
}
4343

4444
// finalizeEventChannel performs cleanup at the end of a RunStream goroutine:
45-
// clears elicitation state, emits the StreamStopped event, fires hooks, and
46-
// closes the events channel.
47-
func (r *LocalRuntime) finalizeEventChannel(ctx context.Context, sess *session.Session, events chan Event) {
48-
// Clear the elicitation events channel before closing the events channel
49-
// to prevent a send-on-closed-channel panic in elicitationHandler.
50-
// Skip for background sessions (ToolsApproved=true) — they never set the
51-
// channel, so clearing it would null out the parent session's channel.
52-
if !sess.ToolsApproved {
53-
r.clearElicitationEventsChannel()
54-
}
45+
// restores the previous elicitation channel, emits the StreamStopped event,
46+
// fires hooks, and closes the events channel.
47+
func (r *LocalRuntime) finalizeEventChannel(ctx context.Context, sess *session.Session, prevElicitationCh, events chan Event) {
48+
// Swap back the parent's elicitation channel before closing this
49+
// stream's channel. This prevents a send-on-closed-channel panic
50+
// and restores elicitation for the parent session.
51+
r.swapElicitationEventsChannel(prevElicitationCh)
5552

5653
defer close(events)
5754

@@ -80,14 +77,11 @@ func (r *LocalRuntime) RunStream(ctx context.Context, sess *session.Session) <-c
8077
))
8178
defer sessionSpan.End()
8279

83-
// Set the events channel for elicitation requests.
84-
// Skip for background sessions (ToolsApproved=true): they have all tools
85-
// pre-approved and will never trigger elicitation prompts. Setting the
86-
// channel would overwrite the parent session's channel; clearing it at
87-
// teardown would break any pending MCP auth flow in the parent.
88-
if !sess.ToolsApproved {
89-
r.setElicitationEventsChannel(events)
90-
}
80+
// Swap in this stream's events channel for elicitation and save the
81+
// previous one so it can be restored on teardown. This allows nested
82+
// RunStream calls to temporarily own elicitation without losing the
83+
// parent's channel.
84+
prevElicitationCh := r.swapElicitationEventsChannel(events)
9185

9286
a := r.resolveSessionAgent(sess)
9387

@@ -120,7 +114,7 @@ func (r *LocalRuntime) RunStream(ctx context.Context, sess *session.Session) <-c
120114

121115
events <- StreamStarted(sess.ID, a.Name())
122116

123-
defer r.finalizeEventChannel(ctx, sess, events)
117+
defer r.finalizeEventChannel(ctx, sess, prevElicitationCh, events)
124118

125119
r.registerDefaultTools()
126120

pkg/runtime/runtime.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -998,18 +998,16 @@ func (r *LocalRuntime) Summarize(ctx context.Context, sess *session.Session, add
998998
events <- NewTokenUsageEvent(sess.ID, a.Name(), SessionUsage(sess, contextLimit))
999999
}
10001000

1001-
// setElicitationEventsChannel sets the current events channel for elicitation requests
1002-
func (r *LocalRuntime) setElicitationEventsChannel(events chan Event) {
1001+
// swapElicitationEventsChannel atomically replaces the current elicitation
1002+
// events channel and returns the previous one. Each RunStream call swaps in
1003+
// its own channel on entry and swaps the previous one back on exit, so nested
1004+
// streams (sub-sessions, background agents) don't lose the parent's channel.
1005+
func (r *LocalRuntime) swapElicitationEventsChannel(ch chan Event) chan Event {
10031006
r.elicitationEventsChannelMux.Lock()
10041007
defer r.elicitationEventsChannelMux.Unlock()
1005-
r.elicitationEventsChannel = events
1006-
}
1007-
1008-
// clearElicitationEventsChannel clears the current events channel
1009-
func (r *LocalRuntime) clearElicitationEventsChannel() {
1010-
r.elicitationEventsChannelMux.Lock()
1011-
defer r.elicitationEventsChannelMux.Unlock()
1012-
r.elicitationEventsChannel = nil
1008+
prev := r.elicitationEventsChannel
1009+
r.elicitationEventsChannel = ch
1010+
return prev
10131011
}
10141012

10151013
// elicitationHandler creates an elicitation handler that can be used by MCP clients
@@ -1018,7 +1016,7 @@ func (r *LocalRuntime) elicitationHandler(ctx context.Context, req *mcp.ElicitPa
10181016
slog.Debug("Elicitation request received from MCP server", "message", req.Message)
10191017

10201018
// Hold the read lock while sending to the channel to prevent a race
1021-
// with clearElicitationEventsChannel / close(events).
1019+
// with swapElicitationEventsChannel / close(events).
10221020
r.elicitationEventsChannelMux.RLock()
10231021
eventsChannel := r.elicitationEventsChannel
10241022
if eventsChannel == nil {

0 commit comments

Comments
 (0)