fix(background-agent): prevent false task completion on status API outage
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -4,8 +4,25 @@ import { describe, test, expect, mock } from "bun:test"
|
||||
import { tmpdir } from "node:os"
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import { BackgroundManager } from "./manager"
|
||||
import { MIN_SESSION_GONE_POLLS } from "./session-existence"
|
||||
import type { BackgroundTask } from "./types"
|
||||
|
||||
function createPluginContext(client: object): PluginInput {
|
||||
const directory = tmpdir()
|
||||
return {
|
||||
project: {
|
||||
id: "test-project",
|
||||
worktree: directory,
|
||||
time: { created: Date.now() },
|
||||
},
|
||||
directory,
|
||||
worktree: directory,
|
||||
serverUrl: new URL("http://localhost:4096"),
|
||||
$: {} as PluginInput["$"],
|
||||
client: client as PluginInput["client"],
|
||||
}
|
||||
}
|
||||
|
||||
function createManagerWithStatus(statusImpl: () => Promise<{ data: Record<string, { type: string }> }>): BackgroundManager {
|
||||
const client = {
|
||||
session: {
|
||||
@@ -18,7 +35,7 @@ function createManagerWithStatus(statusImpl: () => Promise<{ data: Record<string
|
||||
},
|
||||
}
|
||||
|
||||
return new BackgroundManager({ pluginContext: { client, directory: tmpdir() } as unknown as PluginInput })
|
||||
return new BackgroundManager({ pluginContext: createPluginContext(client) })
|
||||
}
|
||||
|
||||
describe("BackgroundManager polling overlap", () => {
|
||||
@@ -42,9 +59,9 @@ describe("BackgroundManager polling overlap", () => {
|
||||
})
|
||||
|
||||
//#when
|
||||
const firstPoll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks()
|
||||
const firstPoll = manager["pollRunningTasks"]()
|
||||
await Promise.resolve()
|
||||
const secondPoll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks()
|
||||
const secondPoll = manager["pollRunningTasks"]()
|
||||
releaseStatus?.()
|
||||
await Promise.all([firstPoll, secondPoll])
|
||||
manager.shutdown()
|
||||
@@ -72,8 +89,7 @@ function createRunningTask(sessionId: string): BackgroundTask {
|
||||
}
|
||||
|
||||
function injectTask(manager: BackgroundManager, task: BackgroundTask): void {
|
||||
const tasks = (manager as unknown as { tasks: Map<string, BackgroundTask> }).tasks
|
||||
tasks.set(task.id, task)
|
||||
manager["tasks"].set(task.id, task)
|
||||
}
|
||||
|
||||
function createManagerWithClient(clientOverrides: Record<string, unknown> = {}): BackgroundManager {
|
||||
@@ -98,7 +114,7 @@ function createManagerWithClient(clientOverrides: Record<string, unknown> = {}):
|
||||
},
|
||||
}
|
||||
return new BackgroundManager(
|
||||
{ pluginContext: { client, directory: tmpdir() } as unknown as PluginInput, config: undefined, enableParentSessionNotifications: false },
|
||||
{ pluginContext: createPluginContext(client), config: undefined, enableParentSessionNotifications: false },
|
||||
)
|
||||
}
|
||||
|
||||
@@ -151,7 +167,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
@@ -184,6 +200,62 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
expect(task.consecutiveMissedPolls).toBe(1)
|
||||
expect(getSession).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test("#when status polling is unavailable #then it does not complete or increment missed polls", async () => {
|
||||
const cases: Array<{ name: string; status?: (() => Promise<{ data: Record<string, { type: string }> }>) | undefined }> = [
|
||||
{ name: "missing status method", status: undefined },
|
||||
{ name: "throwing status method", status: async () => { throw new Error("status unavailable") } },
|
||||
]
|
||||
|
||||
for (const testCase of cases) {
|
||||
//#given
|
||||
let abortCallCount = 0
|
||||
const manager = createManagerWithClient({
|
||||
status: testCase.status,
|
||||
abort: async () => {
|
||||
abortCallCount += 1
|
||||
return {}
|
||||
},
|
||||
})
|
||||
const task = createRunningTask(`ses-${testCase.name.replace(/ /g, "-")}`)
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = manager["pollRunningTasks"]
|
||||
for (let count = 0; count < MIN_SESSION_GONE_POLLS + 1; count += 1) {
|
||||
await poll.call(manager)
|
||||
}
|
||||
|
||||
//#then
|
||||
expect(task.status).toBe("running")
|
||||
expect(task.completedAt).toBeUndefined()
|
||||
expect(task.error).toBeUndefined()
|
||||
expect(task.consecutiveMissedPolls ?? 0).toBe(0)
|
||||
expect(abortCallCount).toBe(0)
|
||||
|
||||
await manager.shutdown()
|
||||
}
|
||||
})
|
||||
|
||||
test("#when reliable status polling omits the session #then it completes through the session-gone path", async () => {
|
||||
//#given
|
||||
const manager = createManagerWithClient({
|
||||
status: async () => ({ data: {} }),
|
||||
})
|
||||
const task = createRunningTask("ses-reliably-gone")
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = manager["pollRunningTasks"]
|
||||
for (let count = 0; count < MIN_SESSION_GONE_POLLS; count += 1) {
|
||||
await poll.call(manager)
|
||||
}
|
||||
await manager.shutdown()
|
||||
|
||||
//#then
|
||||
expect(task.status).toBe("completed")
|
||||
expect(task.completedAt).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a running task whose session status is idle", () => {
|
||||
@@ -196,7 +268,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
@@ -228,7 +300,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
})
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
@@ -265,7 +337,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
})
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
@@ -285,7 +357,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
@@ -304,7 +376,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
@@ -322,7 +394,7 @@ describe("BackgroundManager pollRunningTasks", () => {
|
||||
injectTask(manager, task)
|
||||
|
||||
//#when
|
||||
const poll = (manager as unknown as { pollRunningTasks: () => Promise<void> }).pollRunningTasks
|
||||
const poll = manager["pollRunningTasks"]
|
||||
await poll.call(manager)
|
||||
manager.shutdown()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user