fix(modes): fall back when runCtx is nil in SubmitFollowUp (#231)
* fix(modes): fall back when runCtx is nil in SubmitFollowUp * fix(modes): pin nil-runCtx fallback with deterministic test
- commit
- cb619f761e13dcb1c269f9e8fb9d181c474b1460
- author
- bnema · · 2026-09-27 04:04 UTC
- committer
- GitHub · · 2026-09-27 04:04 UTC
- parent
- 88f9527
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown - refs
- tag: v0.45.0
2 files · +34 -1
M packages/agent/modes/interactive_host.go +9 -1
| @@ -260,7 +260,15 @@ func (i *Interactive) SubmitFollowUp(ctx context.Context, text string) error { | |||
| 260 | 260 | } | |
| 261 | 261 | } | |
| 262 | 262 | i.mu.Unlock() | |
| 263 | - | i.startTurn(i.runCtx, text) | |
| 263 | + | // The active turn can finish before the busy check above, landing | |
| 264 | + | // here instead of the scheduled path. runCtx is only set by Run, so | |
| 265 | + | // fall back like the turn-completion handler does rather than | |
| 266 | + | // panicking in context.WithCancel. | |
| 267 | + | parent := i.runCtx | |
| 268 | + | if parent == nil { | |
| 269 | + | parent = context.Background() | |
| 270 | + | } | |
| 271 | + | i.startTurn(parent, text) | |
| 264 | 272 | return nil | |
| 265 | 273 | } | |
| 266 | 274 | func (i *Interactive) submitOrQueue(text string, images []provider.ImageBlock, userInput bool) { | |
| @@ -260,7 +260,15 @@ func (i *Interactive) SubmitFollowUp(ctx context.Context, text string) error { | |||||
| 260 | } | 260 | } | ||
| 261 | } | 261 | } | ||
| 262 | i.mu.Unlock() | 262 | i.mu.Unlock() | ||
| 263 | - | i.startTurn(i.runCtx, text) | 263 | + | // The active turn can finish before the busy check above, landing |
| 264 | + | // here instead of the scheduled path. runCtx is only set by Run, so | |||
| 265 | + | // fall back like the turn-completion handler does rather than | |||
| 266 | + | // panicking in context.WithCancel. | |||
| 267 | + | parent := i.runCtx | |||
| 268 | + | if parent == nil { | |||
| 269 | + | parent = context.Background() | |||
| 270 | + | } | |||
| 271 | + | i.startTurn(parent, text) | |||
| 264 | return nil | 272 | return nil | ||
| 265 | } | 273 | } | ||
| 266 | func (i *Interactive) submitOrQueue(text string, images []provider.ImageBlock, userInput bool) { | 274 | func (i *Interactive) submitOrQueue(text string, images []provider.ImageBlock, userInput bool) { | ||
M packages/agent/modes/repetition_guard_test.go +25 -0
| @@ -190,3 +190,28 @@ func TestInteractiveRepetitionGuardResolvesScheduledFollowUp(t *testing.T) { | |||
| 190 | 190 | t.Fatalf("provider calls = %d, want 8", got) | |
| 191 | 191 | } | |
| 192 | 192 | } | |
| 193 | + | ||
| 194 | + | // SubmitFollowUp on an idle interactive without a Run context must start a | |
| 195 | + | // turn instead of panicking in context.WithCancel(nil parent). This pins the | |
| 196 | + | // fallback deterministically; the racy busy-to-idle interleaving behind the | |
| 197 | + | // Windows CI panic needs no timing to cover the nil-parent path itself. | |
| 198 | + | func TestSubmitFollowUpWithoutRunContextStartsTurn(t *testing.T) { | |
| 199 | + | client := &repetitiveInteractiveClient{requests: make(chan repetitiveInteractiveRequest, 16)} | |
| 200 | + | agent := core.NewAgent(client, "model", "system", core.Registry{"repeat": &repetitiveInteractiveTool{}}) | |
| 201 | + | interactive := NewInteractive(InteractiveConfig{Agent: agent}) | |
| 202 | + | if interactive.runCtx != nil { | |
| 203 | + | t.Fatalf("runCtx = %#v, want nil without Run", interactive.runCtx) | |
| 204 | + | } | |
| 205 | + | if err := interactive.SubmitFollowUp(context.Background(), "scheduled task"); err != nil { | |
| 206 | + | t.Fatalf("SubmitFollowUp = %v, want nil", err) | |
| 207 | + | } | |
| 208 | + | select { | |
| 209 | + | case request := <-client.requests: | |
| 210 | + | if request.call != 1 { | |
| 211 | + | t.Fatalf("provider request = %#v, want first call", request) | |
| 212 | + | } | |
| 213 | + | case <-time.After(3 * time.Second): | |
| 214 | + | t.Fatal("provider request was not observed") | |
| 215 | + | } | |
| 216 | + | waitInteractiveIdle(t, interactive) | |
| 217 | + | } | |
| @@ -190,3 +190,28 @@ func TestInteractiveRepetitionGuardResolvesScheduledFollowUp(t *testing.T) { | |||||
| 190 | t.Fatalf("provider calls = %d, want 8", got) | 190 | t.Fatalf("provider calls = %d, want 8", got) | ||
| 191 | } | 191 | } | ||
| 192 | } | 192 | } | ||
| 193 | + | ||||
| 194 | + | // SubmitFollowUp on an idle interactive without a Run context must start a | |||
| 195 | + | // turn instead of panicking in context.WithCancel(nil parent). This pins the | |||
| 196 | + | // fallback deterministically; the racy busy-to-idle interleaving behind the | |||
| 197 | + | // Windows CI panic needs no timing to cover the nil-parent path itself. | |||
| 198 | + | func TestSubmitFollowUpWithoutRunContextStartsTurn(t *testing.T) { | |||
| 199 | + | client := &repetitiveInteractiveClient{requests: make(chan repetitiveInteractiveRequest, 16)} | |||
| 200 | + | agent := core.NewAgent(client, "model", "system", core.Registry{"repeat": &repetitiveInteractiveTool{}}) | |||
| 201 | + | interactive := NewInteractive(InteractiveConfig{Agent: agent}) | |||
| 202 | + | if interactive.runCtx != nil { | |||
| 203 | + | t.Fatalf("runCtx = %#v, want nil without Run", interactive.runCtx) | |||
| 204 | + | } | |||
| 205 | + | if err := interactive.SubmitFollowUp(context.Background(), "scheduled task"); err != nil { | |||
| 206 | + | t.Fatalf("SubmitFollowUp = %v, want nil", err) | |||
| 207 | + | } | |||
| 208 | + | select { | |||
| 209 | + | case request := <-client.requests: | |||
| 210 | + | if request.call != 1 { | |||
| 211 | + | t.Fatalf("provider request = %#v, want first call", request) | |||
| 212 | + | } | |||
| 213 | + | case <-time.After(3 * time.Second): | |||
| 214 | + | t.Fatal("provider request was not observed") | |||
| 215 | + | } | |||
| 216 | + | waitInteractiveIdle(t, interactive) | |||
| 217 | + | } | |||