Merge pull request #2135 from code-yeongyu/fix/issue-2115-background-output-block
fix(background-task): make background_output block=true actually wait for task completion
This commit is contained in:
commit
8f7ed2988a
@ -0,0 +1,112 @@
|
|||||||
|
/// <reference types="bun-types" />
|
||||||
|
|
||||||
|
import { describe, expect, test } from "bun:test"
|
||||||
|
import type { ToolContext } from "@opencode-ai/plugin/tool"
|
||||||
|
import type { BackgroundTask } from "../../features/background-agent"
|
||||||
|
import type { BackgroundOutputClient, BackgroundOutputManager } from "./clients"
|
||||||
|
import { createBackgroundOutput } from "./create-background-output"
|
||||||
|
|
||||||
|
const projectDir = "/Users/yeongyu/local-workspaces/oh-my-opencode"
|
||||||
|
|
||||||
|
const mockContext = {
|
||||||
|
sessionID: "test-session",
|
||||||
|
messageID: "test-message",
|
||||||
|
agent: "test-agent",
|
||||||
|
directory: projectDir,
|
||||||
|
worktree: projectDir,
|
||||||
|
abort: new AbortController().signal,
|
||||||
|
metadata: () => {},
|
||||||
|
ask: async () => {},
|
||||||
|
} as unknown as ToolContext
|
||||||
|
|
||||||
|
function createTask(overrides: Partial<BackgroundTask> = {}): BackgroundTask {
|
||||||
|
return {
|
||||||
|
id: "task-1",
|
||||||
|
sessionID: "ses-1",
|
||||||
|
parentSessionID: "main-1",
|
||||||
|
parentMessageID: "msg-1",
|
||||||
|
description: "background task",
|
||||||
|
prompt: "do work",
|
||||||
|
agent: "test-agent",
|
||||||
|
status: "running",
|
||||||
|
...overrides,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMockClient(): BackgroundOutputClient {
|
||||||
|
return {
|
||||||
|
session: {
|
||||||
|
messages: async () => ({ data: [] }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("createBackgroundOutput block=true polling", () => {
|
||||||
|
test("returns terminal error output when task fails during blocking wait", async () => {
|
||||||
|
// #given
|
||||||
|
let pollCount = 0
|
||||||
|
const task = createTask({ status: "running" })
|
||||||
|
const manager: BackgroundOutputManager = {
|
||||||
|
getTask: (id: string) => {
|
||||||
|
if (id !== task.id) return undefined
|
||||||
|
|
||||||
|
pollCount += 1
|
||||||
|
if (pollCount >= 2) {
|
||||||
|
task.status = "error"
|
||||||
|
task.error = "task failed"
|
||||||
|
}
|
||||||
|
|
||||||
|
return task
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const tool = createBackgroundOutput(manager, createMockClient())
|
||||||
|
|
||||||
|
// #when
|
||||||
|
const output = await tool.execute(
|
||||||
|
{
|
||||||
|
task_id: task.id,
|
||||||
|
block: true,
|
||||||
|
timeout: 3000,
|
||||||
|
full_session: false,
|
||||||
|
},
|
||||||
|
mockContext
|
||||||
|
)
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(pollCount).toBeGreaterThanOrEqual(2)
|
||||||
|
expect(output).toContain("Status | **error**")
|
||||||
|
expect(output).not.toContain("Timed out waiting")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("returns latest output with timeout note when task stays running", async () => {
|
||||||
|
// #given
|
||||||
|
let pollCount = 0
|
||||||
|
const task = createTask({ status: "running" })
|
||||||
|
const manager: BackgroundOutputManager = {
|
||||||
|
getTask: (id: string) => {
|
||||||
|
if (id !== task.id) return undefined
|
||||||
|
pollCount += 1
|
||||||
|
return task
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const tool = createBackgroundOutput(manager, createMockClient())
|
||||||
|
|
||||||
|
// #when
|
||||||
|
const output = await tool.execute(
|
||||||
|
{
|
||||||
|
task_id: task.id,
|
||||||
|
block: true,
|
||||||
|
timeout: 10,
|
||||||
|
},
|
||||||
|
mockContext
|
||||||
|
)
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(pollCount).toBeGreaterThanOrEqual(2)
|
||||||
|
expect(output).toContain("# Full Session Output")
|
||||||
|
expect(output).toContain("Timed out waiting")
|
||||||
|
expect(output).toContain("still running")
|
||||||
|
})
|
||||||
|
})
|
||||||
@ -33,6 +33,14 @@ function formatResolvedTitle(task: BackgroundTask): string {
|
|||||||
return `${label} - ${task.description}`
|
return `${label} - ${task.description}`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isTaskActiveStatus(status: BackgroundTask["status"]): boolean {
|
||||||
|
return status === "pending" || status === "running"
|
||||||
|
}
|
||||||
|
|
||||||
|
function appendTimeoutNote(output: string, timeoutMs: number): string {
|
||||||
|
return `${output}\n\n> **Timed out waiting** after ${timeoutMs}ms. Task is still running; showing latest available output.`
|
||||||
|
}
|
||||||
|
|
||||||
export function createBackgroundOutput(manager: BackgroundOutputManager, client: BackgroundOutputClient): ToolDefinition {
|
export function createBackgroundOutput(manager: BackgroundOutputManager, client: BackgroundOutputClient): ToolDefinition {
|
||||||
return tool({
|
return tool({
|
||||||
description: BACKGROUND_OUTPUT_DESCRIPTION,
|
description: BACKGROUND_OUTPUT_DESCRIPTION,
|
||||||
@ -83,7 +91,9 @@ export function createBackgroundOutput(manager: BackgroundOutputManager, client:
|
|||||||
|
|
||||||
let resolvedTask = task
|
let resolvedTask = task
|
||||||
|
|
||||||
if (shouldBlock && (task.status === "pending" || task.status === "running")) {
|
let didTimeoutWhileActive = false
|
||||||
|
|
||||||
|
if (shouldBlock && isTaskActiveStatus(task.status)) {
|
||||||
const startTime = Date.now()
|
const startTime = Date.now()
|
||||||
while (Date.now() - startTime < timeoutMs) {
|
while (Date.now() - startTime < timeoutMs) {
|
||||||
await delay(1000)
|
await delay(1000)
|
||||||
@ -93,30 +103,39 @@ export function createBackgroundOutput(manager: BackgroundOutputManager, client:
|
|||||||
return `Task was deleted: ${args.task_id}`
|
return `Task was deleted: ${args.task_id}`
|
||||||
}
|
}
|
||||||
|
|
||||||
if (currentTask.status !== "pending" && currentTask.status !== "running") {
|
resolvedTask = currentTask
|
||||||
resolvedTask = currentTask
|
|
||||||
|
if (!isTaskActiveStatus(currentTask.status)) {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const finalCheck = manager.getTask(args.task_id)
|
if (isTaskActiveStatus(resolvedTask.status)) {
|
||||||
if (finalCheck) {
|
const finalCheck = manager.getTask(args.task_id)
|
||||||
resolvedTask = finalCheck
|
if (finalCheck) {
|
||||||
|
resolvedTask = finalCheck
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (isTaskActiveStatus(resolvedTask.status)) {
|
||||||
|
didTimeoutWhileActive = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const isActive = resolvedTask.status === "pending" || resolvedTask.status === "running"
|
const isActive = isTaskActiveStatus(resolvedTask.status)
|
||||||
const includeThinking = isActive || (args.include_thinking ?? false)
|
const includeThinking = isActive || (args.include_thinking ?? false)
|
||||||
const includeToolResults = isActive || (args.include_tool_results ?? false)
|
const includeToolResults = isActive || (args.include_tool_results ?? false)
|
||||||
|
|
||||||
if (fullSession) {
|
if (fullSession) {
|
||||||
return await formatFullSession(resolvedTask, client, {
|
const output = await formatFullSession(resolvedTask, client, {
|
||||||
includeThinking,
|
includeThinking,
|
||||||
messageLimit: args.message_limit,
|
messageLimit: args.message_limit,
|
||||||
sinceMessageId: args.since_message_id,
|
sinceMessageId: args.since_message_id,
|
||||||
includeToolResults,
|
includeToolResults,
|
||||||
thinkingMaxChars: args.thinking_max_chars,
|
thinkingMaxChars: args.thinking_max_chars,
|
||||||
})
|
})
|
||||||
|
|
||||||
|
return didTimeoutWhileActive ? appendTimeoutNote(output, timeoutMs) : output
|
||||||
}
|
}
|
||||||
|
|
||||||
if (resolvedTask.status === "completed") {
|
if (resolvedTask.status === "completed") {
|
||||||
@ -127,7 +146,8 @@ export function createBackgroundOutput(manager: BackgroundOutputManager, client:
|
|||||||
return formatTaskStatus(resolvedTask)
|
return formatTaskStatus(resolvedTask)
|
||||||
}
|
}
|
||||||
|
|
||||||
return formatTaskStatus(resolvedTask)
|
const statusOutput = formatTaskStatus(resolvedTask)
|
||||||
|
return didTimeoutWhileActive ? appendTimeoutNote(statusOutput, timeoutMs) : statusOutput
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
return `Error getting output: ${error instanceof Error ? error.message : String(error)}`
|
return `Error getting output: ${error instanceof Error ? error.message : String(error)}`
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user