fix: improve background task completion detection and message extraction (#638)
* fix: background task completion detection and silent notifications - Fix TS2742 by adding explicit ToolDefinition type annotations - Add stability detection (3 consecutive stable polls after 10s minimum) - Remove early continue when sessionStatus is undefined - Add silent notification system via tool.execute.after hook injection - Change task retention from 200ms to 5 minutes for background_output retrieval - Fix formatTaskResult to sort messages by time descending Fixes hanging background tasks that never complete due to missing sessionStatus. * fix: improve background task completion detection and message extraction - Add stability-based completion detection (10s min + 3 stable polls) - Fix message extraction to recognize 'reasoning' parts from thinking models - Switch from promptAsync() to prompt() for proper agent initialization - Remove model parameter from prompt body (use agent's configured model) - Add fire-and-forget prompt pattern for sisyphus_task sync mode - Add silent notification via tool.execute.after hook injection - Fix indentation issues in manager.ts and index.ts Incorporates fixes from: - PR #592: Stability detection mechanism - PR #610: Model parameter passing (partially) - PR #628: Completion detection improvements Known limitation: Thinking models (e.g. claude-*-thinking-*) cause JSON Parse errors in child sessions. Use non-thinking models for background agents until OpenCode core resolves this. * fix: add tool_result handling and pendingByParent tracking for resume/external tasks Addresses code review feedback from PR #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. * fix: address code review feedback - log messages, model passthrough, sorting, race condition - Fix misleading log messages: 'promptAsync' -> 'prompt (fire-and-forget)' - Restore model passthrough in launch() for Sisyphus category configs - Fix call-omo-agent sorting: use time.created number instead of String(time) - Fix race condition: check promptError inside polling loop, not just after 100ms
This commit is contained in:
@@ -13,6 +13,7 @@ import { subagentSessions } from "../claude-code-session-state"
|
||||
import { getTaskToastManager } from "../task-toast-manager"
|
||||
|
||||
const TASK_TTL_MS = 30 * 60 * 1000
|
||||
const MIN_STABILITY_TIME_MS = 10 * 1000 // Must run at least 10s before stability detection kicks in
|
||||
|
||||
type OpencodeClient = PluginInput["client"]
|
||||
|
||||
@@ -43,6 +44,7 @@ interface Todo {
|
||||
export class BackgroundManager {
|
||||
private tasks: Map<string, BackgroundTask>
|
||||
private notifications: Map<string, BackgroundTask[]>
|
||||
private pendingByParent: Map<string, Set<string>> // Track pending tasks per parent for batching
|
||||
private client: OpencodeClient
|
||||
private directory: string
|
||||
private pollingInterval?: ReturnType<typeof setInterval>
|
||||
@@ -51,12 +53,20 @@ export class BackgroundManager {
|
||||
constructor(ctx: PluginInput, config?: BackgroundTaskConfig) {
|
||||
this.tasks = new Map()
|
||||
this.notifications = new Map()
|
||||
this.pendingByParent = new Map()
|
||||
this.client = ctx.client
|
||||
this.directory = ctx.directory
|
||||
this.concurrencyManager = new ConcurrencyManager(config)
|
||||
}
|
||||
|
||||
async launch(input: LaunchInput): Promise<BackgroundTask> {
|
||||
log("[background-agent] launch() called with:", {
|
||||
agent: input.agent,
|
||||
model: input.model,
|
||||
description: input.description,
|
||||
parentSessionID: input.parentSessionID,
|
||||
})
|
||||
|
||||
if (!input.agent || input.agent.trim() === "") {
|
||||
throw new Error("Agent parameter is required")
|
||||
}
|
||||
@@ -106,6 +116,11 @@ export class BackgroundManager {
|
||||
this.tasks.set(task.id, task)
|
||||
this.startPolling()
|
||||
|
||||
// Track for batched notifications
|
||||
const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set()
|
||||
pending.add(task.id)
|
||||
this.pendingByParent.set(input.parentSessionID, pending)
|
||||
|
||||
log("[background-agent] Launching task:", { taskId: task.id, sessionID, agent: input.agent })
|
||||
|
||||
const toastManager = getTaskToastManager()
|
||||
@@ -119,10 +134,21 @@ export class BackgroundManager {
|
||||
})
|
||||
}
|
||||
|
||||
this.client.session.promptAsync({
|
||||
log("[background-agent] Calling prompt (fire-and-forget) for launch with:", {
|
||||
sessionID,
|
||||
agent: input.agent,
|
||||
model: input.model,
|
||||
hasSkillContent: !!input.skillContent,
|
||||
promptLength: input.prompt.length,
|
||||
})
|
||||
|
||||
// Use prompt() instead of promptAsync() to properly initialize agent loop (fire-and-forget)
|
||||
// Include model if caller provided one (e.g., from Sisyphus category configs)
|
||||
this.client.session.prompt({
|
||||
path: { id: sessionID },
|
||||
body: {
|
||||
agent: input.agent,
|
||||
...(input.model ? { model: input.model } : {}),
|
||||
system: input.skillContent,
|
||||
tools: {
|
||||
task: false,
|
||||
@@ -146,7 +172,9 @@ export class BackgroundManager {
|
||||
this.concurrencyManager.release(existingTask.concurrencyKey)
|
||||
}
|
||||
this.markForNotification(existingTask)
|
||||
this.notifyParentSession(existingTask)
|
||||
this.notifyParentSession(existingTask).catch(err => {
|
||||
log("[background-agent] Failed to notify on error:", err)
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
@@ -222,6 +250,11 @@ export class BackgroundManager {
|
||||
subagentSessions.add(input.sessionID)
|
||||
this.startPolling()
|
||||
|
||||
// Track for batched notifications (external tasks need tracking too)
|
||||
const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set()
|
||||
pending.add(task.id)
|
||||
this.pendingByParent.set(input.parentSessionID, pending)
|
||||
|
||||
log("[background-agent] Registered external task:", { taskId: task.id, sessionID: input.sessionID })
|
||||
|
||||
return task
|
||||
@@ -249,6 +282,11 @@ export class BackgroundManager {
|
||||
this.startPolling()
|
||||
subagentSessions.add(existingTask.sessionID)
|
||||
|
||||
// Track for batched notifications (P2 fix: resumed tasks need tracking too)
|
||||
const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set()
|
||||
pending.add(existingTask.id)
|
||||
this.pendingByParent.set(input.parentSessionID, pending)
|
||||
|
||||
const toastManager = getTaskToastManager()
|
||||
if (toastManager) {
|
||||
toastManager.addTask({
|
||||
@@ -261,7 +299,15 @@ export class BackgroundManager {
|
||||
|
||||
log("[background-agent] Resuming task:", { taskId: existingTask.id, sessionID: existingTask.sessionID })
|
||||
|
||||
this.client.session.promptAsync({
|
||||
log("[background-agent] Resuming task - calling prompt (fire-and-forget) with:", {
|
||||
sessionID: existingTask.sessionID,
|
||||
agent: existingTask.agent,
|
||||
promptLength: input.prompt.length,
|
||||
})
|
||||
|
||||
// Note: Don't pass model in body - use agent's configured model instead
|
||||
// Use prompt() instead of promptAsync() to properly initialize agent loop
|
||||
this.client.session.prompt({
|
||||
path: { id: existingTask.sessionID },
|
||||
body: {
|
||||
agent: existingTask.agent,
|
||||
@@ -272,13 +318,15 @@ export class BackgroundManager {
|
||||
parts: [{ type: "text", text: input.prompt }],
|
||||
},
|
||||
}).catch((error) => {
|
||||
log("[background-agent] resume promptAsync error:", error)
|
||||
log("[background-agent] resume prompt error:", error)
|
||||
existingTask.status = "error"
|
||||
const errorMessage = error instanceof Error ? error.message : String(error)
|
||||
existingTask.error = errorMessage
|
||||
existingTask.completedAt = new Date()
|
||||
this.markForNotification(existingTask)
|
||||
this.notifyParentSession(existingTask)
|
||||
this.notifyParentSession(existingTask).catch(err => {
|
||||
log("[background-agent] Failed to notify on resume error:", err)
|
||||
})
|
||||
})
|
||||
|
||||
return existingTask
|
||||
@@ -333,7 +381,22 @@ export class BackgroundManager {
|
||||
const task = this.findBySession(sessionID)
|
||||
if (!task || task.status !== "running") return
|
||||
|
||||
this.checkSessionTodos(sessionID).then((hasIncompleteTodos) => {
|
||||
// Edge guard: Require minimum elapsed time (5 seconds) before accepting idle
|
||||
const elapsedMs = Date.now() - task.startedAt.getTime()
|
||||
const MIN_IDLE_TIME_MS = 5000
|
||||
if (elapsedMs < MIN_IDLE_TIME_MS) {
|
||||
log("[background-agent] Ignoring early session.idle, elapsed:", { elapsedMs, taskId: task.id })
|
||||
return
|
||||
}
|
||||
|
||||
// Edge guard: Verify session has actual assistant output before completing
|
||||
this.validateSessionHasOutput(sessionID).then(async (hasValidOutput) => {
|
||||
if (!hasValidOutput) {
|
||||
log("[background-agent] Session.idle but no valid output yet, waiting:", task.id)
|
||||
return
|
||||
}
|
||||
|
||||
const hasIncompleteTodos = await this.checkSessionTodos(sessionID)
|
||||
if (hasIncompleteTodos) {
|
||||
log("[background-agent] Task has incomplete todos, waiting for todo-continuation:", task.id)
|
||||
return
|
||||
@@ -342,8 +405,10 @@ export class BackgroundManager {
|
||||
task.status = "completed"
|
||||
task.completedAt = new Date()
|
||||
this.markForNotification(task)
|
||||
this.notifyParentSession(task)
|
||||
await this.notifyParentSession(task)
|
||||
log("[background-agent] Task completed via session.idle event:", task.id)
|
||||
}).catch(err => {
|
||||
log("[background-agent] Error in session.idle handler:", err)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -384,6 +449,66 @@ export class BackgroundManager {
|
||||
this.notifications.delete(sessionID)
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that a session has actual assistant/tool output before marking complete.
|
||||
* Prevents premature completion when session.idle fires before agent responds.
|
||||
*/
|
||||
private async validateSessionHasOutput(sessionID: string): Promise<boolean> {
|
||||
try {
|
||||
const response = await this.client.session.messages({
|
||||
path: { id: sessionID },
|
||||
})
|
||||
|
||||
const messages = response.data ?? []
|
||||
|
||||
// Check for at least one assistant or tool message
|
||||
const hasAssistantOrToolMessage = messages.some(
|
||||
(m: { info?: { role?: string } }) =>
|
||||
m.info?.role === "assistant" || m.info?.role === "tool"
|
||||
)
|
||||
|
||||
if (!hasAssistantOrToolMessage) {
|
||||
log("[background-agent] No assistant/tool messages found in session:", sessionID)
|
||||
return false
|
||||
}
|
||||
|
||||
// Additionally check that at least one message has content (not just empty)
|
||||
// OpenCode API uses different part types than Anthropic's API:
|
||||
// - "reasoning" with .text property (thinking/reasoning content)
|
||||
// - "tool" with .state.output property (tool call results)
|
||||
// - "text" with .text property (final text output)
|
||||
// - "step-start"/"step-finish" (metadata, no content)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const hasContent = messages.some((m: any) => {
|
||||
if (m.info?.role !== "assistant" && m.info?.role !== "tool") return false
|
||||
const parts = m.parts ?? []
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
return parts.some((p: any) =>
|
||||
// Text content (final output)
|
||||
(p.type === "text" && p.text && p.text.trim().length > 0) ||
|
||||
// Reasoning content (thinking blocks)
|
||||
(p.type === "reasoning" && p.text && p.text.trim().length > 0) ||
|
||||
// Tool calls (indicates work was done)
|
||||
p.type === "tool" ||
|
||||
// Tool results (output from executed tools) - important for tool-only tasks
|
||||
(p.type === "tool_result" && p.content &&
|
||||
(typeof p.content === "string" ? p.content.trim().length > 0 : p.content.length > 0))
|
||||
)
|
||||
})
|
||||
|
||||
if (!hasContent) {
|
||||
log("[background-agent] Messages exist but no content found in session:", sessionID)
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
} catch (error) {
|
||||
log("[background-agent] Error validating session output:", error)
|
||||
// On error, allow completion to proceed (don't block indefinitely)
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
private clearNotificationsForTask(taskId: string): void {
|
||||
for (const [sessionID, tasks] of this.notifications.entries()) {
|
||||
const filtered = tasks.filter((t) => t.id !== taskId)
|
||||
@@ -411,17 +536,33 @@ export class BackgroundManager {
|
||||
}
|
||||
}
|
||||
|
||||
cleanup(): void {
|
||||
cleanup(): void {
|
||||
this.stopPolling()
|
||||
this.tasks.clear()
|
||||
this.notifications.clear()
|
||||
this.pendingByParent.clear()
|
||||
}
|
||||
|
||||
private notifyParentSession(task: BackgroundTask): void {
|
||||
/**
|
||||
* Get all running tasks (for compaction hook)
|
||||
*/
|
||||
getRunningTasks(): BackgroundTask[] {
|
||||
return Array.from(this.tasks.values()).filter(t => t.status === "running")
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all completed tasks still in memory (for compaction hook)
|
||||
*/
|
||||
getCompletedTasks(): BackgroundTask[] {
|
||||
return Array.from(this.tasks.values()).filter(t => t.status !== "running")
|
||||
}
|
||||
|
||||
private async notifyParentSession(task: BackgroundTask): Promise<void> {
|
||||
const duration = this.formatDuration(task.startedAt, task.completedAt)
|
||||
|
||||
log("[background-agent] notifyParentSession called for task:", task.id)
|
||||
|
||||
// Show toast notification
|
||||
const toastManager = getTaskToastManager()
|
||||
if (toastManager) {
|
||||
toastManager.showCompletionToast({
|
||||
@@ -431,47 +572,83 @@ export class BackgroundManager {
|
||||
})
|
||||
}
|
||||
|
||||
const message = `[BACKGROUND TASK COMPLETED] Task "${task.description}" finished in ${duration}. Use background_output with task_id="${task.id}" to get results.`
|
||||
// Update pending tracking and check if all tasks complete
|
||||
const pendingSet = this.pendingByParent.get(task.parentSessionID)
|
||||
if (pendingSet) {
|
||||
pendingSet.delete(task.id)
|
||||
if (pendingSet.size === 0) {
|
||||
this.pendingByParent.delete(task.parentSessionID)
|
||||
}
|
||||
}
|
||||
|
||||
log("[background-agent] Sending notification to parent session:", { parentSessionID: task.parentSessionID })
|
||||
const allComplete = !pendingSet || pendingSet.size === 0
|
||||
const remainingCount = pendingSet?.size ?? 0
|
||||
|
||||
// Build notification message
|
||||
const statusText = task.status === "error" ? "FAILED" : "COMPLETED"
|
||||
const errorInfo = task.error ? `\n**Error:** ${task.error}` : ""
|
||||
|
||||
let notification: string
|
||||
if (allComplete) {
|
||||
// All tasks complete - build summary
|
||||
const completedTasks = Array.from(this.tasks.values())
|
||||
.filter(t => t.parentSessionID === task.parentSessionID && t.status !== "running")
|
||||
.map(t => `- \`${t.id}\`: ${t.description}`)
|
||||
.join("\n")
|
||||
|
||||
notification = `<system-reminder>
|
||||
[ALL BACKGROUND TASKS COMPLETE]
|
||||
|
||||
**Completed:**
|
||||
${completedTasks || `- \`${task.id}\`: ${task.description}`}
|
||||
|
||||
Use \`background_output(task_id="<id>")\` to retrieve each result.
|
||||
</system-reminder>`
|
||||
} else {
|
||||
// Individual completion - silent notification
|
||||
notification = `<system-reminder>
|
||||
[BACKGROUND TASK ${statusText}]
|
||||
**ID:** \`${task.id}\`
|
||||
**Description:** ${task.description}
|
||||
**Duration:** ${duration}${errorInfo}
|
||||
|
||||
**${remainingCount} task${remainingCount === 1 ? "" : "s"} still in progress.** You WILL be notified when ALL complete.
|
||||
Do NOT poll - continue productive work.
|
||||
|
||||
Use \`background_output(task_id="${task.id}")\` to retrieve this result when ready.
|
||||
</system-reminder>`
|
||||
}
|
||||
|
||||
// Inject notification via session.prompt with noReply
|
||||
try {
|
||||
await this.client.session.prompt({
|
||||
path: { id: task.parentSessionID },
|
||||
body: {
|
||||
noReply: !allComplete, // Silent unless all complete
|
||||
agent: task.parentAgent,
|
||||
parts: [{ type: "text", text: notification }],
|
||||
},
|
||||
})
|
||||
log("[background-agent] Sent notification to parent session:", {
|
||||
taskId: task.id,
|
||||
allComplete,
|
||||
noReply: !allComplete,
|
||||
})
|
||||
} catch (error) {
|
||||
log("[background-agent] Failed to send notification:", error)
|
||||
}
|
||||
|
||||
// Cleanup after retention period
|
||||
const taskId = task.id
|
||||
setTimeout(async () => {
|
||||
setTimeout(() => {
|
||||
if (task.concurrencyKey) {
|
||||
this.concurrencyManager.release(task.concurrencyKey)
|
||||
task.concurrencyKey = undefined
|
||||
}
|
||||
|
||||
try {
|
||||
const body: {
|
||||
agent?: string
|
||||
model?: { providerID: string; modelID: string }
|
||||
parts: Array<{ type: "text"; text: string }>
|
||||
} = {
|
||||
parts: [{ type: "text", text: message }],
|
||||
}
|
||||
|
||||
if (task.parentAgent !== undefined) {
|
||||
body.agent = task.parentAgent
|
||||
}
|
||||
|
||||
if (task.parentModel?.providerID && task.parentModel?.modelID) {
|
||||
body.model = { providerID: task.parentModel.providerID, modelID: task.parentModel.modelID }
|
||||
}
|
||||
|
||||
await this.client.session.prompt({
|
||||
path: { id: task.parentSessionID },
|
||||
body,
|
||||
query: { directory: this.directory },
|
||||
})
|
||||
log("[background-agent] Successfully sent prompt to parent session:", { parentSessionID: task.parentSessionID })
|
||||
} catch (error) {
|
||||
log("[background-agent] prompt failed:", String(error))
|
||||
} finally {
|
||||
this.clearNotificationsForTask(taskId)
|
||||
this.tasks.delete(taskId)
|
||||
log("[background-agent] Removed completed task from memory:", taskId)
|
||||
}
|
||||
}, 200)
|
||||
this.clearNotificationsForTask(taskId)
|
||||
this.tasks.delete(taskId)
|
||||
log("[background-agent] Removed completed task from memory:", taskId)
|
||||
}, 5 * 60 * 1000)
|
||||
}
|
||||
|
||||
private formatDuration(start: Date, end?: Date): string {
|
||||
@@ -540,15 +717,18 @@ export class BackgroundManager {
|
||||
for (const task of this.tasks.values()) {
|
||||
if (task.status !== "running") continue
|
||||
|
||||
try {
|
||||
try {
|
||||
const sessionStatus = allStatuses[task.sessionID]
|
||||
|
||||
if (!sessionStatus) {
|
||||
log("[background-agent] Session not found in status:", task.sessionID)
|
||||
continue
|
||||
}
|
||||
// Don't skip if session not in status - fall through to message-based detection
|
||||
if (sessionStatus?.type === "idle") {
|
||||
// Edge guard: Validate session has actual output before completing
|
||||
const hasValidOutput = await this.validateSessionHasOutput(task.sessionID)
|
||||
if (!hasValidOutput) {
|
||||
log("[background-agent] Polling idle but no valid output yet, waiting:", task.id)
|
||||
continue
|
||||
}
|
||||
|
||||
if (sessionStatus.type === "idle") {
|
||||
const hasIncompleteTodos = await this.checkSessionTodos(task.sessionID)
|
||||
if (hasIncompleteTodos) {
|
||||
log("[background-agent] Task has incomplete todos via polling, waiting:", task.id)
|
||||
@@ -558,7 +738,7 @@ export class BackgroundManager {
|
||||
task.status = "completed"
|
||||
task.completedAt = new Date()
|
||||
this.markForNotification(task)
|
||||
this.notifyParentSession(task)
|
||||
await this.notifyParentSession(task)
|
||||
log("[background-agent] Task completed via polling:", task.id)
|
||||
continue
|
||||
}
|
||||
@@ -599,10 +779,41 @@ export class BackgroundManager {
|
||||
task.progress.toolCalls = toolCalls
|
||||
task.progress.lastTool = lastTool
|
||||
task.progress.lastUpdate = new Date()
|
||||
if (lastMessage) {
|
||||
if (lastMessage) {
|
||||
task.progress.lastMessage = lastMessage
|
||||
task.progress.lastMessageAt = new Date()
|
||||
}
|
||||
|
||||
// Stability detection: complete when message count unchanged for 3 polls
|
||||
const currentMsgCount = messages.length
|
||||
const elapsedMs = Date.now() - task.startedAt.getTime()
|
||||
|
||||
if (elapsedMs >= MIN_STABILITY_TIME_MS) {
|
||||
if (task.lastMsgCount === currentMsgCount) {
|
||||
task.stablePolls = (task.stablePolls ?? 0) + 1
|
||||
if (task.stablePolls >= 3) {
|
||||
// Edge guard: Validate session has actual output before completing
|
||||
const hasValidOutput = await this.validateSessionHasOutput(task.sessionID)
|
||||
if (!hasValidOutput) {
|
||||
log("[background-agent] Stability reached but no valid output, waiting:", task.id)
|
||||
continue
|
||||
}
|
||||
|
||||
const hasIncompleteTodos = await this.checkSessionTodos(task.sessionID)
|
||||
if (!hasIncompleteTodos) {
|
||||
task.status = "completed"
|
||||
task.completedAt = new Date()
|
||||
this.markForNotification(task)
|
||||
await this.notifyParentSession(task)
|
||||
log("[background-agent] Task completed via stability detection:", task.id)
|
||||
continue
|
||||
}
|
||||
}
|
||||
} else {
|
||||
task.stablePolls = 0
|
||||
}
|
||||
}
|
||||
task.lastMsgCount = currentMsgCount
|
||||
}
|
||||
} catch (error) {
|
||||
log("[background-agent] Poll error for task:", { taskId: task.id, error })
|
||||
|
||||
@@ -32,6 +32,10 @@ export interface BackgroundTask {
|
||||
concurrencyKey?: string
|
||||
/** Parent session's agent name for notification */
|
||||
parentAgent?: string
|
||||
/** Last message count for stability detection */
|
||||
lastMsgCount?: number
|
||||
/** Number of consecutive polls with stable message count */
|
||||
stablePolls?: number
|
||||
}
|
||||
|
||||
export interface LaunchInput {
|
||||
|
||||
Reference in New Issue
Block a user