refactor: migrate delegate_task to task tool with metadata fixes
- Rename delegate_task tool to task across codebase (100 files) - Update model references: claude-opus-4-6 → 4-5, gpt-5.3-codex → 5.2-codex - Add tool-metadata-store to restore metadata overwritten by fromPlugin() - Add session ID polling for BackgroundManager task sessions - Await async ctx.metadata() calls in tool executors - Add ses_ prefix guard to getMessageDir for performance - Harden BackgroundManager with idle deferral and error handling - Fix duplicate task key in sisyphus-junior test object literals - Fix unawaited showOutputToUser in ast_grep_replace - Fix background=true → run_in_background=true in ultrawork prompt - Fix duplicate task/task references in docs and comments
This commit is contained in:
@@ -1,8 +1,9 @@
|
||||
import { describe, test, expect, beforeEach } from "bun:test"
|
||||
import { afterEach } from "bun:test"
|
||||
declare const require: (name: string) => any
|
||||
const { describe, test, expect, beforeEach, afterEach } = require("bun:test")
|
||||
import { tmpdir } from "node:os"
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import type { BackgroundTask, ResumeInput } from "./types"
|
||||
import { MIN_IDLE_TIME_MS } from "./constants"
|
||||
import { BackgroundManager } from "./manager"
|
||||
import { ConcurrencyManager } from "./concurrency"
|
||||
|
||||
@@ -1088,6 +1089,34 @@ describe("BackgroundManager.tryCompleteTask", () => {
|
||||
// #then
|
||||
expect(abortedSessionIDs).toEqual(["session-1"])
|
||||
})
|
||||
|
||||
test("should clean pendingByParent even when notifyParentSession throws", async () => {
|
||||
// given
|
||||
;(manager as unknown as { notifyParentSession: () => Promise<void> }).notifyParentSession = async () => {
|
||||
throw new Error("notify failed")
|
||||
}
|
||||
|
||||
const task: BackgroundTask = {
|
||||
id: "task-pending-cleanup",
|
||||
sessionID: "session-pending-cleanup",
|
||||
parentSessionID: "parent-pending-cleanup",
|
||||
parentMessageID: "msg-1",
|
||||
description: "pending cleanup task",
|
||||
prompt: "test",
|
||||
agent: "explore",
|
||||
status: "running",
|
||||
startedAt: new Date(),
|
||||
}
|
||||
getTaskMap(manager).set(task.id, task)
|
||||
getPendingByParent(manager).set(task.parentSessionID, new Set([task.id]))
|
||||
|
||||
// when
|
||||
await tryCompleteTaskForTest(manager, task)
|
||||
|
||||
// then
|
||||
expect(task.status).toBe("completed")
|
||||
expect(getPendingByParent(manager).get(task.parentSessionID)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe("BackgroundManager.trackTask", () => {
|
||||
@@ -1110,7 +1139,7 @@ describe("BackgroundManager.trackTask", () => {
|
||||
sessionID: "session-1",
|
||||
parentSessionID: "parent-session",
|
||||
description: "external task",
|
||||
agent: "delegate_task",
|
||||
agent: "task",
|
||||
concurrencyKey: "external-key",
|
||||
}
|
||||
|
||||
@@ -1145,7 +1174,7 @@ describe("BackgroundManager.resume concurrency key", () => {
|
||||
sessionID: "session-1",
|
||||
parentSessionID: "parent-session",
|
||||
description: "external task",
|
||||
agent: "delegate_task",
|
||||
agent: "task",
|
||||
concurrencyKey: "external-key",
|
||||
})
|
||||
|
||||
@@ -2408,3 +2437,179 @@ describe("BackgroundManager.completionTimers - Memory Leak Fix", () => {
|
||||
expect(completionTimers.size).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe("BackgroundManager.handleEvent - early session.idle deferral", () => {
|
||||
test("should defer and retry when session.idle fires before MIN_IDLE_TIME_MS", async () => {
|
||||
//#given - a running task started less than MIN_IDLE_TIME_MS ago
|
||||
const sessionID = "session-early-idle"
|
||||
const messagesCalls: string[] = []
|
||||
const realDateNow = Date.now
|
||||
const baseNow = realDateNow()
|
||||
|
||||
const client = {
|
||||
session: {
|
||||
prompt: async () => ({}),
|
||||
abort: async () => ({}),
|
||||
messages: async (args: { path: { id: string } }) => {
|
||||
messagesCalls.push(args.path.id)
|
||||
return {
|
||||
data: [
|
||||
{
|
||||
info: { role: "assistant" },
|
||||
parts: [{ type: "text", text: "ok" }],
|
||||
},
|
||||
],
|
||||
}
|
||||
},
|
||||
todo: async () => ({ data: [] }),
|
||||
},
|
||||
}
|
||||
|
||||
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||
stubNotifyParentSession(manager)
|
||||
|
||||
const remainingMs = 1200
|
||||
const task: BackgroundTask = {
|
||||
id: "task-early-idle",
|
||||
sessionID,
|
||||
parentSessionID: "parent-session",
|
||||
parentMessageID: "msg-1",
|
||||
description: "early idle task",
|
||||
prompt: "test",
|
||||
agent: "explore",
|
||||
status: "running",
|
||||
startedAt: new Date(baseNow),
|
||||
}
|
||||
|
||||
getTaskMap(manager).set(task.id, task)
|
||||
|
||||
//#when - session.idle fires
|
||||
try {
|
||||
Date.now = () => baseNow + (MIN_IDLE_TIME_MS - 100)
|
||||
manager.handleEvent({ type: "session.idle", properties: { sessionID } })
|
||||
|
||||
// Advance time so deferred callback (if any) sees elapsed >= MIN_IDLE_TIME_MS
|
||||
Date.now = () => baseNow + (MIN_IDLE_TIME_MS + 10)
|
||||
|
||||
//#then - idle should be deferred (not dropped), and task should eventually complete
|
||||
expect(task.status).toBe("running")
|
||||
await new Promise((resolve) => setTimeout(resolve, 220))
|
||||
expect(task.status).toBe("completed")
|
||||
expect(messagesCalls).toEqual([sessionID])
|
||||
} finally {
|
||||
Date.now = realDateNow
|
||||
manager.shutdown()
|
||||
}
|
||||
})
|
||||
|
||||
test("should not defer when session.idle fires after MIN_IDLE_TIME_MS", async () => {
|
||||
//#given - a running task started more than MIN_IDLE_TIME_MS ago
|
||||
const sessionID = "session-late-idle"
|
||||
const client = {
|
||||
session: {
|
||||
prompt: async () => ({}),
|
||||
abort: async () => ({}),
|
||||
messages: async () => ({
|
||||
data: [
|
||||
{
|
||||
info: { role: "assistant" },
|
||||
parts: [{ type: "text", text: "ok" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
todo: async () => ({ data: [] }),
|
||||
},
|
||||
}
|
||||
|
||||
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||
stubNotifyParentSession(manager)
|
||||
|
||||
const task: BackgroundTask = {
|
||||
id: "task-late-idle",
|
||||
sessionID,
|
||||
parentSessionID: "parent-session",
|
||||
parentMessageID: "msg-1",
|
||||
description: "late idle task",
|
||||
prompt: "test",
|
||||
agent: "explore",
|
||||
status: "running",
|
||||
startedAt: new Date(Date.now() - (MIN_IDLE_TIME_MS + 10)),
|
||||
}
|
||||
|
||||
getTaskMap(manager).set(task.id, task)
|
||||
|
||||
//#when
|
||||
manager.handleEvent({ type: "session.idle", properties: { sessionID } })
|
||||
|
||||
//#then - should be processed immediately
|
||||
await new Promise((resolve) => setTimeout(resolve, 10))
|
||||
expect(task.status).toBe("completed")
|
||||
|
||||
manager.shutdown()
|
||||
})
|
||||
|
||||
test("should not process deferred idle if task already completed by other means", async () => {
|
||||
//#given - a running task
|
||||
const sessionID = "session-deferred-noop"
|
||||
let messagesCallCount = 0
|
||||
const realDateNow = Date.now
|
||||
const baseNow = realDateNow()
|
||||
|
||||
const client = {
|
||||
session: {
|
||||
prompt: async () => ({}),
|
||||
abort: async () => ({}),
|
||||
messages: async () => {
|
||||
messagesCallCount += 1
|
||||
return {
|
||||
data: [
|
||||
{
|
||||
info: { role: "assistant" },
|
||||
parts: [{ type: "text", text: "ok" }],
|
||||
},
|
||||
],
|
||||
}
|
||||
},
|
||||
todo: async () => ({ data: [] }),
|
||||
},
|
||||
}
|
||||
|
||||
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||
stubNotifyParentSession(manager)
|
||||
|
||||
const remainingMs = 120
|
||||
const task: BackgroundTask = {
|
||||
id: "task-deferred-noop",
|
||||
sessionID,
|
||||
parentSessionID: "parent-session",
|
||||
parentMessageID: "msg-1",
|
||||
description: "deferred noop task",
|
||||
prompt: "test",
|
||||
agent: "explore",
|
||||
status: "running",
|
||||
startedAt: new Date(baseNow),
|
||||
}
|
||||
getTaskMap(manager).set(task.id, task)
|
||||
|
||||
//#when - session.idle fires early, then task completes via another path before defer timer
|
||||
try {
|
||||
Date.now = () => baseNow + (MIN_IDLE_TIME_MS - remainingMs)
|
||||
manager.handleEvent({ type: "session.idle", properties: { sessionID } })
|
||||
expect(messagesCallCount).toBe(0)
|
||||
|
||||
await tryCompleteTaskForTest(manager, task)
|
||||
expect(task.status).toBe("completed")
|
||||
|
||||
// Advance time so deferred callback (if any) sees elapsed >= MIN_IDLE_TIME_MS
|
||||
Date.now = () => baseNow + (MIN_IDLE_TIME_MS + 10)
|
||||
|
||||
//#then - deferred callback should be a no-op
|
||||
await new Promise((resolve) => setTimeout(resolve, remainingMs + 80))
|
||||
expect(task.status).toBe("completed")
|
||||
expect(messagesCallCount).toBe(0)
|
||||
} finally {
|
||||
Date.now = realDateNow
|
||||
manager.shutdown()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -88,6 +88,7 @@ export class BackgroundManager {
|
||||
private queuesByKey: Map<string, QueueItem[]> = new Map()
|
||||
private processingKeys: Set<string> = new Set()
|
||||
private completionTimers: Map<string, ReturnType<typeof setTimeout>> = new Map()
|
||||
private idleDeferralTimers: Map<string, ReturnType<typeof setTimeout>> = new Map()
|
||||
|
||||
constructor(
|
||||
ctx: PluginInput,
|
||||
@@ -328,7 +329,6 @@ export class BackgroundManager {
|
||||
tools: {
|
||||
...getAgentToolRestrictions(input.agent),
|
||||
task: false,
|
||||
delegate_task: false,
|
||||
call_omo_agent: true,
|
||||
question: false,
|
||||
},
|
||||
@@ -357,6 +357,7 @@ export class BackgroundManager {
|
||||
}).catch(() => {})
|
||||
|
||||
this.markForNotification(existingTask)
|
||||
this.cleanupPendingByParent(existingTask)
|
||||
this.notifyParentSession(existingTask).catch(err => {
|
||||
log("[background-agent] Failed to notify on error:", err)
|
||||
})
|
||||
@@ -410,7 +411,7 @@ export class BackgroundManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* Track a task created elsewhere (e.g., from delegate_task) for notification tracking.
|
||||
* Track a task created elsewhere (e.g., from task) for notification tracking.
|
||||
* This allows tasks created by other tools to receive the same toast/prompt notifications.
|
||||
*/
|
||||
async trackTask(input: {
|
||||
@@ -458,7 +459,7 @@ export class BackgroundManager {
|
||||
return existingTask
|
||||
}
|
||||
|
||||
const concurrencyGroup = input.concurrencyKey ?? input.agent ?? "delegate_task"
|
||||
const concurrencyGroup = input.concurrencyKey ?? input.agent ?? "task"
|
||||
|
||||
// Acquire concurrency slot if a key is provided
|
||||
if (input.concurrencyKey) {
|
||||
@@ -472,7 +473,7 @@ export class BackgroundManager {
|
||||
parentMessageID: "",
|
||||
description: input.description,
|
||||
prompt: "",
|
||||
agent: input.agent || "delegate_task",
|
||||
agent: input.agent || "task",
|
||||
status: "running",
|
||||
startedAt: new Date(),
|
||||
progress: {
|
||||
@@ -587,7 +588,6 @@ export class BackgroundManager {
|
||||
tools: {
|
||||
...getAgentToolRestrictions(existingTask.agent),
|
||||
task: false,
|
||||
delegate_task: false,
|
||||
call_omo_agent: true,
|
||||
question: false,
|
||||
},
|
||||
@@ -614,6 +614,7 @@ export class BackgroundManager {
|
||||
}
|
||||
|
||||
this.markForNotification(existingTask)
|
||||
this.cleanupPendingByParent(existingTask)
|
||||
this.notifyParentSession(existingTask).catch(err => {
|
||||
log("[background-agent] Failed to notify on resume error:", err)
|
||||
})
|
||||
@@ -651,6 +652,13 @@ export class BackgroundManager {
|
||||
const task = this.findBySession(sessionID)
|
||||
if (!task) return
|
||||
|
||||
// Clear any pending idle deferral timer since the task is still active
|
||||
const existingTimer = this.idleDeferralTimers.get(task.id)
|
||||
if (existingTimer) {
|
||||
clearTimeout(existingTimer)
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
}
|
||||
|
||||
if (partInfo?.type === "tool" || partInfo?.tool) {
|
||||
if (!task.progress) {
|
||||
task.progress = {
|
||||
@@ -677,7 +685,17 @@ export class BackgroundManager {
|
||||
// Edge guard: Require minimum elapsed time (5 seconds) before accepting idle
|
||||
const elapsedMs = Date.now() - startedAt.getTime()
|
||||
if (elapsedMs < MIN_IDLE_TIME_MS) {
|
||||
log("[background-agent] Ignoring early session.idle, elapsed:", { elapsedMs, taskId: task.id })
|
||||
const remainingMs = MIN_IDLE_TIME_MS - elapsedMs
|
||||
if (!this.idleDeferralTimers.has(task.id)) {
|
||||
log("[background-agent] Deferring early session.idle:", { elapsedMs, remainingMs, taskId: task.id })
|
||||
const timer = setTimeout(() => {
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
this.handleEvent({ type: "session.idle", properties: { sessionID } })
|
||||
}, remainingMs)
|
||||
this.idleDeferralTimers.set(task.id, timer)
|
||||
} else {
|
||||
log("[background-agent] session.idle already deferred:", { elapsedMs, taskId: task.id })
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
@@ -736,6 +754,12 @@ export class BackgroundManager {
|
||||
clearTimeout(existingTimer)
|
||||
this.completionTimers.delete(task.id)
|
||||
}
|
||||
|
||||
const idleTimer = this.idleDeferralTimers.get(task.id)
|
||||
if (idleTimer) {
|
||||
clearTimeout(idleTimer)
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
}
|
||||
this.cleanupPendingByParent(task)
|
||||
this.tasks.delete(task.id)
|
||||
this.clearNotificationsForTask(task.id)
|
||||
@@ -890,6 +914,12 @@ export class BackgroundManager {
|
||||
this.completionTimers.delete(task.id)
|
||||
}
|
||||
|
||||
const idleTimer = this.idleDeferralTimers.get(task.id)
|
||||
if (idleTimer) {
|
||||
clearTimeout(idleTimer)
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
}
|
||||
|
||||
this.cleanupPendingByParent(task)
|
||||
|
||||
if (abortSession && task.sessionID) {
|
||||
@@ -1025,6 +1055,15 @@ export class BackgroundManager {
|
||||
|
||||
this.markForNotification(task)
|
||||
|
||||
// Ensure pending tracking is cleaned up even if notification fails
|
||||
this.cleanupPendingByParent(task)
|
||||
|
||||
const idleTimer = this.idleDeferralTimers.get(task.id)
|
||||
if (idleTimer) {
|
||||
clearTimeout(idleTimer)
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
}
|
||||
|
||||
if (task.sessionID) {
|
||||
this.client.session.abort({
|
||||
path: { id: task.sessionID },
|
||||
@@ -1511,6 +1550,11 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
|
||||
}
|
||||
this.completionTimers.clear()
|
||||
|
||||
for (const timer of this.idleDeferralTimers.values()) {
|
||||
clearTimeout(timer)
|
||||
}
|
||||
this.idleDeferralTimers.clear()
|
||||
|
||||
this.concurrencyManager.clear()
|
||||
this.tasks.clear()
|
||||
this.notifications.clear()
|
||||
|
||||
@@ -146,7 +146,6 @@ export async function startTask(
|
||||
tools: {
|
||||
...getAgentToolRestrictions(input.agent),
|
||||
task: false,
|
||||
delegate_task: false,
|
||||
call_omo_agent: true,
|
||||
question: false,
|
||||
},
|
||||
@@ -231,7 +230,6 @@ export async function resumeTask(
|
||||
tools: {
|
||||
...getAgentToolRestrictions(task.agent),
|
||||
task: false,
|
||||
delegate_task: false,
|
||||
call_omo_agent: true,
|
||||
question: false,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user