github / github/copilot-sdk

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

オープン
#742 コメント 0 件 リアクション 2 件 担当者 0 名 GitHub で見る
主要言語
Java
スター
10.5k
フォーク
1.5k
平均マージ
1日 11時間
マージ済み PR(30日)
127

説明

## 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

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。