Skip to content

Commit de24ea1

Browse files
PureWeenCopilot
andcommitted
fix: sendAndWait resolves prematurely on session.idle with active background tasks (PolyPilot #299)
Before this fix all four SDKs' sendAndWait/SendAndWaitAsync/send_and_wait/SendAndWait resolved immediately on ANY session.idle event, even when backgroundTasks.agents[] or backgroundTasks.shells[] was non-empty — indicating background agents/shells are still running and the session is not truly idle. Fix: only resolve when backgroundTasks is absent or both arrays are empty. Changed files: - nodejs/src/session.ts: check bgTasks.agents.length / bgTasks.shells.length - dotnet/src/Session.cs: check bgTasks.Agents.Length / bgTasks.Shells.Length - python/copilot/session.py: check len(bg_tasks.agents) / len(bg_tasks.shells) - go/session.go: check len(bgTasks.Agents) / len(bgTasks.Shells) Tests: added 4 Node.js unit tests in test/client.test.ts that reproduce the bug (tests 2+3 fail before fix, pass after): - resolves immediately when session.idle has no backgroundTasks (baseline) - does NOT resolve when session.idle reports active background agents [bug repro] - does NOT resolve when session.idle reports active background shells - resolves when session.idle has empty backgroundTasks arrays Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1587e34 commit de24ea1

File tree

5 files changed

+223
-7
lines changed

5 files changed

+223
-7
lines changed

dotnet/src/Session.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,12 @@ void Handler(SessionEvent evt)
216216
lastAssistantMessage = assistantMessage;
217217
break;
218218

219-
case SessionIdleEvent:
220-
tcs.TrySetResult(lastAssistantMessage);
219+
case SessionIdleEvent idleEvent:
220+
var bgTasks = idleEvent.Data?.BackgroundTasks;
221+
bool hasActiveTasks = bgTasks != null &&
222+
((bgTasks.Agents?.Length ?? 0) > 0 || (bgTasks.Shells?.Length ?? 0) > 0);
223+
if (!hasActiveTasks)
224+
tcs.TrySetResult(lastAssistantMessage);
221225
break;
222226

223227
case SessionErrorEvent errorEvent:

go/session.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,13 @@ func (s *Session) SendAndWait(ctx context.Context, options MessageOptions) (*Ses
190190
lastAssistantMessage = &eventCopy
191191
mu.Unlock()
192192
case SessionEventTypeSessionIdle:
193-
select {
194-
case idleCh <- struct{}{}:
195-
default:
193+
bgTasks := event.Data.BackgroundTasks
194+
hasActive := bgTasks != nil && (len(bgTasks.Agents) > 0 || len(bgTasks.Shells) > 0)
195+
if !hasActive {
196+
select {
197+
case idleCh <- struct{}{}:
198+
default:
199+
}
196200
}
197201
case SessionEventTypeSessionError:
198202
errMsg := "session error"

nodejs/src/session.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,13 @@ export class CopilotSession {
232232
if (event.type === "assistant.message") {
233233
lastAssistantMessage = event;
234234
} else if (event.type === "session.idle") {
235-
resolveIdle();
235+
const bgTasks = event.data.backgroundTasks;
236+
const hasActiveTasks =
237+
bgTasks !== undefined &&
238+
(bgTasks.agents.length > 0 || bgTasks.shells.length > 0);
239+
if (!hasActiveTasks) {
240+
resolveIdle();
241+
}
236242
} else if (event.type === "session.error") {
237243
const error = new Error(event.data.message);
238244
error.stack = event.data.stack;

nodejs/test/client.test.ts

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,4 +977,201 @@ describe("CopilotClient", () => {
977977
rpcSpy.mockRestore();
978978
});
979979
});
980+
981+
// ─────────────────────────────────────────────────────────────────────────
982+
// Bug repro: sendAndWait must NOT resolve on session.idle while background
983+
// tasks are still running (PolyPilot #299/#389).
984+
//
985+
// Before fix: sendAndWait called resolveIdle() on ANY session.idle, even
986+
// when backgroundTasks.agents[] or backgroundTasks.shells[] was non-empty.
987+
// After fix: resolveIdle() is only called when backgroundTasks is absent or
988+
// both agents[] and shells[] are empty.
989+
// ─────────────────────────────────────────────────────────────────────────
990+
describe("sendAndWait backgroundTasks", () => {
991+
it("resolves immediately when session.idle has no backgroundTasks", async () => {
992+
const client = new CopilotClient({ logLevel: "error" });
993+
await client.start();
994+
onTestFinished(() => client.forceStop());
995+
const session = await client.createSession({ onPermissionRequest: approveAll });
996+
997+
const sendSpy = vi
998+
.spyOn((client as any).connection!, "sendRequest")
999+
.mockImplementation(async (method: string) => {
1000+
if (method === "session.send") return { messageId: "mock-clean-idle" };
1001+
throw new Error(`Unexpected RPC: ${method}`);
1002+
});
1003+
onTestFinished(() => sendSpy.mockRestore());
1004+
1005+
let resolved = false;
1006+
const promise = session.sendAndWait({ prompt: "hello" }, 5_000).then((r) => {
1007+
resolved = true;
1008+
return r;
1009+
});
1010+
1011+
await new Promise<void>((r) => setTimeout(r, 20));
1012+
1013+
// Clean idle (no backgroundTasks) — sendAndWait SHOULD resolve
1014+
(session as any)._dispatchEvent({
1015+
id: "evt-clean-idle",
1016+
timestamp: new Date().toISOString(),
1017+
parentId: null,
1018+
ephemeral: true,
1019+
type: "session.idle",
1020+
data: {},
1021+
});
1022+
1023+
await promise;
1024+
expect(resolved).toBe(true);
1025+
});
1026+
1027+
it("does NOT resolve when session.idle reports active background agents [bug #299 repro]", async () => {
1028+
const client = new CopilotClient({ logLevel: "error" });
1029+
await client.start();
1030+
onTestFinished(() => client.forceStop());
1031+
const session = await client.createSession({ onPermissionRequest: approveAll });
1032+
1033+
const sendSpy = vi
1034+
.spyOn((client as any).connection!, "sendRequest")
1035+
.mockImplementation(async (method: string) => {
1036+
if (method === "session.send") return { messageId: "mock-bg-idle" };
1037+
throw new Error(`Unexpected RPC: ${method}`);
1038+
});
1039+
onTestFinished(() => sendSpy.mockRestore());
1040+
1041+
let resolved = false;
1042+
const promise = session
1043+
.sendAndWait({ prompt: "trigger background agents" }, 5_000)
1044+
.then((r) => {
1045+
resolved = true;
1046+
return r;
1047+
});
1048+
1049+
await new Promise<void>((r) => setTimeout(r, 20));
1050+
1051+
// Dispatch session.idle WITH active background agent — must NOT resolve yet
1052+
(session as any)._dispatchEvent({
1053+
id: "evt-bg-idle",
1054+
timestamp: new Date().toISOString(),
1055+
parentId: null,
1056+
ephemeral: true,
1057+
type: "session.idle",
1058+
data: {
1059+
backgroundTasks: {
1060+
agents: [{ agentId: "bg-1", agentType: "worker" }],
1061+
shells: [],
1062+
},
1063+
},
1064+
});
1065+
1066+
await new Promise<void>((r) => setTimeout(r, 100));
1067+
// Before fix: resolved === true (premature resolution — this assertion FAILS)
1068+
// After fix: resolved === false (correctly waiting)
1069+
expect(resolved).toBe(false);
1070+
1071+
// Dispatch final idle with no background tasks — NOW it should resolve
1072+
(session as any)._dispatchEvent({
1073+
id: "evt-final-idle",
1074+
timestamp: new Date().toISOString(),
1075+
parentId: null,
1076+
ephemeral: true,
1077+
type: "session.idle",
1078+
data: {},
1079+
});
1080+
1081+
await promise;
1082+
expect(resolved).toBe(true);
1083+
});
1084+
1085+
it("does NOT resolve when session.idle reports active background shells", async () => {
1086+
const client = new CopilotClient({ logLevel: "error" });
1087+
await client.start();
1088+
onTestFinished(() => client.forceStop());
1089+
const session = await client.createSession({ onPermissionRequest: approveAll });
1090+
1091+
const sendSpy = vi
1092+
.spyOn((client as any).connection!, "sendRequest")
1093+
.mockImplementation(async (method: string) => {
1094+
if (method === "session.send") return { messageId: "mock-shell-idle" };
1095+
throw new Error(`Unexpected RPC: ${method}`);
1096+
});
1097+
onTestFinished(() => sendSpy.mockRestore());
1098+
1099+
let resolved = false;
1100+
const promise = session.sendAndWait({ prompt: "trigger shell" }, 5_000).then((r) => {
1101+
resolved = true;
1102+
return r;
1103+
});
1104+
1105+
await new Promise<void>((r) => setTimeout(r, 20));
1106+
1107+
// Dispatch session.idle with an active background shell — must NOT resolve yet
1108+
(session as any)._dispatchEvent({
1109+
id: "evt-shell-idle",
1110+
timestamp: new Date().toISOString(),
1111+
parentId: null,
1112+
ephemeral: true,
1113+
type: "session.idle",
1114+
data: {
1115+
backgroundTasks: {
1116+
agents: [],
1117+
shells: [{ shellId: "sh-1" }],
1118+
},
1119+
},
1120+
});
1121+
1122+
await new Promise<void>((r) => setTimeout(r, 100));
1123+
expect(resolved).toBe(false);
1124+
1125+
// Dispatch final idle with empty backgroundTasks — should resolve
1126+
(session as any)._dispatchEvent({
1127+
id: "evt-final-shell-idle",
1128+
timestamp: new Date().toISOString(),
1129+
parentId: null,
1130+
ephemeral: true,
1131+
type: "session.idle",
1132+
data: { backgroundTasks: { agents: [], shells: [] } },
1133+
});
1134+
1135+
await promise;
1136+
expect(resolved).toBe(true);
1137+
});
1138+
1139+
it("resolves when session.idle has empty backgroundTasks arrays", async () => {
1140+
const client = new CopilotClient({ logLevel: "error" });
1141+
await client.start();
1142+
onTestFinished(() => client.forceStop());
1143+
const session = await client.createSession({ onPermissionRequest: approveAll });
1144+
1145+
const sendSpy = vi
1146+
.spyOn((client as any).connection!, "sendRequest")
1147+
.mockImplementation(async (method: string) => {
1148+
if (method === "session.send") return { messageId: "mock-empty-bg" };
1149+
throw new Error(`Unexpected RPC: ${method}`);
1150+
});
1151+
onTestFinished(() => sendSpy.mockRestore());
1152+
1153+
let resolved = false;
1154+
const promise = session
1155+
.sendAndWait({ prompt: "empty background tasks" }, 5_000)
1156+
.then((r) => {
1157+
resolved = true;
1158+
return r;
1159+
});
1160+
1161+
await new Promise<void>((r) => setTimeout(r, 20));
1162+
1163+
// Both arrays empty → session is truly idle, SHOULD resolve
1164+
(session as any)._dispatchEvent({
1165+
id: "evt-empty-bg-idle",
1166+
timestamp: new Date().toISOString(),
1167+
parentId: null,
1168+
ephemeral: true,
1169+
type: "session.idle",
1170+
data: { backgroundTasks: { agents: [], shells: [] } },
1171+
});
1172+
1173+
await promise;
1174+
expect(resolved).toBe(true);
1175+
});
1176+
});
9801177
});

python/copilot/session.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,12 @@ def handler(event: SessionEventTypeAlias) -> None:
789789
if event.type == SessionEventType.ASSISTANT_MESSAGE:
790790
last_assistant_message = event
791791
elif event.type == SessionEventType.SESSION_IDLE:
792-
idle_event.set()
792+
bg_tasks = event.data.background_tasks
793+
has_active = bg_tasks is not None and (
794+
len(bg_tasks.agents) > 0 or len(bg_tasks.shells) > 0
795+
)
796+
if not has_active:
797+
idle_event.set()
793798
elif event.type == SessionEventType.SESSION_ERROR:
794799
error_event = Exception(
795800
f"Session error: {getattr(event.data, 'message', str(event.data))}"

0 commit comments

Comments
 (0)