Skip to content

Commit 59828bf

Browse files
authored
Merge pull request #2080 from dgageot/fix-elicitations
fix: decouple elicitation channel from ToolsApproved flag
2 parents 8a026a3 + a73424e commit 59828bf

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)