|
1 | 1 | package copilot |
2 | 2 |
|
3 | 3 | import ( |
| 4 | + "bufio" |
| 5 | + "context" |
| 6 | + "encoding/json" |
| 7 | + "fmt" |
| 8 | + "io" |
| 9 | + "strings" |
4 | 10 | "sync" |
5 | 11 | "sync/atomic" |
6 | 12 | "testing" |
7 | 13 | "time" |
| 14 | + |
| 15 | + "github.com/github/copilot-sdk/go/internal/jsonrpc2" |
8 | 16 | ) |
9 | 17 |
|
10 | 18 | // newTestSession creates a session with an event channel and starts the consumer goroutine. |
@@ -204,3 +212,252 @@ func TestSession_On(t *testing.T) { |
204 | 212 | } |
205 | 213 | }) |
206 | 214 | } |
| 215 | + |
| 216 | +// newTestSessionWithFakeClient creates a Session backed by an in-process fake |
| 217 | +// JSON-RPC 2.0 server that immediately acknowledges any "session.send" call. |
| 218 | +// The returned requestReceived channel is signalled once per incoming request, |
| 219 | +// so tests can synchronize dispatch of follow-up events. |
| 220 | +// The cleanup function must be deferred by the caller. |
| 221 | +func newTestSessionWithFakeClient(t *testing.T) (sess *Session, requestReceived <-chan struct{}, cleanup func()) { |
| 222 | + t.Helper() |
| 223 | + |
| 224 | + stdinR, stdinW := io.Pipe() |
| 225 | + stdoutR, stdoutW := io.Pipe() |
| 226 | + |
| 227 | + client := jsonrpc2.NewClient(stdinW, stdoutR) |
| 228 | + client.Start() |
| 229 | + |
| 230 | + reqCh := make(chan struct{}, 4) |
| 231 | + |
| 232 | + go func() { |
| 233 | + reader := bufio.NewReader(stdinR) |
| 234 | + buf := make([]byte, 8192) |
| 235 | + for { |
| 236 | + var contentLength int |
| 237 | + for { |
| 238 | + line, err := reader.ReadString('\n') |
| 239 | + if err != nil { |
| 240 | + return |
| 241 | + } |
| 242 | + line = strings.TrimSpace(line) |
| 243 | + if line == "" { |
| 244 | + break |
| 245 | + } |
| 246 | + fmt.Sscanf(line, "Content-Length: %d", &contentLength) |
| 247 | + } |
| 248 | + if contentLength == 0 { |
| 249 | + continue |
| 250 | + } |
| 251 | + if contentLength > len(buf) { |
| 252 | + buf = make([]byte, contentLength) |
| 253 | + } |
| 254 | + if _, err := io.ReadFull(reader, buf[:contentLength]); err != nil { |
| 255 | + return |
| 256 | + } |
| 257 | + var req struct { |
| 258 | + ID json.RawMessage `json:"id"` |
| 259 | + } |
| 260 | + json.Unmarshal(buf[:contentLength], &req) //nolint:errcheck |
| 261 | + |
| 262 | + // Signal that a request was received before sending the response, |
| 263 | + // so tests can dispatch events once Send() is in-flight. |
| 264 | + select { |
| 265 | + case reqCh <- struct{}{}: |
| 266 | + default: |
| 267 | + } |
| 268 | + |
| 269 | + result := `{"messageId":"test-msg"}` |
| 270 | + body := fmt.Sprintf(`{"jsonrpc":"2.0","id":%s,"result":%s}`, req.ID, result) |
| 271 | + header := fmt.Sprintf("Content-Length: %d\r\n\r\n", len(body)) |
| 272 | + stdoutW.Write([]byte(header + body)) //nolint:errcheck |
| 273 | + } |
| 274 | + }() |
| 275 | + |
| 276 | + s := &Session{ |
| 277 | + SessionID: "test-session", |
| 278 | + handlers: make([]sessionHandler, 0), |
| 279 | + eventCh: make(chan SessionEvent, 128), |
| 280 | + client: client, |
| 281 | + } |
| 282 | + go s.processEvents() |
| 283 | + |
| 284 | + cleanupFn := func() { |
| 285 | + stdoutW.Close() |
| 286 | + stdinW.Close() |
| 287 | + client.Stop() |
| 288 | + close(s.eventCh) |
| 289 | + } |
| 290 | + |
| 291 | + return s, reqCh, cleanupFn |
| 292 | +} |
| 293 | + |
| 294 | +// TestSendAndWait_BackgroundTasks verifies the fix for PolyPilot#299: |
| 295 | +// SendAndWait must NOT resolve when session.idle carries active background tasks. |
| 296 | +func TestSendAndWait_BackgroundTasks(t *testing.T) { |
| 297 | + t.Run("does not resolve when session.idle has active background agents", func(t *testing.T) { |
| 298 | + session, requestReceived, cleanup := newTestSessionWithFakeClient(t) |
| 299 | + defer cleanup() |
| 300 | + |
| 301 | + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| 302 | + defer cancel() |
| 303 | + |
| 304 | + var resolved atomic.Bool |
| 305 | + done := make(chan struct{}) |
| 306 | + go func() { |
| 307 | + defer close(done) |
| 308 | + session.SendAndWait(ctx, MessageOptions{Prompt: "test"}) //nolint:errcheck |
| 309 | + resolved.Store(true) |
| 310 | + }() |
| 311 | + |
| 312 | + // Wait for the fake server to receive the session.send request. |
| 313 | + select { |
| 314 | + case <-requestReceived: |
| 315 | + case <-time.After(2 * time.Second): |
| 316 | + t.Fatal("timeout waiting for session.send request") |
| 317 | + } |
| 318 | + |
| 319 | + // Dispatch session.idle WITH active background agents — must NOT resolve. |
| 320 | + session.dispatchEvent(SessionEvent{ |
| 321 | + Type: SessionEventTypeSessionIdle, |
| 322 | + Data: Data{ |
| 323 | + BackgroundTasks: &BackgroundTasks{ |
| 324 | + Agents: []BackgroundTasksAgent{{AgentID: "bg-1", AgentType: "worker"}}, |
| 325 | + Shells: []Shell{}, |
| 326 | + }, |
| 327 | + }, |
| 328 | + }) |
| 329 | + |
| 330 | + time.Sleep(100 * time.Millisecond) |
| 331 | + if resolved.Load() { |
| 332 | + t.Error("BUG #299: SendAndWait resolved prematurely while background agents were active") |
| 333 | + } |
| 334 | + |
| 335 | + // Dispatch a clean idle (no background tasks) — now it SHOULD resolve. |
| 336 | + session.dispatchEvent(SessionEvent{ |
| 337 | + Type: SessionEventTypeSessionIdle, |
| 338 | + Data: Data{}, |
| 339 | + }) |
| 340 | + |
| 341 | + select { |
| 342 | + case <-done: |
| 343 | + case <-time.After(2 * time.Second): |
| 344 | + t.Fatal("SendAndWait did not resolve after clean session.idle") |
| 345 | + } |
| 346 | + if !resolved.Load() { |
| 347 | + t.Error("expected SendAndWait to resolve after clean session.idle") |
| 348 | + } |
| 349 | + }) |
| 350 | + |
| 351 | + t.Run("does not resolve when session.idle has active background shells", func(t *testing.T) { |
| 352 | + session, requestReceived, cleanup := newTestSessionWithFakeClient(t) |
| 353 | + defer cleanup() |
| 354 | + |
| 355 | + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| 356 | + defer cancel() |
| 357 | + |
| 358 | + var resolved atomic.Bool |
| 359 | + done := make(chan struct{}) |
| 360 | + go func() { |
| 361 | + defer close(done) |
| 362 | + session.SendAndWait(ctx, MessageOptions{Prompt: "test"}) //nolint:errcheck |
| 363 | + resolved.Store(true) |
| 364 | + }() |
| 365 | + |
| 366 | + select { |
| 367 | + case <-requestReceived: |
| 368 | + case <-time.After(2 * time.Second): |
| 369 | + t.Fatal("timeout waiting for session.send request") |
| 370 | + } |
| 371 | + |
| 372 | + // Dispatch session.idle WITH active background shell — must NOT resolve. |
| 373 | + session.dispatchEvent(SessionEvent{ |
| 374 | + Type: SessionEventTypeSessionIdle, |
| 375 | + Data: Data{ |
| 376 | + BackgroundTasks: &BackgroundTasks{ |
| 377 | + Agents: []BackgroundTasksAgent{}, |
| 378 | + Shells: []Shell{{ShellID: "sh-1"}}, |
| 379 | + }, |
| 380 | + }, |
| 381 | + }) |
| 382 | + |
| 383 | + time.Sleep(100 * time.Millisecond) |
| 384 | + if resolved.Load() { |
| 385 | + t.Error("BUG #299: SendAndWait resolved prematurely while background shells were active") |
| 386 | + } |
| 387 | + |
| 388 | + // Clean idle — should now resolve. |
| 389 | + session.dispatchEvent(SessionEvent{ |
| 390 | + Type: SessionEventTypeSessionIdle, |
| 391 | + Data: Data{BackgroundTasks: &BackgroundTasks{Agents: []BackgroundTasksAgent{}, Shells: []Shell{}}}, |
| 392 | + }) |
| 393 | + |
| 394 | + select { |
| 395 | + case <-done: |
| 396 | + case <-time.After(2 * time.Second): |
| 397 | + t.Fatal("SendAndWait did not resolve after clean session.idle") |
| 398 | + } |
| 399 | + }) |
| 400 | + |
| 401 | + t.Run("resolves when session.idle has no backgroundTasks", func(t *testing.T) { |
| 402 | + session, requestReceived, cleanup := newTestSessionWithFakeClient(t) |
| 403 | + defer cleanup() |
| 404 | + |
| 405 | + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| 406 | + defer cancel() |
| 407 | + |
| 408 | + done := make(chan struct{}) |
| 409 | + go func() { |
| 410 | + defer close(done) |
| 411 | + session.SendAndWait(ctx, MessageOptions{Prompt: "test"}) //nolint:errcheck |
| 412 | + }() |
| 413 | + |
| 414 | + select { |
| 415 | + case <-requestReceived: |
| 416 | + case <-time.After(2 * time.Second): |
| 417 | + t.Fatal("timeout waiting for session.send request") |
| 418 | + } |
| 419 | + |
| 420 | + session.dispatchEvent(SessionEvent{ |
| 421 | + Type: SessionEventTypeSessionIdle, |
| 422 | + Data: Data{}, |
| 423 | + }) |
| 424 | + |
| 425 | + select { |
| 426 | + case <-done: |
| 427 | + case <-time.After(2 * time.Second): |
| 428 | + t.Fatal("SendAndWait did not resolve when session.idle had no backgroundTasks") |
| 429 | + } |
| 430 | + }) |
| 431 | + |
| 432 | + t.Run("resolves when session.idle has empty backgroundTasks arrays", func(t *testing.T) { |
| 433 | + session, requestReceived, cleanup := newTestSessionWithFakeClient(t) |
| 434 | + defer cleanup() |
| 435 | + |
| 436 | + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| 437 | + defer cancel() |
| 438 | + |
| 439 | + done := make(chan struct{}) |
| 440 | + go func() { |
| 441 | + defer close(done) |
| 442 | + session.SendAndWait(ctx, MessageOptions{Prompt: "test"}) //nolint:errcheck |
| 443 | + }() |
| 444 | + |
| 445 | + select { |
| 446 | + case <-requestReceived: |
| 447 | + case <-time.After(2 * time.Second): |
| 448 | + t.Fatal("timeout waiting for session.send request") |
| 449 | + } |
| 450 | + |
| 451 | + session.dispatchEvent(SessionEvent{ |
| 452 | + Type: SessionEventTypeSessionIdle, |
| 453 | + Data: Data{BackgroundTasks: &BackgroundTasks{Agents: []BackgroundTasksAgent{}, Shells: []Shell{}}}, |
| 454 | + }) |
| 455 | + |
| 456 | + select { |
| 457 | + case <-done: |
| 458 | + case <-time.After(2 * time.Second): |
| 459 | + t.Fatal("SendAndWait did not resolve when backgroundTasks arrays were empty") |
| 460 | + } |
| 461 | + }) |
| 462 | +} |
| 463 | + |
0 commit comments