fix(modes): guard stale side-chat confirmations
- commit
- c2e9fac2b0c8593048429da8cdca64b1fd8e5416
- author
- brice · · 2026-08-06 08:00 UTC
- parent
- 29820a2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +178 -39
M packages/agent/modes/activity.go +20 -20
| @@ -111,24 +111,24 @@ func (a *agentActivity) apply(ev core.AgentEvent) { | |||
| 111 | 111 | } | |
| 112 | 112 | switch e := ev.(type) { | |
| 113 | 113 | case core.EvTurnStart: | |
| 114 | - | a.activity.kind = activityPreparingRequest | |
| 114 | + | a.kind = activityPreparingRequest | |
| 115 | 115 | a.pendingToolCalls = nil | |
| 116 | 116 | a.toolOrder = nil | |
| 117 | 117 | case core.EvRequestStarted: | |
| 118 | - | a.activity.kind = activitySendingRequest | |
| 118 | + | a.kind = activitySendingRequest | |
| 119 | 119 | if e.Provider != "" { | |
| 120 | - | a.activity.provider = e.Provider | |
| 120 | + | a.provider = e.Provider | |
| 121 | 121 | } | |
| 122 | 122 | if e.Model != "" { | |
| 123 | - | a.activity.model = e.Model | |
| 123 | + | a.model = e.Model | |
| 124 | 124 | } | |
| 125 | 125 | case core.EvAssistantStart: | |
| 126 | - | a.activity.kind = activityWaitingForResponse | |
| 126 | + | a.kind = activityWaitingForResponse | |
| 127 | 127 | case core.EvTextDelta: | |
| 128 | - | a.activity.kind = activityReceivingResponse | |
| 128 | + | a.kind = activityReceivingResponse | |
| 129 | 129 | case core.EvToolUseStart: | |
| 130 | - | a.activity.kind = activityPreparingTool | |
| 131 | - | a.activity.tool = e.Name | |
| 130 | + | a.kind = activityPreparingTool | |
| 131 | + | a.tool = e.Name | |
| 132 | 132 | case core.EvToolCall: | |
| 133 | 133 | if a.pendingToolCalls == nil { | |
| 134 | 134 | a.pendingToolCalls = make(map[string]string) | |
| @@ -137,25 +137,25 @@ func (a *agentActivity) apply(ev core.AgentEvent) { | |||
| 137 | 137 | a.toolOrder = append(a.toolOrder, e.ID) | |
| 138 | 138 | } | |
| 139 | 139 | a.pendingToolCalls[e.ID] = e.Name | |
| 140 | - | a.activity.kind = activityPreparingTool | |
| 141 | - | a.activity.tool = e.Name | |
| 140 | + | a.kind = activityPreparingTool | |
| 141 | + | a.tool = e.Name | |
| 142 | 142 | case core.EvToolExecutionStarted: | |
| 143 | - | a.activity.kind = activityRunningTool | |
| 144 | - | a.activity.tool = e.Name | |
| 143 | + | a.kind = activityRunningTool | |
| 144 | + | a.tool = e.Name | |
| 145 | 145 | case core.EvToolResult: | |
| 146 | 146 | if _, exists := a.pendingToolCalls[e.ID]; !exists { | |
| 147 | 147 | return | |
| 148 | 148 | } | |
| 149 | 149 | delete(a.pendingToolCalls, e.ID) | |
| 150 | 150 | if len(a.pendingToolCalls) == 0 { | |
| 151 | - | a.activity.kind = activitySendingToolResults | |
| 152 | - | a.activity.tool = "" | |
| 151 | + | a.kind = activitySendingToolResults | |
| 152 | + | a.tool = "" | |
| 153 | 153 | return | |
| 154 | 154 | } | |
| 155 | 155 | for _, id := range a.toolOrder { | |
| 156 | 156 | if name, ok := a.pendingToolCalls[id]; ok { | |
| 157 | - | a.activity.kind = activityPreparingTool | |
| 158 | - | a.activity.tool = name | |
| 157 | + | a.kind = activityPreparingTool | |
| 158 | + | a.tool = name | |
| 159 | 159 | return | |
| 160 | 160 | } | |
| 161 | 161 | } | |
| @@ -167,9 +167,9 @@ func (a *agentActivity) apply(ev core.AgentEvent) { | |||
| 167 | 167 | a.pendingToolCalls = nil | |
| 168 | 168 | a.toolOrder = nil | |
| 169 | 169 | } | |
| 170 | - | a.activity.kind = activityRetryingRequest | |
| 171 | - | a.activity.attempt = e.Attempt | |
| 172 | - | a.activity.maxTries = e.MaxAttempts | |
| 173 | - | a.activity.retryIn = e.Delay | |
| 170 | + | a.kind = activityRetryingRequest | |
| 171 | + | a.attempt = e.Attempt | |
| 172 | + | a.maxTries = e.MaxAttempts | |
| 173 | + | a.retryIn = e.Delay | |
| 174 | 174 | } | |
| 175 | 175 | } | |
| @@ -111,24 +111,24 @@ func (a *agentActivity) apply(ev core.AgentEvent) { | |||||
| 111 | } | 111 | } | ||
| 112 | switch e := ev.(type) { | 112 | switch e := ev.(type) { | ||
| 113 | case core.EvTurnStart: | 113 | case core.EvTurnStart: | ||
| 114 | - | a.activity.kind = activityPreparingRequest | 114 | + | a.kind = activityPreparingRequest |
| 115 | a.pendingToolCalls = nil | 115 | a.pendingToolCalls = nil | ||
| 116 | a.toolOrder = nil | 116 | a.toolOrder = nil | ||
| 117 | case core.EvRequestStarted: | 117 | case core.EvRequestStarted: | ||
| 118 | - | a.activity.kind = activitySendingRequest | 118 | + | a.kind = activitySendingRequest |
| 119 | if e.Provider != "" { | 119 | if e.Provider != "" { | ||
| 120 | - | a.activity.provider = e.Provider | 120 | + | a.provider = e.Provider |
| 121 | } | 121 | } | ||
| 122 | if e.Model != "" { | 122 | if e.Model != "" { | ||
| 123 | - | a.activity.model = e.Model | 123 | + | a.model = e.Model |
| 124 | } | 124 | } | ||
| 125 | case core.EvAssistantStart: | 125 | case core.EvAssistantStart: | ||
| 126 | - | a.activity.kind = activityWaitingForResponse | 126 | + | a.kind = activityWaitingForResponse |
| 127 | case core.EvTextDelta: | 127 | case core.EvTextDelta: | ||
| 128 | - | a.activity.kind = activityReceivingResponse | 128 | + | a.kind = activityReceivingResponse |
| 129 | case core.EvToolUseStart: | 129 | case core.EvToolUseStart: | ||
| 130 | - | a.activity.kind = activityPreparingTool | 130 | + | a.kind = activityPreparingTool |
| 131 | - | a.activity.tool = e.Name | 131 | + | a.tool = e.Name |
| 132 | case core.EvToolCall: | 132 | case core.EvToolCall: | ||
| 133 | if a.pendingToolCalls == nil { | 133 | if a.pendingToolCalls == nil { | ||
| 134 | a.pendingToolCalls = make(map[string]string) | 134 | a.pendingToolCalls = make(map[string]string) | ||
| @@ -137,25 +137,25 @@ func (a *agentActivity) apply(ev core.AgentEvent) { | |||||
| 137 | a.toolOrder = append(a.toolOrder, e.ID) | 137 | a.toolOrder = append(a.toolOrder, e.ID) | ||
| 138 | } | 138 | } | ||
| 139 | a.pendingToolCalls[e.ID] = e.Name | 139 | a.pendingToolCalls[e.ID] = e.Name | ||
| 140 | - | a.activity.kind = activityPreparingTool | 140 | + | a.kind = activityPreparingTool |
| 141 | - | a.activity.tool = e.Name | 141 | + | a.tool = e.Name |
| 142 | case core.EvToolExecutionStarted: | 142 | case core.EvToolExecutionStarted: | ||
| 143 | - | a.activity.kind = activityRunningTool | 143 | + | a.kind = activityRunningTool |
| 144 | - | a.activity.tool = e.Name | 144 | + | a.tool = e.Name |
| 145 | case core.EvToolResult: | 145 | case core.EvToolResult: | ||
| 146 | if _, exists := a.pendingToolCalls[e.ID]; !exists { | 146 | if _, exists := a.pendingToolCalls[e.ID]; !exists { | ||
| 147 | return | 147 | return | ||
| 148 | } | 148 | } | ||
| 149 | delete(a.pendingToolCalls, e.ID) | 149 | delete(a.pendingToolCalls, e.ID) | ||
| 150 | if len(a.pendingToolCalls) == 0 { | 150 | if len(a.pendingToolCalls) == 0 { | ||
| 151 | - | a.activity.kind = activitySendingToolResults | 151 | + | a.kind = activitySendingToolResults |
| 152 | - | a.activity.tool = "" | 152 | + | a.tool = "" |
| 153 | return | 153 | return | ||
| 154 | } | 154 | } | ||
| 155 | for _, id := range a.toolOrder { | 155 | for _, id := range a.toolOrder { | ||
| 156 | if name, ok := a.pendingToolCalls[id]; ok { | 156 | if name, ok := a.pendingToolCalls[id]; ok { | ||
| 157 | - | a.activity.kind = activityPreparingTool | 157 | + | a.kind = activityPreparingTool |
| 158 | - | a.activity.tool = name | 158 | + | a.tool = name |
| 159 | return | 159 | return | ||
| 160 | } | 160 | } | ||
| 161 | } | 161 | } | ||
| @@ -167,9 +167,9 @@ func (a *agentActivity) apply(ev core.AgentEvent) { | |||||
| 167 | a.pendingToolCalls = nil | 167 | a.pendingToolCalls = nil | ||
| 168 | a.toolOrder = nil | 168 | a.toolOrder = nil | ||
| 169 | } | 169 | } | ||
| 170 | - | a.activity.kind = activityRetryingRequest | 170 | + | a.kind = activityRetryingRequest |
| 171 | - | a.activity.attempt = e.Attempt | 171 | + | a.attempt = e.Attempt |
| 172 | - | a.activity.maxTries = e.MaxAttempts | 172 | + | a.maxTries = e.MaxAttempts |
| 173 | - | a.activity.retryIn = e.Delay | 173 | + | a.retryIn = e.Delay |
| 174 | } | 174 | } | ||
| 175 | } | 175 | } | ||
M packages/agent/modes/btw_dialog.go +35 -7
| @@ -40,6 +40,7 @@ type btwDialog struct { | |||
| 40 | 40 | lineInput bool | |
| 41 | 41 | cancel context.CancelFunc | |
| 42 | 42 | generation uint64 | |
| 43 | + | closeHook func() | |
| 43 | 44 | ||
| 44 | 45 | // spin drives the same braille animation as the main status bar. Its | |
| 45 | 46 | // activity label is owned by this dialog under d.mu. | |
| @@ -68,6 +69,23 @@ func newBtwDialog() *btwDialog { | |||
| 68 | 69 | return &btwDialog{} | |
| 69 | 70 | } | |
| 70 | 71 | ||
| 72 | + | func btwOrigin(generation uint64) string { | |
| 73 | + | return fmt.Sprintf("btw:%d", generation) | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | func isBtwOrigin(origin string) bool { | |
| 77 | + | return strings.HasPrefix(origin, "btw:") | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func (d *btwDialog) setCloseHook(hook func()) { | |
| 81 | + | if d == nil { | |
| 82 | + | return | |
| 83 | + | } | |
| 84 | + | d.mu.Lock() | |
| 85 | + | d.closeHook = hook | |
| 86 | + | d.mu.Unlock() | |
| 87 | + | } | |
| 88 | + | ||
| 71 | 89 | // Active reports whether the dialog is visible and consuming keys. | |
| 72 | 90 | func (d *btwDialog) Active() bool { | |
| 73 | 91 | if d == nil { | |
| @@ -92,19 +110,25 @@ func (d *btwDialog) Loading() bool { | |||
| 92 | 110 | return d.active && d.loading | |
| 93 | 111 | } | |
| 94 | 112 | ||
| 95 | - | // SetToolPreview attaches the side-effect-free confirmation preview to the | |
| 96 | - | // matching side-chat tool box before execution begins. | |
| 97 | - | func (d *btwDialog) SetToolPreview(id, summary, content string) bool { | |
| 98 | - | if d == nil || id == "" { | |
| 113 | + | // enqueueToolConfirmation attaches the side-effect-free confirmation preview | |
| 114 | + | // to the matching side-chat tool box and queues its prompt while d.mu is | |
| 115 | + | // held. Keeping both actions together prevents Close from advancing the | |
| 116 | + | // generation between the ownership check and the confirmation enqueue. | |
| 117 | + | func (d *btwDialog) enqueueToolConfirmation(origin, id, summary, content string, enqueue func()) bool { | |
| 118 | + | if d == nil || id == "" || enqueue == nil { | |
| 99 | 119 | return false | |
| 100 | 120 | } | |
| 101 | 121 | d.mu.Lock() | |
| 102 | 122 | defer d.mu.Unlock() | |
| 123 | + | if !d.active || origin != btwOrigin(d.generation) { | |
| 124 | + | return false | |
| 125 | + | } | |
| 103 | 126 | for turnIdx := range d.turns { | |
| 104 | 127 | if tool := findBtwTool(&d.turns[turnIdx], id); tool != nil { | |
| 105 | 128 | tool.Args = summary | |
| 106 | 129 | tool.Preview = content | |
| 107 | 130 | d.activity.activity = activity{kind: activityAwaitingConfirmation, tool: tool.Name} | |
| 131 | + | enqueue() | |
| 108 | 132 | return true | |
| 109 | 133 | } | |
| 110 | 134 | } | |
| @@ -146,7 +170,7 @@ func (d *btwDialog) Open(th tui.Theme, agent *core.Agent, system, model, cwd, se | |||
| 146 | 170 | prompt = "" | |
| 147 | 171 | } | |
| 148 | 172 | d.editor = tui.NewEditor(prompt) | |
| 149 | - | d.sideAgent = newBtwAgent(agent, system, model) | |
| 173 | + | d.sideAgent = newBtwAgent(agent, system, model, btwOrigin(d.generation)) | |
| 150 | 174 | d.cwd = cwd | |
| 151 | 175 | d.mu.Unlock() | |
| 152 | 176 | ||
| @@ -169,10 +193,14 @@ func (d *btwDialog) Close() { | |||
| 169 | 193 | d.cancel = nil | |
| 170 | 194 | d.sideAgent = nil | |
| 171 | 195 | d.toolView = nil | |
| 196 | + | closeHook := d.closeHook | |
| 172 | 197 | d.mu.Unlock() | |
| 173 | 198 | if cancel != nil { | |
| 174 | 199 | cancel() | |
| 175 | 200 | } | |
| 201 | + | if closeHook != nil { | |
| 202 | + | closeHook() | |
| 203 | + | } | |
| 176 | 204 | } | |
| 177 | 205 | ||
| 178 | 206 | // HandleKey routes a keypress to the dialog. Returns true if the | |
| @@ -286,7 +314,7 @@ func (d *btwDialog) submit(invalidate func()) { | |||
| 286 | 314 | }() | |
| 287 | 315 | } | |
| 288 | 316 | ||
| 289 | - | func newBtwAgent(main *core.Agent, system, model string) *core.Agent { | |
| 317 | + | func newBtwAgent(main *core.Agent, system, model, origin string) *core.Agent { | |
| 290 | 318 | agent := core.NewAgent(main.Client, model, system, main.ToolsSnapshot()) | |
| 291 | 319 | agent.MaxSteps = main.MaxSteps | |
| 292 | 320 | agent.Reasoning = main.Reasoning | |
| @@ -295,7 +323,7 @@ func newBtwAgent(main *core.Agent, system, model string) *core.Agent { | |||
| 295 | 323 | agent.MaxTokens = main.MaxTokens | |
| 296 | 324 | if beforeToolExecute := main.BeforeToolExecute; beforeToolExecute != nil { | |
| 297 | 325 | agent.BeforeToolExecute = func(call provider.ToolCallBlock) (bool, string, json.RawMessage) { | |
| 298 | - | call.Origin = "btw" | |
| 326 | + | call.Origin = origin | |
| 299 | 327 | return beforeToolExecute(call) | |
| 300 | 328 | } | |
| 301 | 329 | } | |
| @@ -40,6 +40,7 @@ type btwDialog struct { | |||||
| 40 | lineInput bool | 40 | lineInput bool | ||
| 41 | cancel context.CancelFunc | 41 | cancel context.CancelFunc | ||
| 42 | generation uint64 | 42 | generation uint64 | ||
| 43 | + | closeHook func() | |||
| 43 | 44 | ||||
| 44 | // spin drives the same braille animation as the main status bar. Its | 45 | // spin drives the same braille animation as the main status bar. Its | ||
| 45 | // activity label is owned by this dialog under d.mu. | 46 | // activity label is owned by this dialog under d.mu. | ||
| @@ -68,6 +69,23 @@ func newBtwDialog() *btwDialog { | |||||
| 68 | return &btwDialog{} | 69 | return &btwDialog{} | ||
| 69 | } | 70 | } | ||
| 70 | 71 | ||||
| 72 | + | func btwOrigin(generation uint64) string { | |||
| 73 | + | return fmt.Sprintf("btw:%d", generation) | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | func isBtwOrigin(origin string) bool { | |||
| 77 | + | return strings.HasPrefix(origin, "btw:") | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | func (d *btwDialog) setCloseHook(hook func()) { | |||
| 81 | + | if d == nil { | |||
| 82 | + | return | |||
| 83 | + | } | |||
| 84 | + | d.mu.Lock() | |||
| 85 | + | d.closeHook = hook | |||
| 86 | + | d.mu.Unlock() | |||
| 87 | + | } | |||
| 88 | + | ||||
| 71 | // Active reports whether the dialog is visible and consuming keys. | 89 | // Active reports whether the dialog is visible and consuming keys. | ||
| 72 | func (d *btwDialog) Active() bool { | 90 | func (d *btwDialog) Active() bool { | ||
| 73 | if d == nil { | 91 | if d == nil { | ||
| @@ -92,19 +110,25 @@ func (d *btwDialog) Loading() bool { | |||||
| 92 | return d.active && d.loading | 110 | return d.active && d.loading | ||
| 93 | } | 111 | } | ||
| 94 | 112 | ||||
| 95 | - | // SetToolPreview attaches the side-effect-free confirmation preview to the | 113 | + | // enqueueToolConfirmation attaches the side-effect-free confirmation preview |
| 96 | - | // matching side-chat tool box before execution begins. | 114 | + | // to the matching side-chat tool box and queues its prompt while d.mu is |
| 97 | - | func (d *btwDialog) SetToolPreview(id, summary, content string) bool { | 115 | + | // held. Keeping both actions together prevents Close from advancing the |
| 98 | - | if d == nil || id == "" { | 116 | + | // generation between the ownership check and the confirmation enqueue. |
| 117 | + | func (d *btwDialog) enqueueToolConfirmation(origin, id, summary, content string, enqueue func()) bool { | |||
| 118 | + | if d == nil || id == "" || enqueue == nil { | |||
| 99 | return false | 119 | return false | ||
| 100 | } | 120 | } | ||
| 101 | d.mu.Lock() | 121 | d.mu.Lock() | ||
| 102 | defer d.mu.Unlock() | 122 | defer d.mu.Unlock() | ||
| 123 | + | if !d.active || origin != btwOrigin(d.generation) { | |||
| 124 | + | return false | |||
| 125 | + | } | |||
| 103 | for turnIdx := range d.turns { | 126 | for turnIdx := range d.turns { | ||
| 104 | if tool := findBtwTool(&d.turns[turnIdx], id); tool != nil { | 127 | if tool := findBtwTool(&d.turns[turnIdx], id); tool != nil { | ||
| 105 | tool.Args = summary | 128 | tool.Args = summary | ||
| 106 | tool.Preview = content | 129 | tool.Preview = content | ||
| 107 | d.activity.activity = activity{kind: activityAwaitingConfirmation, tool: tool.Name} | 130 | d.activity.activity = activity{kind: activityAwaitingConfirmation, tool: tool.Name} | ||
| 131 | + | enqueue() | |||
| 108 | return true | 132 | return true | ||
| 109 | } | 133 | } | ||
| 110 | } | 134 | } | ||
| @@ -146,7 +170,7 @@ func (d *btwDialog) Open(th tui.Theme, agent *core.Agent, system, model, cwd, se | |||||
| 146 | prompt = "" | 170 | prompt = "" | ||
| 147 | } | 171 | } | ||
| 148 | d.editor = tui.NewEditor(prompt) | 172 | d.editor = tui.NewEditor(prompt) | ||
| 149 | - | d.sideAgent = newBtwAgent(agent, system, model) | 173 | + | d.sideAgent = newBtwAgent(agent, system, model, btwOrigin(d.generation)) |
| 150 | d.cwd = cwd | 174 | d.cwd = cwd | ||
| 151 | d.mu.Unlock() | 175 | d.mu.Unlock() | ||
| 152 | 176 | ||||
| @@ -169,10 +193,14 @@ func (d *btwDialog) Close() { | |||||
| 169 | d.cancel = nil | 193 | d.cancel = nil | ||
| 170 | d.sideAgent = nil | 194 | d.sideAgent = nil | ||
| 171 | d.toolView = nil | 195 | d.toolView = nil | ||
| 196 | + | closeHook := d.closeHook | |||
| 172 | d.mu.Unlock() | 197 | d.mu.Unlock() | ||
| 173 | if cancel != nil { | 198 | if cancel != nil { | ||
| 174 | cancel() | 199 | cancel() | ||
| 175 | } | 200 | } | ||
| 201 | + | if closeHook != nil { | |||
| 202 | + | closeHook() | |||
| 203 | + | } | |||
| 176 | } | 204 | } | ||
| 177 | 205 | ||||
| 178 | // HandleKey routes a keypress to the dialog. Returns true if the | 206 | // HandleKey routes a keypress to the dialog. Returns true if the | ||
| @@ -286,7 +314,7 @@ func (d *btwDialog) submit(invalidate func()) { | |||||
| 286 | }() | 314 | }() | ||
| 287 | } | 315 | } | ||
| 288 | 316 | ||||
| 289 | - | func newBtwAgent(main *core.Agent, system, model string) *core.Agent { | 317 | + | func newBtwAgent(main *core.Agent, system, model, origin string) *core.Agent { |
| 290 | agent := core.NewAgent(main.Client, model, system, main.ToolsSnapshot()) | 318 | agent := core.NewAgent(main.Client, model, system, main.ToolsSnapshot()) | ||
| 291 | agent.MaxSteps = main.MaxSteps | 319 | agent.MaxSteps = main.MaxSteps | ||
| 292 | agent.Reasoning = main.Reasoning | 320 | agent.Reasoning = main.Reasoning | ||
| @@ -295,7 +323,7 @@ func newBtwAgent(main *core.Agent, system, model string) *core.Agent { | |||||
| 295 | agent.MaxTokens = main.MaxTokens | 323 | agent.MaxTokens = main.MaxTokens | ||
| 296 | if beforeToolExecute := main.BeforeToolExecute; beforeToolExecute != nil { | 324 | if beforeToolExecute := main.BeforeToolExecute; beforeToolExecute != nil { | ||
| 297 | agent.BeforeToolExecute = func(call provider.ToolCallBlock) (bool, string, json.RawMessage) { | 325 | agent.BeforeToolExecute = func(call provider.ToolCallBlock) (bool, string, json.RawMessage) { | ||
| 298 | - | call.Origin = "btw" | 326 | + | call.Origin = origin |
| 299 | return beforeToolExecute(call) | 327 | return beforeToolExecute(call) | ||
| 300 | } | 328 | } | ||
| 301 | } | 329 | } | ||
M packages/agent/modes/btw_dialog_test.go +2 -2
| @@ -157,8 +157,8 @@ func TestBtwDialogRunsToolsWithMainAgentPolicyInIsolatedTranscript(t *testing.T) | |||
| 157 | 157 | if got := policyCalls.Load(); got != 1 { | |
| 158 | 158 | t.Fatalf("main agent tool-policy calls = %d, want 1", got) | |
| 159 | 159 | } | |
| 160 | - | if policyOrigin != "btw" { | |
| 161 | - | t.Fatalf("side-chat tool policy origin = %q, want btw", policyOrigin) | |
| 160 | + | if policyOrigin != btwOrigin(1) { | |
| 161 | + | t.Fatalf("side-chat tool policy origin = %q, want %q", policyOrigin, btwOrigin(1)) | |
| 162 | 162 | } | |
| 163 | 163 | ||
| 164 | 164 | client.mu.Lock() | |
| @@ -157,8 +157,8 @@ func TestBtwDialogRunsToolsWithMainAgentPolicyInIsolatedTranscript(t *testing.T) | |||||
| 157 | if got := policyCalls.Load(); got != 1 { | 157 | if got := policyCalls.Load(); got != 1 { | ||
| 158 | t.Fatalf("main agent tool-policy calls = %d, want 1", got) | 158 | t.Fatalf("main agent tool-policy calls = %d, want 1", got) | ||
| 159 | } | 159 | } | ||
| 160 | - | if policyOrigin != "btw" { | 160 | + | if policyOrigin != btwOrigin(1) { |
| 161 | - | t.Fatalf("side-chat tool policy origin = %q, want btw", policyOrigin) | 161 | + | t.Fatalf("side-chat tool policy origin = %q, want %q", policyOrigin, btwOrigin(1)) |
| 162 | } | 162 | } | ||
| 163 | 163 | ||||
| 164 | client.mu.Lock() | 164 | client.mu.Lock() | ||
M packages/agent/modes/confirm_dialog.go +38 -0
| @@ -121,6 +121,44 @@ func (d *confirmDialog) CancelAll(reason string) { | |||
| 121 | 121 | } | |
| 122 | 122 | } | |
| 123 | 123 | ||
| 124 | + | // CancelChildConfirmations refuses side-chat confirmations without affecting | |
| 125 | + | // queued main-thread calls. Closing /btw must unblock its agent even though | |
| 126 | + | // the confirmation UI is no longer visible. | |
| 127 | + | func (d *confirmDialog) CancelChildConfirmations(reason string) { | |
| 128 | + | if d == nil { | |
| 129 | + | return | |
| 130 | + | } | |
| 131 | + | d.mu.Lock() | |
| 132 | + | pending := d.pending | |
| 133 | + | remaining := make([]*confirmRequest, 0, len(pending)) | |
| 134 | + | cancelled := make([]*confirmRequest, 0, len(pending)) | |
| 135 | + | activeCancelled := false | |
| 136 | + | for index, req := range pending { | |
| 137 | + | if req.returnToChild { | |
| 138 | + | cancelled = append(cancelled, req) | |
| 139 | + | activeCancelled = activeCancelled || index == 0 | |
| 140 | + | continue | |
| 141 | + | } | |
| 142 | + | remaining = append(remaining, req) | |
| 143 | + | } | |
| 144 | + | d.pending = remaining | |
| 145 | + | if activeCancelled { | |
| 146 | + | d.cursor = 0 | |
| 147 | + | d.focused = len(remaining) > 0 | |
| 148 | + | if d.focused { | |
| 149 | + | d.activeSince = time.Now() | |
| 150 | + | } | |
| 151 | + | } | |
| 152 | + | d.mu.Unlock() | |
| 153 | + | ||
| 154 | + | for _, req := range cancelled { | |
| 155 | + | select { | |
| 156 | + | case req.resp <- core.ConfirmDecision{Allow: false, Reason: reason}: | |
| 157 | + | default: | |
| 158 | + | } | |
| 159 | + | } | |
| 160 | + | } | |
| 161 | + | ||
| 124 | 162 | // AllowAllPending approves every pending request and drains the | |
| 125 | 163 | // queue. Used by /yolo so any confirmation dialog already on screen | |
| 126 | 164 | // resolves immediately as "yes". | |
| @@ -121,6 +121,44 @@ func (d *confirmDialog) CancelAll(reason string) { | |||||
| 121 | } | 121 | } | ||
| 122 | } | 122 | } | ||
| 123 | 123 | ||||
| 124 | + | // CancelChildConfirmations refuses side-chat confirmations without affecting | |||
| 125 | + | // queued main-thread calls. Closing /btw must unblock its agent even though | |||
| 126 | + | // the confirmation UI is no longer visible. | |||
| 127 | + | func (d *confirmDialog) CancelChildConfirmations(reason string) { | |||
| 128 | + | if d == nil { | |||
| 129 | + | return | |||
| 130 | + | } | |||
| 131 | + | d.mu.Lock() | |||
| 132 | + | pending := d.pending | |||
| 133 | + | remaining := make([]*confirmRequest, 0, len(pending)) | |||
| 134 | + | cancelled := make([]*confirmRequest, 0, len(pending)) | |||
| 135 | + | activeCancelled := false | |||
| 136 | + | for index, req := range pending { | |||
| 137 | + | if req.returnToChild { | |||
| 138 | + | cancelled = append(cancelled, req) | |||
| 139 | + | activeCancelled = activeCancelled || index == 0 | |||
| 140 | + | continue | |||
| 141 | + | } | |||
| 142 | + | remaining = append(remaining, req) | |||
| 143 | + | } | |||
| 144 | + | d.pending = remaining | |||
| 145 | + | if activeCancelled { | |||
| 146 | + | d.cursor = 0 | |||
| 147 | + | d.focused = len(remaining) > 0 | |||
| 148 | + | if d.focused { | |||
| 149 | + | d.activeSince = time.Now() | |||
| 150 | + | } | |||
| 151 | + | } | |||
| 152 | + | d.mu.Unlock() | |||
| 153 | + | ||||
| 154 | + | for _, req := range cancelled { | |||
| 155 | + | select { | |||
| 156 | + | case req.resp <- core.ConfirmDecision{Allow: false, Reason: reason}: | |||
| 157 | + | default: | |||
| 158 | + | } | |||
| 159 | + | } | |||
| 160 | + | } | |||
| 161 | + | ||||
| 124 | // AllowAllPending approves every pending request and drains the | 162 | // AllowAllPending approves every pending request and drains the | ||
| 125 | // queue. Used by /yolo so any confirmation dialog already on screen | 163 | // queue. Used by /yolo so any confirmation dialog already on screen | ||
| 126 | // resolves immediately as "yes". | 164 | // resolves immediately as "yes". | ||
M packages/agent/modes/confirm_dialog_test.go +62 -1
| @@ -39,7 +39,7 @@ func TestConfirmToolCallAttachesDiffBeforeDecision(t *testing.T) { | |||
| 39 | 39 | Name: "edit", | |
| 40 | 40 | Summary: "sample.go", | |
| 41 | 41 | Content: "-old\n+new\n", | |
| 42 | - | Origin: "btw", | |
| 42 | + | Origin: btwOrigin(0), | |
| 43 | 43 | }) | |
| 44 | 44 | }() | |
| 45 | 45 | ||
| @@ -80,6 +80,67 @@ func TestConfirmToolCallAttachesDiffBeforeDecision(t *testing.T) { | |||
| 80 | 80 | } | |
| 81 | 81 | } | |
| 82 | 82 | ||
| 83 | + | func TestConfirmToolCallRejectsClosedSideChatGeneration(t *testing.T) { | |
| 84 | + | i := newConfirmationInputInteractive() | |
| 85 | + | i.btwDialog.Open(tui.Dark, i.agent, i.agent.System, i.agent.Model, t.TempDir(), "", false, false, true, nil) | |
| 86 | + | i.btwDialog.mu.Lock() | |
| 87 | + | origin := btwOrigin(i.btwDialog.generation) | |
| 88 | + | i.btwDialog.mu.Unlock() | |
| 89 | + | i.btwDialog.Close() | |
| 90 | + | ||
| 91 | + | decision := i.ConfirmToolCall(core.ToolCallConfirmation{ | |
| 92 | + | ID: "call-1", | |
| 93 | + | Name: "edit", | |
| 94 | + | Origin: origin, | |
| 95 | + | }) | |
| 96 | + | if decision.Allow || decision.Reason != "side-chat tool call canceled" { | |
| 97 | + | t.Fatalf("stale side-chat decision = %+v", decision) | |
| 98 | + | } | |
| 99 | + | if i.confirmDialog.Active() { | |
| 100 | + | t.Fatal("stale side-chat confirmation activated the main dialog") | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | ||
| 104 | + | func TestBtwCloseCancelsSideChatConfirmation(t *testing.T) { | |
| 105 | + | i := newConfirmationInputInteractive() | |
| 106 | + | d := i.btwDialog | |
| 107 | + | d.mu.Lock() | |
| 108 | + | d.active = true | |
| 109 | + | d.generation = 1 | |
| 110 | + | d.turns = []btwTurn{{Tools: []tui.ToolCallView{{ID: "call-1", Name: "edit"}}}} | |
| 111 | + | d.mu.Unlock() | |
| 112 | + | ||
| 113 | + | decision := make(chan core.ConfirmDecision, 1) | |
| 114 | + | go func() { | |
| 115 | + | decision <- i.ConfirmToolCall(core.ToolCallConfirmation{ | |
| 116 | + | ID: "call-1", | |
| 117 | + | Name: "edit", | |
| 118 | + | Origin: btwOrigin(1), | |
| 119 | + | }) | |
| 120 | + | }() | |
| 121 | + | ||
| 122 | + | deadline := time.Now().Add(time.Second) | |
| 123 | + | for !i.confirmDialog.Active() && time.Now().Before(deadline) { | |
| 124 | + | runtime.Gosched() | |
| 125 | + | } | |
| 126 | + | if !i.confirmDialog.Active() { | |
| 127 | + | t.Fatal("side-chat confirmation did not enter the queue") | |
| 128 | + | } | |
| 129 | + | d.Close() | |
| 130 | + | ||
| 131 | + | select { | |
| 132 | + | case got := <-decision: | |
| 133 | + | if got.Allow || got.Reason != "side chat closed" { | |
| 134 | + | t.Fatalf("close decision = %+v", got) | |
| 135 | + | } | |
| 136 | + | case <-time.After(time.Second): | |
| 137 | + | t.Fatal("closing /btw did not unblock its confirmation") | |
| 138 | + | } | |
| 139 | + | if i.confirmDialog.Active() { | |
| 140 | + | t.Fatal("side-chat confirmation remained active after /btw closed") | |
| 141 | + | } | |
| 142 | + | } | |
| 143 | + | ||
| 83 | 144 | func newConfirmationInputInteractive() *Interactive { | |
| 84 | 145 | agent := core.NewAgent(nil, "test-model", "test system", nil) | |
| 85 | 146 | return NewInteractive(InteractiveConfig{ | |
| @@ -39,7 +39,7 @@ func TestConfirmToolCallAttachesDiffBeforeDecision(t *testing.T) { | |||||
| 39 | Name: "edit", | 39 | Name: "edit", | ||
| 40 | Summary: "sample.go", | 40 | Summary: "sample.go", | ||
| 41 | Content: "-old\n+new\n", | 41 | Content: "-old\n+new\n", | ||
| 42 | - | Origin: "btw", | 42 | + | Origin: btwOrigin(0), |
| 43 | }) | 43 | }) | ||
| 44 | }() | 44 | }() | ||
| 45 | 45 | ||||
| @@ -80,6 +80,67 @@ func TestConfirmToolCallAttachesDiffBeforeDecision(t *testing.T) { | |||||
| 80 | } | 80 | } | ||
| 81 | } | 81 | } | ||
| 82 | 82 | ||||
| 83 | + | func TestConfirmToolCallRejectsClosedSideChatGeneration(t *testing.T) { | |||
| 84 | + | i := newConfirmationInputInteractive() | |||
| 85 | + | i.btwDialog.Open(tui.Dark, i.agent, i.agent.System, i.agent.Model, t.TempDir(), "", false, false, true, nil) | |||
| 86 | + | i.btwDialog.mu.Lock() | |||
| 87 | + | origin := btwOrigin(i.btwDialog.generation) | |||
| 88 | + | i.btwDialog.mu.Unlock() | |||
| 89 | + | i.btwDialog.Close() | |||
| 90 | + | ||||
| 91 | + | decision := i.ConfirmToolCall(core.ToolCallConfirmation{ | |||
| 92 | + | ID: "call-1", | |||
| 93 | + | Name: "edit", | |||
| 94 | + | Origin: origin, | |||
| 95 | + | }) | |||
| 96 | + | if decision.Allow || decision.Reason != "side-chat tool call canceled" { | |||
| 97 | + | t.Fatalf("stale side-chat decision = %+v", decision) | |||
| 98 | + | } | |||
| 99 | + | if i.confirmDialog.Active() { | |||
| 100 | + | t.Fatal("stale side-chat confirmation activated the main dialog") | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | ||||
| 104 | + | func TestBtwCloseCancelsSideChatConfirmation(t *testing.T) { | |||
| 105 | + | i := newConfirmationInputInteractive() | |||
| 106 | + | d := i.btwDialog | |||
| 107 | + | d.mu.Lock() | |||
| 108 | + | d.active = true | |||
| 109 | + | d.generation = 1 | |||
| 110 | + | d.turns = []btwTurn{{Tools: []tui.ToolCallView{{ID: "call-1", Name: "edit"}}}} | |||
| 111 | + | d.mu.Unlock() | |||
| 112 | + | ||||
| 113 | + | decision := make(chan core.ConfirmDecision, 1) | |||
| 114 | + | go func() { | |||
| 115 | + | decision <- i.ConfirmToolCall(core.ToolCallConfirmation{ | |||
| 116 | + | ID: "call-1", | |||
| 117 | + | Name: "edit", | |||
| 118 | + | Origin: btwOrigin(1), | |||
| 119 | + | }) | |||
| 120 | + | }() | |||
| 121 | + | ||||
| 122 | + | deadline := time.Now().Add(time.Second) | |||
| 123 | + | for !i.confirmDialog.Active() && time.Now().Before(deadline) { | |||
| 124 | + | runtime.Gosched() | |||
| 125 | + | } | |||
| 126 | + | if !i.confirmDialog.Active() { | |||
| 127 | + | t.Fatal("side-chat confirmation did not enter the queue") | |||
| 128 | + | } | |||
| 129 | + | d.Close() | |||
| 130 | + | ||||
| 131 | + | select { | |||
| 132 | + | case got := <-decision: | |||
| 133 | + | if got.Allow || got.Reason != "side chat closed" { | |||
| 134 | + | t.Fatalf("close decision = %+v", got) | |||
| 135 | + | } | |||
| 136 | + | case <-time.After(time.Second): | |||
| 137 | + | t.Fatal("closing /btw did not unblock its confirmation") | |||
| 138 | + | } | |||
| 139 | + | if i.confirmDialog.Active() { | |||
| 140 | + | t.Fatal("side-chat confirmation remained active after /btw closed") | |||
| 141 | + | } | |||
| 142 | + | } | |||
| 143 | + | ||||
| 83 | func newConfirmationInputInteractive() *Interactive { | 144 | func newConfirmationInputInteractive() *Interactive { | ||
| 84 | agent := core.NewAgent(nil, "test-model", "test system", nil) | 145 | agent := core.NewAgent(nil, "test-model", "test system", nil) | ||
| 85 | return NewInteractive(InteractiveConfig{ | 146 | return NewInteractive(InteractiveConfig{ | ||
M packages/agent/modes/interactive.go +21 -9
| @@ -808,6 +808,9 @@ func NewInteractive(cfg InteractiveConfig) *Interactive { | |||
| 808 | 808 | clock: time.Now, | |
| 809 | 809 | reloadErrors: append([]string(nil), cfg.StartupExtensionErrors...), | |
| 810 | 810 | } | |
| 811 | + | i.btwDialog.setCloseHook(func() { | |
| 812 | + | i.confirmDialog.CancelChildConfirmations("side chat closed") | |
| 813 | + | }) | |
| 811 | 814 | i.fileSuggest.SetRecursive(cfg.RecursiveFileSuggest != nil && *cfg.RecursiveFileSuggest) | |
| 812 | 815 | i.fileSuggest.SetRespectGitignore(cfg.RespectGitignore == nil || *cfg.RespectGitignore) | |
| 813 | 816 | if cfg.LlamaCPPConfig != nil { | |
| @@ -7385,12 +7388,22 @@ func (i *Interactive) Confirm(toolName string, preview string) core.ConfirmDecis | |||
| 7385 | 7388 | // tool panel, then blocks until the user approves or refuses the call. | |
| 7386 | 7389 | func (i *Interactive) ConfirmToolCall(call core.ToolCallConfirmation) core.ConfirmDecision { | |
| 7387 | 7390 | resp := make(chan core.ConfirmDecision, 1) | |
| 7388 | - | returnToChild := false | |
| 7389 | - | if call.ID != "" && call.Origin == "btw" { | |
| 7390 | - | if i.btwDialog != nil && i.btwDialog.Active() { | |
| 7391 | - | returnToChild = i.btwDialog.SetToolPreview(call.ID, call.Summary, call.Content) | |
| 7391 | + | if isBtwOrigin(call.Origin) { | |
| 7392 | + | req := &confirmRequest{ | |
| 7393 | + | toolName: call.Name, | |
| 7394 | + | preview: call.Summary, | |
| 7395 | + | resp: resp, | |
| 7396 | + | returnToChild: true, | |
| 7397 | + | } | |
| 7398 | + | if call.ID == "" || i.btwDialog == nil || !i.btwDialog.enqueueToolConfirmation(call.Origin, call.ID, call.Summary, call.Content, func() { | |
| 7399 | + | i.confirmDialog.Enqueue(req) | |
| 7400 | + | }) { | |
| 7401 | + | return core.ConfirmDecision{Allow: false, Reason: "side-chat tool call canceled"} | |
| 7392 | 7402 | } | |
| 7393 | - | } else if call.ID != "" { | |
| 7403 | + | i.invalidate() | |
| 7404 | + | return <-resp | |
| 7405 | + | } | |
| 7406 | + | if call.ID != "" { | |
| 7394 | 7407 | i.mu.Lock() | |
| 7395 | 7408 | if tc, ok := i.toolCalls[call.ID]; ok { | |
| 7396 | 7409 | tc.Args = call.Summary | |
| @@ -7400,10 +7413,9 @@ func (i *Interactive) ConfirmToolCall(call core.ToolCallConfirmation) core.Confi | |||
| 7400 | 7413 | i.mu.Unlock() | |
| 7401 | 7414 | } | |
| 7402 | 7415 | i.confirmDialog.Enqueue(&confirmRequest{ | |
| 7403 | - | toolName: call.Name, | |
| 7404 | - | preview: call.Summary, | |
| 7405 | - | resp: resp, | |
| 7406 | - | returnToChild: returnToChild, | |
| 7416 | + | toolName: call.Name, | |
| 7417 | + | preview: call.Summary, | |
| 7418 | + | resp: resp, | |
| 7407 | 7419 | }) | |
| 7408 | 7420 | i.invalidate() | |
| 7409 | 7421 | return <-resp | |
| @@ -808,6 +808,9 @@ func NewInteractive(cfg InteractiveConfig) *Interactive { | |||||
| 808 | clock: time.Now, | 808 | clock: time.Now, | ||
| 809 | reloadErrors: append([]string(nil), cfg.StartupExtensionErrors...), | 809 | reloadErrors: append([]string(nil), cfg.StartupExtensionErrors...), | ||
| 810 | } | 810 | } | ||
| 811 | + | i.btwDialog.setCloseHook(func() { | |||
| 812 | + | i.confirmDialog.CancelChildConfirmations("side chat closed") | |||
| 813 | + | }) | |||
| 811 | i.fileSuggest.SetRecursive(cfg.RecursiveFileSuggest != nil && *cfg.RecursiveFileSuggest) | 814 | i.fileSuggest.SetRecursive(cfg.RecursiveFileSuggest != nil && *cfg.RecursiveFileSuggest) | ||
| 812 | i.fileSuggest.SetRespectGitignore(cfg.RespectGitignore == nil || *cfg.RespectGitignore) | 815 | i.fileSuggest.SetRespectGitignore(cfg.RespectGitignore == nil || *cfg.RespectGitignore) | ||
| 813 | if cfg.LlamaCPPConfig != nil { | 816 | if cfg.LlamaCPPConfig != nil { | ||
| @@ -7385,12 +7388,22 @@ func (i *Interactive) Confirm(toolName string, preview string) core.ConfirmDecis | |||||
| 7385 | // tool panel, then blocks until the user approves or refuses the call. | 7388 | // tool panel, then blocks until the user approves or refuses the call. | ||
| 7386 | func (i *Interactive) ConfirmToolCall(call core.ToolCallConfirmation) core.ConfirmDecision { | 7389 | func (i *Interactive) ConfirmToolCall(call core.ToolCallConfirmation) core.ConfirmDecision { | ||
| 7387 | resp := make(chan core.ConfirmDecision, 1) | 7390 | resp := make(chan core.ConfirmDecision, 1) | ||
| 7388 | - | returnToChild := false | 7391 | + | if isBtwOrigin(call.Origin) { |
| 7389 | - | if call.ID != "" && call.Origin == "btw" { | 7392 | + | req := &confirmRequest{ |
| 7390 | - | if i.btwDialog != nil && i.btwDialog.Active() { | 7393 | + | toolName: call.Name, |
| 7391 | - | returnToChild = i.btwDialog.SetToolPreview(call.ID, call.Summary, call.Content) | 7394 | + | preview: call.Summary, |
| 7395 | + | resp: resp, | |||
| 7396 | + | returnToChild: true, | |||
| 7397 | + | } | |||
| 7398 | + | if call.ID == "" || i.btwDialog == nil || !i.btwDialog.enqueueToolConfirmation(call.Origin, call.ID, call.Summary, call.Content, func() { | |||
| 7399 | + | i.confirmDialog.Enqueue(req) | |||
| 7400 | + | }) { | |||
| 7401 | + | return core.ConfirmDecision{Allow: false, Reason: "side-chat tool call canceled"} | |||
| 7392 | } | 7402 | } | ||
| 7393 | - | } else if call.ID != "" { | 7403 | + | i.invalidate() |
| 7404 | + | return <-resp | |||
| 7405 | + | } | |||
| 7406 | + | if call.ID != "" { | |||
| 7394 | i.mu.Lock() | 7407 | i.mu.Lock() | ||
| 7395 | if tc, ok := i.toolCalls[call.ID]; ok { | 7408 | if tc, ok := i.toolCalls[call.ID]; ok { | ||
| 7396 | tc.Args = call.Summary | 7409 | tc.Args = call.Summary | ||
| @@ -7400,10 +7413,9 @@ func (i *Interactive) ConfirmToolCall(call core.ToolCallConfirmation) core.Confi | |||||
| 7400 | i.mu.Unlock() | 7413 | i.mu.Unlock() | ||
| 7401 | } | 7414 | } | ||
| 7402 | i.confirmDialog.Enqueue(&confirmRequest{ | 7415 | i.confirmDialog.Enqueue(&confirmRequest{ | ||
| 7403 | - | toolName: call.Name, | 7416 | + | toolName: call.Name, |
| 7404 | - | preview: call.Summary, | 7417 | + | preview: call.Summary, |
| 7405 | - | resp: resp, | 7418 | + | resp: resp, |
| 7406 | - | returnToChild: returnToChild, | |||
| 7407 | }) | 7419 | }) | ||
| 7408 | i.invalidate() | 7420 | i.invalidate() | ||
| 7409 | return <-resp | 7421 | return <-resp | ||