github / github/copilot-sdk

resumeSession() on already-active session causes doubled events — SDK should guard

Đang mở
#742 0 bình luận 2 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Java
Star
10.5k
Fork
1.5k
Merge trung bình
1 ngày 11 giờ
Pull request đã merge (30 ngày)
127

Mô tả

## Problem

Calling `client.resumeSession(sessionId)` on a session that's already active (created via `createSession()` on the same connection) causes all subsequent `session.event` notifications to fire **twice**. This is a server-side issue (the CLI registers a second event subscription without deduplicating), but the SDK can and should protect callers.

## Reproduction

```javascript
const { CopilotClient, approveAll } = await import('@github/copilot-sdk');
const client = new CopilotClient({ cwd: process.cwd(), autoStart: true });

const session = await client.createSession({
model: 'claude-sonnet-4-5',
onPermissionRequest: approveAll,
});

// Events are single here ✅
let count1 = 0;
const unsub1 = session.on(() => count1++);
await session.sendAndWait({ prompt: 'Say hello' });
unsub1();

// Resume the SAME active session
const resumed = await client.resumeSession(session.sessionId, {
onPermissionRequest: approveAll,
});

// Events are now doubled ❌
let count2 = 0;
const unsub2 = resumed.on(() => count2++);
await resumed.sendAndWait({ prompt: 'Say hello again' });
unsub2();

console.log(count1, count2); // ~9, ~17
```

## Suggested Fix

In `client.ts` `resumeSession()`, check if the session already exists in the local `sessions` Map before sending `session.resume` to the server:

```typescript
async resumeSession(sessionId: string, config: ResumeSessionConfig): Promise {
// Guard: if we already have a live session for this ID, return it
const existing = this.sessions.get(sessionId);
if (existing) {
// Re-register handlers if config changed
existing.registerTools(config.tools);
existing.registerPermissionHandler(config.onPermissionRequest);
if (config.hooks) existing.registerHooks(config.hooks);
return existing;
}

// ... existing resumeSession logic for truly new sessions
}
```

This prevents the server-side duplicate subscription while still allowing callers to update tools/permissions. Callers who genuinely want a fresh session can `destroy()` first.

## Impact

Any SDK caller that uses `resumeSession()` as a health check (verify session is alive before sending a message) gets silently broken — doubled events for the rest of the session. We discovered this when implementing plan-mode session recovery in a VS Code extension.

## Workaround

We use `session.abort()` as a lightweight liveness check instead of `resumeSession()`. `abort()` is a no-op on idle sessions and throws `"Session not found"` if the session was garbage-collected. Same signal, no side effects.

## Environment

- SDK: v0.1.22
- CLI: v0.0.421
- Node: v24.13.1

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.