Skip to content

Commit 59dddb4

Browse files
Gladdonillirenekris
authored andcommitted
fix: add tool_result handling and pendingByParent tracking for resume/external tasks
Addresses code review feedback from PR code-yeongyu#638: P1: Add tool_result type to validateSessionHasOutput() to prevent false negatives for tool-only background tasks that would otherwise timeout after 30 minutes despite having valid results. P2: Add pendingByParent tracking to resume() and registerExternalTask() to prevent premature 'ALL COMPLETE' notifications when mixing launched and resumed tasks.
1 parent d0d3fd8 commit 59dddb4

File tree

1 file changed

+22
-8
lines changed

1 file changed

+22
-8
lines changed

src/features/background-agent/manager.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,11 @@ export class BackgroundManager {
249249
subagentSessions.add(input.sessionID)
250250
this.startPolling()
251251

252+
// Track for batched notifications (external tasks need tracking too)
253+
const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set()
254+
pending.add(task.id)
255+
this.pendingByParent.set(input.parentSessionID, pending)
256+
252257
log("[background-agent] Registered external task:", { taskId: task.id, sessionID: input.sessionID })
253258

254259
return task
@@ -276,6 +281,11 @@ export class BackgroundManager {
276281
this.startPolling()
277282
subagentSessions.add(existingTask.sessionID)
278283

284+
// Track for batched notifications (P2 fix: resumed tasks need tracking too)
285+
const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set()
286+
pending.add(existingTask.id)
287+
this.pendingByParent.set(input.parentSessionID, pending)
288+
279289
const toastManager = getTaskToastManager()
280290
if (toastManager) {
281291
toastManager.addTask({
@@ -471,14 +481,18 @@ export class BackgroundManager {
471481
const hasContent = messages.some((m: any) => {
472482
if (m.info?.role !== "assistant" && m.info?.role !== "tool") return false
473483
const parts = m.parts ?? []
474-
return parts.some((p: { type?: string; text?: string }) =>
475-
// Text content (final output)
476-
(p.type === "text" && p.text && p.text.trim().length > 0) ||
477-
// Reasoning content (thinking blocks)
478-
(p.type === "reasoning" && p.text && p.text.trim().length > 0) ||
479-
// Tool calls (indicates work was done)
480-
p.type === "tool"
481-
)
484+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
485+
return parts.some((p: any) =>
486+
// Text content (final output)
487+
(p.type === "text" && p.text && p.text.trim().length > 0) ||
488+
// Reasoning content (thinking blocks)
489+
(p.type === "reasoning" && p.text && p.text.trim().length > 0) ||
490+
// Tool calls (indicates work was done)
491+
p.type === "tool" ||
492+
// Tool results (output from executed tools) - important for tool-only tasks
493+
(p.type === "tool_result" && p.content &&
494+
(typeof p.content === "string" ? p.content.trim().length > 0 : p.content.length > 0))
495+
)
482496
})
483497

484498
if (!hasContent) {

0 commit comments

Comments
 (0)