luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: improve background task completion detection and message extraction (#638)

Browse Source 
* 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:
Gladdonilli
2026-01-10 13:00:25 +08:00
committed by GitHub
parent 9bd16a87ea
commit bd116968db
11 changed files with 540 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/tools/background-task/tools.ts
+49 -13
View File
@@ -176,8 +176,13 @@ async function formatTaskResult(task: BackgroundTask, client: OpencodeClient): P
// Handle both SDK response structures: direct array or wrapped in .data
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const messages = ((messagesResult as any).data ?? messagesResult) as Array<{
info?: { role?: string }
parts?: Array<{ type?: string; text?: string }>
info?: { role?: string; time?: string }
parts?: Array<{
type?: string
text?: string
content?: string | Array<{ type: string; text?: string }>
name?: string
}>
}>
if (!Array.isArray(messages) || messages.length === 0) {
@@ -193,11 +198,13 @@ Session ID: ${task.sessionID}
(No messages found)`
}
const assistantMessages = messages.filter(
(m) => m.info?.role === "assistant"
// Include both assistant messages AND tool messages
// Tool results (grep, glob, bash output) come from role "tool"
const relevantMessages = messages.filter(
(m) => m.info?.role === "assistant" || m.info?.role === "tool"
)
if (assistantMessages.length === 0) {
if (relevantMessages.length === 0) {
return `Task Result
Task ID: ${task.id}
@@ -207,17 +214,46 @@ Session ID: ${task.sessionID}
---
(No assistant response found)`
(No assistant or tool response found)`
}
const lastMessage = assistantMessages[assistantMessages.length - 1]
const textParts = lastMessage?.parts?.filter(
(p) => p.type === "text"
) ?? []
const textContent = textParts
.map((p) => p.text ?? "")
// Sort by time ascending (oldest first) to process messages in order
const sortedMessages = [...relevantMessages].sort((a, b) => {
const timeA = String((a as { info?: { time?: string } }).info?.time ?? "")
const timeB = String((b as { info?: { time?: string } }).info?.time ?? "")
return timeA.localeCompare(timeB)
})
// Extract content from ALL messages, not just the last one
// Tool results may be in earlier messages while the final message is empty
const extractedContent: string[] = []
for (const message of sortedMessages) {
for (const part of message.parts ?? []) {
// Handle both "text" and "reasoning" parts (thinking models use "reasoning")
if ((part.type === "text" || part.type === "reasoning") && part.text) {
extractedContent.push(part.text)
} else if (part.type === "tool_result") {
// Tool results contain the actual output from tool calls
const toolResult = part as { content?: string | Array<{ type: string; text?: string }> }
if (typeof toolResult.content === "string" && toolResult.content) {
extractedContent.push(toolResult.content)
} else if (Array.isArray(toolResult.content)) {
// Handle array of content blocks
for (const block of toolResult.content) {
// Handle both "text" and "reasoning" parts (thinking models use "reasoning")
if ((block.type === "text" || block.type === "reasoning") && block.text) {
extractedContent.push(block.text)
}
}
}
}
}
}
const textContent = extractedContent
.filter((text) => text.length > 0)
.join("\n")
.join("\n\n")
const duration = formatDuration(task.startedAt, task.completedAt)
src/tools/call-omo-agent/tools.ts
+46 -10
View File
@@ -170,23 +170,59 @@ async function executeSync(
const messages = messagesResult.data
log(`[call_omo_agent] Got ${messages.length} messages`)
// Include both assistant messages AND tool messages
// Tool results (grep, glob, bash output) come from role "tool"
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const lastAssistantMessage = messages
.filter((m: any) => m.info.role === "assistant")
.sort((a: any, b: any) => (b.info.time?.created || 0) - (a.info.time?.created || 0))[0]
const relevantMessages = messages.filter(
(m: any) => m.info?.role === "assistant" || m.info?.role === "tool"
)
if (!lastAssistantMessage) {
log(`[call_omo_agent] No assistant message found`)
if (relevantMessages.length === 0) {
log(`[call_omo_agent] No assistant or tool messages found`)
log(`[call_omo_agent] All messages:`, JSON.stringify(messages, null, 2))
return `Error: No assistant response found\n\n<task_metadata>\nsession_id: ${sessionID}\n</task_metadata>`
return `Error: No assistant or tool response found\n\n<task_metadata>\nsession_id: ${sessionID}\n</task_metadata>`
}
log(`[call_omo_agent] Found assistant message with ${lastAssistantMessage.parts.length} parts`)
log(`[call_omo_agent] Found ${relevantMessages.length} relevant messages`)
// Sort by time ascending (oldest first) to process messages in order
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const textParts = lastAssistantMessage.parts.filter((p: any) => p.type === "text")
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const responseText = textParts.map((p: any) => p.text).join("\n")
const sortedMessages = [...relevantMessages].sort((a: any, b: any) => {
const timeA = a.info?.time?.created ?? 0
const timeB = b.info?.time?.created ?? 0
return timeA - timeB
})
// Extract content from ALL messages, not just the last one
// Tool results may be in earlier messages while the final message is empty
const extractedContent: string[] = []
for (const message of sortedMessages) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
for (const part of (message as any).parts ?? []) {
// Handle both "text" and "reasoning" parts (thinking models use "reasoning")
if ((part.type === "text" || part.type === "reasoning") && part.text) {
extractedContent.push(part.text)
} else if (part.type === "tool_result") {
// Tool results contain the actual output from tool calls
const toolResult = part as { content?: string | Array<{ type: string; text?: string }> }
if (typeof toolResult.content === "string" && toolResult.content) {
extractedContent.push(toolResult.content)
} else if (Array.isArray(toolResult.content)) {
// Handle array of content blocks
for (const block of toolResult.content) {
if ((block.type === "text" || block.type === "reasoning") && block.text) {
extractedContent.push(block.text)
}
}
}
}
}
}
const responseText = extractedContent
.filter((text) => text.length > 0)
.join("\n\n")
log(`[call_omo_agent] Got response, length: ${responseText.length}`)
src/tools/sisyphus-task/tools.ts
+83 -9
View File
@@ -221,6 +221,33 @@ Use \`background_output\` with task_id="${task.id}" to check progress.`
return `❌ Failed to send resume prompt: ${errorMessage}\n\nSession ID: ${args.resume}`
}
// Wait for message stability after prompt completes
const POLL_INTERVAL_MS = 500
const MIN_STABILITY_TIME_MS = 5000
const STABILITY_POLLS_REQUIRED = 3
const pollStart = Date.now()
let lastMsgCount = 0
let stablePolls = 0
while (Date.now() - pollStart < 60000) {
await new Promise(resolve => setTimeout(resolve, POLL_INTERVAL_MS))
const elapsed = Date.now() - pollStart
if (elapsed < MIN_STABILITY_TIME_MS) continue
const messagesCheck = await client.session.messages({ path: { id: args.resume } })
const msgs = ((messagesCheck as { data?: unknown }).data ?? messagesCheck) as Array<unknown>
const currentMsgCount = msgs.length
if (currentMsgCount > 0 && currentMsgCount === lastMsgCount) {
stablePolls++
if (stablePolls >= STABILITY_POLLS_REQUIRED) break
} else {
stablePolls = 0
lastMsgCount = currentMsgCount
}
}
const messagesResult = await client.session.messages({
path: { id: args.resume },
})
@@ -250,7 +277,8 @@ Use \`background_output\` with task_id="${task.id}" to check progress.`
return `❌ No assistant response found.\n\nSession ID: ${args.resume}`
}
const textParts = lastMessage?.parts?.filter((p) => p.type === "text") ?? []
// Extract text from both "text" and "reasoning" parts (thinking models use "reasoning")
const textParts = lastMessage?.parts?.filter((p) => p.type === "text" || p.type === "reasoning") ?? []
const textContent = textParts.map((p) => p.text ?? "").filter(Boolean).join("\n")
const duration = formatDuration(startTime)
@@ -390,13 +418,13 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id
metadata: { sessionId: sessionID, category: args.category, sync: true },
})
// Use promptAsync to avoid changing main session's active state
// Use fire-and-forget prompt() - awaiting causes JSON parse errors with thinking models
// Note: Don't pass model in body - use agent's configured model instead
let promptError: Error | undefined
await client.session.promptAsync({
client.session.prompt({
path: { id: sessionID },
body: {
agent: agentToUse,
model: categoryModel,
system: systemContent,
tools: {
task: false,
@@ -408,6 +436,9 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id
promptError = error instanceof Error ? error : new Error(String(error))
})
// Small delay to let the prompt start
await new Promise(resolve => setTimeout(resolve, 100))
if (promptError) {
if (toastManager && taskId !== undefined) {
toastManager.removeTask(taskId)
@@ -419,21 +450,63 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id
return `❌ Failed to send prompt: ${errorMessage}\n\nSession ID: ${sessionID}`
}
// Poll for session completion
// Poll for session completion with stability detection
// The session may show as "idle" before messages appear, so we also check message stability
const POLL_INTERVAL_MS = 500
const MAX_POLL_TIME_MS = 10 * 60 * 1000
const MIN_STABILITY_TIME_MS = 10000 // Minimum 10s before accepting completion
const STABILITY_POLLS_REQUIRED = 3
const pollStart = Date.now()
let lastMsgCount = 0
let stablePolls = 0
while (Date.now() - pollStart < MAX_POLL_TIME_MS) {
await new Promise(resolve => setTimeout(resolve, POLL_INTERVAL_MS))
// Check for async errors that may have occurred after the initial 100ms delay
// TypeScript doesn't understand async mutation, so we cast to check
const asyncError = promptError as Error | undefined
if (asyncError) {
if (toastManager && taskId !== undefined) {
toastManager.removeTask(taskId)
}
const errorMessage = asyncError.message
if (errorMessage.includes("agent.name") || errorMessage.includes("undefined")) {
return `❌ Agent "${agentToUse}" not found. Make sure the agent is registered in your opencode.json or provided by a plugin.\n\nSession ID: ${sessionID}`
}
return `❌ Failed to send prompt: ${errorMessage}\n\nSession ID: ${sessionID}`
}
const statusResult = await client.session.status()
const allStatuses = (statusResult.data ?? {}) as Record<string, { type: string }>
const sessionStatus = allStatuses[sessionID]
// Break if session is idle OR no longer in status (completed and removed)
if (!sessionStatus || sessionStatus.type === "idle") {
break
// If session is actively running, reset stability
if (sessionStatus && sessionStatus.type !== "idle") {
stablePolls = 0
lastMsgCount = 0
continue
}
// Session is idle or not in status - check message stability
const elapsed = Date.now() - pollStart
if (elapsed < MIN_STABILITY_TIME_MS) {
continue // Don't accept completion too early
}
// Get current message count
const messagesCheck = await client.session.messages({ path: { id: sessionID } })
const msgs = ((messagesCheck as { data?: unknown }).data ?? messagesCheck) as Array<unknown>
const currentMsgCount = msgs.length
if (currentMsgCount > 0 && currentMsgCount === lastMsgCount) {
stablePolls++
if (stablePolls >= STABILITY_POLLS_REQUIRED) {
break // Messages stable for 3 polls - task complete
}
} else {
stablePolls = 0
lastMsgCount = currentMsgCount
}
}
@@ -459,7 +532,8 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id
return `❌ No assistant response found.\n\nSession ID: ${sessionID}`
}
const textParts = lastMessage?.parts?.filter((p) => p.type === "text") ?? []
// Extract text from both "text" and "reasoning" parts (thinking models use "reasoning")
const textParts = lastMessage?.parts?.filter((p) => p.type === "text" || p.type === "reasoning") ?? []
const textContent = textParts.map((p) => p.text ?? "").filter(Boolean).join("\n")
const duration = formatDuration(startTime)
src/tools/skill/tools.ts
+1 -1
View File
@@ -194,4 +194,4 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
})
}
export const skill = createSkillTool()
export const skill: ToolDefinition = createSkillTool()
src/tools/slashcommand/tools.ts
+1 -1
View File
@@ -249,4 +249,4 @@ export function createSlashcommandTool(options: SlashcommandToolOptions = {}): T
}
// Default instance for backward compatibility (lazy loading)
export const slashcommand = createSlashcommandTool()
export const slashcommand: ToolDefinition = createSlashcommandTool()