From 03871262b24477f88f8753500b86ce90e475bb15 Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Wed, 14 Jan 2026 15:09:32 -0800 Subject: [PATCH 1/7] feat(concurrency): prevent background task races and leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure queue waiters settle once, centralize completion with status guards, and release slots before async work so shutdown and cancellations don’t leak concurrency. Internal hardening only. --- .../background-agent/concurrency.test.ts | 67 ++++++ src/features/background-agent/concurrency.ts | 91 +++++++- src/features/background-agent/manager.test.ts | 176 ++++++++++++++ src/features/background-agent/manager.ts | 219 +++++++++++++----- 4 files changed, 482 insertions(+), 71 deletions(-) diff --git a/src/features/background-agent/concurrency.test.ts b/src/features/background-agent/concurrency.test.ts index 677440e4..c7128fa6 100644 --- a/src/features/background-agent/concurrency.test.ts +++ b/src/features/background-agent/concurrency.test.ts @@ -349,3 +349,70 @@ describe("ConcurrencyManager.acquire/release", () => { await waitPromise }) }) + +describe("ConcurrencyManager.cleanup", () => { + test("cancelWaiters should reject all pending acquires", async () => { + // #given + const config: BackgroundTaskConfig = { defaultConcurrency: 1 } + const manager = new ConcurrencyManager(config) + await manager.acquire("model-a") + + // Queue waiters + const errors: Error[] = [] + const p1 = manager.acquire("model-a").catch(e => errors.push(e)) + const p2 = manager.acquire("model-a").catch(e => errors.push(e)) + + // #when + manager.cancelWaiters("model-a") + await Promise.all([p1, p2]) + + // #then + expect(errors.length).toBe(2) + expect(errors[0].message).toContain("cancelled") + }) + + test("clear should cancel all models and reset state", async () => { + // #given + const config: BackgroundTaskConfig = { defaultConcurrency: 1 } + const manager = new ConcurrencyManager(config) + await manager.acquire("model-a") + await manager.acquire("model-b") + + const errors: Error[] = [] + const p1 = manager.acquire("model-a").catch(e => errors.push(e)) + const p2 = manager.acquire("model-b").catch(e => errors.push(e)) + + // #when + manager.clear() + await Promise.all([p1, p2]) + + // #then + expect(errors.length).toBe(2) + expect(manager.getCount("model-a")).toBe(0) + expect(manager.getCount("model-b")).toBe(0) + }) + + test("getCount and getQueueLength should return correct values", async () => { + // #given + const config: BackgroundTaskConfig = { defaultConcurrency: 2 } + const manager = new ConcurrencyManager(config) + + // #when + await manager.acquire("model-a") + expect(manager.getCount("model-a")).toBe(1) + expect(manager.getQueueLength("model-a")).toBe(0) + + await manager.acquire("model-a") + expect(manager.getCount("model-a")).toBe(2) + + // Queue one more + const p = manager.acquire("model-a").catch(() => {}) + await Promise.resolve() // let it queue + + expect(manager.getQueueLength("model-a")).toBe(1) + + // Cleanup + manager.cancelWaiters("model-a") + await p + }) +}) diff --git a/src/features/background-agent/concurrency.ts b/src/features/background-agent/concurrency.ts index e9d24b8c..1559d088 100644 --- a/src/features/background-agent/concurrency.ts +++ b/src/features/background-agent/concurrency.ts @@ -1,9 +1,21 @@ import type { BackgroundTaskConfig } from "../../config/schema" +/** + * Queue entry with settled-flag pattern to prevent double-resolution. + * + * The settled flag ensures that cancelWaiters() doesn't reject + * an entry that was already resolved by release(). + */ +interface QueueEntry { + resolve: () => void + rawReject: (error: Error) => void + settled: boolean +} + export class ConcurrencyManager { private config?: BackgroundTaskConfig private counts: Map = new Map() - private queues: Map void>> = new Map() + private queues: Map = new Map() constructor(config?: BackgroundTaskConfig) { this.config = config @@ -38,9 +50,20 @@ export class ConcurrencyManager { return } - return new Promise((resolve) => { + return new Promise((resolve, reject) => { const queue = this.queues.get(model) ?? [] - queue.push(resolve) + + const entry: QueueEntry = { + resolve: () => { + if (entry.settled) return + entry.settled = true + resolve() + }, + rawReject: reject, + settled: false, + } + + queue.push(entry) this.queues.set(model, queue) }) } @@ -52,15 +75,63 @@ export class ConcurrencyManager { } const queue = this.queues.get(model) - if (queue && queue.length > 0) { + + // Try to hand off to a waiting entry (skip any settled entries from cancelWaiters) + while (queue && queue.length > 0) { const next = queue.shift()! - this.counts.set(model, this.counts.get(model) ?? 0) - next() - } else { - const current = this.counts.get(model) ?? 0 - if (current > 0) { - this.counts.set(model, current - 1) + if (!next.settled) { + // Hand off the slot to this waiter (count stays the same) + next.resolve() + return } } + + // No handoff occurred - decrement the count to free the slot + const current = this.counts.get(model) ?? 0 + if (current > 0) { + this.counts.set(model, current - 1) + } + } + + /** + * Cancel all waiting acquires for a model. Used during cleanup. + */ + cancelWaiters(model: string): void { + const queue = this.queues.get(model) + if (queue) { + for (const entry of queue) { + if (!entry.settled) { + entry.settled = true + entry.rawReject(new Error(`Concurrency queue cancelled for model: ${model}`)) + } + } + this.queues.delete(model) + } + } + + /** + * Clear all state. Used during manager cleanup/shutdown. + * Cancels all pending waiters. + */ + clear(): void { + for (const [model] of this.queues) { + this.cancelWaiters(model) + } + this.counts.clear() + this.queues.clear() + } + + /** + * Get current count for a model (for testing/debugging) + */ + getCount(model: string): number { + return this.counts.get(model) ?? 0 + } + + /** + * Get queue length for a model (for testing/debugging) + */ + getQueueLength(model: string): number { + return this.queues.get(model)?.length ?? 0 } } diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 0aeedf6b..29e5b32e 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -122,6 +122,10 @@ class MockBackgroundManager { throw new Error(`Task not found for session: ${input.sessionId}`) } + if (existingTask.status === "running") { + return existingTask + } + this.resumeCalls.push({ sessionId: input.sessionId, prompt: input.prompt }) existingTask.status = "running" @@ -572,6 +576,7 @@ describe("BackgroundManager.resume", () => { parentSessionID: "old-parent", description: "original description", agent: "explore", + status: "completed", }) manager.addTask(existingTask) @@ -598,6 +603,7 @@ describe("BackgroundManager.resume", () => { id: "task-a", sessionID: "session-a", parentSessionID: "session-parent", + status: "completed", }) manager.addTask(task) @@ -623,6 +629,7 @@ describe("BackgroundManager.resume", () => { id: "task-a", sessionID: "session-a", parentSessionID: "session-parent", + status: "completed", }) taskWithProgress.progress = { toolCalls: 42, @@ -642,6 +649,29 @@ describe("BackgroundManager.resume", () => { // #then expect(result.progress?.toolCalls).toBe(42) }) + + test("should ignore resume when task is already running", () => { + // #given + const runningTask = createMockTask({ + id: "task-a", + sessionID: "session-a", + parentSessionID: "session-parent", + status: "running", + }) + manager.addTask(runningTask) + + // #when + const result = manager.resume({ + sessionId: "session-a", + prompt: "resume should be ignored", + parentSessionID: "new-parent", + parentMessageID: "new-msg", + }) + + // #then + expect(result.parentSessionID).toBe("session-parent") + expect(manager.resumeCalls).toHaveLength(0) + }) }) describe("LaunchInput.skillContent", () => { @@ -813,3 +843,149 @@ function buildNotificationPromptBody( return body } + +describe("tryCompleteTask pattern - race condition prevention", () => { + /** + * These tests verify the tryCompleteTask pattern behavior + * by simulating the guard logic in a mock implementation. + */ + + test("should prevent double completion when task already completed", () => { + // #given - task already completed + const task: BackgroundTask = { + id: "task-1", + sessionID: "session-1", + parentSessionID: "session-parent", + parentMessageID: "msg-1", + description: "test task", + prompt: "test", + agent: "explore", + status: "completed", + startedAt: new Date(), + completedAt: new Date(), + } + + // #when - try to complete again (simulating tryCompleteTask guard) + const canComplete = task.status === "running" + + // #then - should not allow completion + expect(canComplete).toBe(false) + }) + + test("should allow completion when task is running", () => { + // #given - task is running + const task: BackgroundTask = { + id: "task-1", + sessionID: "session-1", + parentSessionID: "session-parent", + parentMessageID: "msg-1", + description: "test task", + prompt: "test", + agent: "explore", + status: "running", + startedAt: new Date(), + } + + // #when - check if can complete + const canComplete = task.status === "running" + + // #then + expect(canComplete).toBe(true) + }) + + test("should prevent completion when task is cancelled", () => { + // #given - task cancelled by session.deleted + const task: BackgroundTask = { + id: "task-1", + sessionID: "session-1", + parentSessionID: "session-parent", + parentMessageID: "msg-1", + description: "test task", + prompt: "test", + agent: "explore", + status: "cancelled", + startedAt: new Date(), + } + + // #when + const canComplete = task.status === "running" + + // #then + expect(canComplete).toBe(false) + }) + + test("should prevent completion when task errored", () => { + // #given - task errored + const task: BackgroundTask = { + id: "task-1", + sessionID: "session-1", + parentSessionID: "session-parent", + parentMessageID: "msg-1", + description: "test task", + prompt: "test", + agent: "explore", + status: "error", + error: "some error", + startedAt: new Date(), + } + + // #when + const canComplete = task.status === "running" + + // #then + expect(canComplete).toBe(false) + }) +}) + +describe("concurrencyKey management", () => { + test("concurrencyKey should be undefined after release", () => { + // #given - task with concurrency key + const task: BackgroundTask = { + id: "task-1", + sessionID: "session-1", + parentSessionID: "session-parent", + parentMessageID: "msg-1", + description: "test task", + prompt: "test", + agent: "explore", + status: "running", + startedAt: new Date(), + concurrencyKey: "anthropic/claude-sonnet-4-5", + } + + // #when - simulate release pattern (what tryCompleteTask does) + if (task.concurrencyKey) { + // concurrencyManager.release(task.concurrencyKey) would be called + task.concurrencyKey = undefined + } + + // #then + expect(task.concurrencyKey).toBeUndefined() + }) + + test("release should be idempotent with concurrencyKey guard", () => { + // #given - task with key already released + const task: BackgroundTask = { + id: "task-1", + sessionID: "session-1", + parentSessionID: "session-parent", + parentMessageID: "msg-1", + description: "test task", + prompt: "test", + agent: "explore", + status: "completed", + startedAt: new Date(), + concurrencyKey: undefined, // already released + } + + // #when - try to release again (guard pattern) + let releaseCount = 0 + if (task.concurrencyKey) { + releaseCount++ + task.concurrencyKey = undefined + } + + // #then - no double release + expect(releaseCount).toBe(0) + }) +}) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 5860258f..ff0a4975 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -52,6 +52,8 @@ export class BackgroundManager { private directory: string private pollingInterval?: ReturnType private concurrencyManager: ConcurrencyManager + private cleanupRegistered = false + private shutdownTriggered = false constructor(ctx: PluginInput, config?: BackgroundTaskConfig) { this.tasks = new Map() @@ -60,6 +62,7 @@ export class BackgroundManager { this.client = ctx.client this.directory = ctx.directory this.concurrencyManager = new ConcurrencyManager(config) + this.registerProcessCleanup() } async launch(input: LaunchInput): Promise { @@ -186,7 +189,7 @@ export class BackgroundManager { existingTask.completedAt = new Date() if (existingTask.concurrencyKey) { this.concurrencyManager.release(existingTask.concurrencyKey) - existingTask.concurrencyKey = undefined // Prevent double-release + existingTask.concurrencyKey = undefined } this.markForNotification(existingTask) this.notifyParentSession(existingTask).catch(err => { @@ -238,14 +241,20 @@ export class BackgroundManager { * Register an external task (e.g., from sisyphus_task) for notification tracking. * This allows tasks created by external tools to receive the same toast/prompt notifications. */ - registerExternalTask(input: { + async registerExternalTask(input: { taskId: string sessionID: string parentSessionID: string description: string agent?: string parentAgent?: string - }): BackgroundTask { + concurrencyKey?: string + }): Promise { + // Acquire concurrency slot if a key is provided + if (input.concurrencyKey) { + await this.concurrencyManager.acquire(input.concurrencyKey) + } + const task: BackgroundTask = { id: input.taskId, sessionID: input.sessionID, @@ -261,6 +270,7 @@ export class BackgroundManager { lastUpdate: new Date(), }, parentAgent: input.parentAgent, + concurrencyKey: input.concurrencyKey, } this.tasks.set(task.id, task) @@ -283,6 +293,21 @@ export class BackgroundManager { throw new Error(`Task not found for session: ${input.sessionId}`) } + if (existingTask.status === "running") { + log("[background-agent] Resume skipped - task already running:", { + taskId: existingTask.id, + sessionID: existingTask.sessionID, + }) + return existingTask + } + + // Re-acquire concurrency using the agent name as the key (same as launch()). + // Note: existingTask.concurrencyKey is cleared when tasks complete, so we + // derive the key from task.agent which persists through completion. + const concurrencyKey = existingTask.agent + await this.concurrencyManager.acquire(concurrencyKey) + existingTask.concurrencyKey = concurrencyKey + existingTask.status = "running" existingTask.completedAt = undefined existingTask.error = undefined @@ -344,11 +369,12 @@ export class BackgroundManager { const errorMessage = error instanceof Error ? error.message : String(error) existingTask.error = errorMessage existingTask.completedAt = new Date() - // Release concurrency on resume error (matches launch error handler) - if (existingTask.concurrencyKey) { - this.concurrencyManager.release(existingTask.concurrencyKey) - existingTask.concurrencyKey = undefined // Prevent double-release - } + + // Release concurrency on error to prevent slot leaks + if (existingTask.concurrencyKey) { + this.concurrencyManager.release(existingTask.concurrencyKey) + existingTask.concurrencyKey = undefined + } this.markForNotification(existingTask) this.notifyParentSession(existingTask).catch(err => { log("[background-agent] Failed to notify on resume error:", err) @@ -417,29 +443,31 @@ export class BackgroundManager { // Edge guard: Verify session has actual assistant output before completing this.validateSessionHasOutput(sessionID).then(async (hasValidOutput) => { + // Re-check status after async operation (could have been completed by polling) + if (task.status !== "running") { + log("[background-agent] Task status changed during validation, skipping:", { taskId: task.id, status: task.status }) + return + } + if (!hasValidOutput) { log("[background-agent] Session.idle but no valid output yet, waiting:", task.id) return } const hasIncompleteTodos = await this.checkSessionTodos(sessionID) + + // Re-check status after async operation again + if (task.status !== "running") { + log("[background-agent] Task status changed during todo check, skipping:", { taskId: task.id, status: task.status }) + return + } + if (hasIncompleteTodos) { log("[background-agent] Task has incomplete todos, waiting for todo-continuation:", task.id) return } - task.status = "completed" - task.completedAt = new Date() - // Release concurrency immediately on completion - if (task.concurrencyKey) { - this.concurrencyManager.release(task.concurrencyKey) - task.concurrencyKey = undefined // Prevent double-release - } - // Clean up pendingByParent to prevent stale entries - this.cleanupPendingByParent(task) - this.markForNotification(task) - await this.notifyParentSession(task) - log("[background-agent] Task completed via session.idle event:", task.id) + await this.tryCompleteTask(task, "session.idle event") }).catch(err => { log("[background-agent] Error in session.idle handler:", err) }) @@ -459,10 +487,10 @@ export class BackgroundManager { task.error = "Session deleted" } - if (task.concurrencyKey) { - this.concurrencyManager.release(task.concurrencyKey) - task.concurrencyKey = undefined // Prevent double-release - } + if (task.concurrencyKey) { + this.concurrencyManager.release(task.concurrencyKey) + task.concurrencyKey = undefined + } // Clean up pendingByParent to prevent stale entries this.cleanupPendingByParent(task) this.tasks.delete(task.id) @@ -587,11 +615,25 @@ export class BackgroundManager { } } -cleanup(): void { - this.stopPolling() - this.tasks.clear() - this.notifications.clear() - this.pendingByParent.clear() + private registerProcessCleanup(): void { + if (this.cleanupRegistered) return + this.cleanupRegistered = true + + const cleanup = () => { + try { + this.shutdown() + } catch (error) { + log("[background-agent] Error during shutdown cleanup:", error) + } + } + + registerProcessSignal("SIGINT", cleanup) + registerProcessSignal("SIGTERM", cleanup) + if (process.platform === "win32") { + registerProcessSignal("SIGBREAK", cleanup) + } + process.on("beforeExit", cleanup) + process.on("exit", cleanup) } /** @@ -608,12 +650,44 @@ cleanup(): void { return Array.from(this.tasks.values()).filter(t => t.status !== "running") } -private async notifyParentSession(task: BackgroundTask): Promise { + /** + * Safely complete a task with race condition protection. + * Returns true if task was successfully completed, false if already completed by another path. + */ + private async tryCompleteTask(task: BackgroundTask, source: string): Promise { + // Guard: Check if task is still running (could have been completed by another path) + if (task.status !== "running") { + log("[background-agent] Task already completed, skipping:", { taskId: task.id, status: task.status, source }) + return false + } + + // Atomically mark as completed to prevent race conditions + task.status = "completed" + task.completedAt = new Date() + + // Release concurrency BEFORE any async operations to prevent slot leaks if (task.concurrencyKey) { this.concurrencyManager.release(task.concurrencyKey) task.concurrencyKey = undefined } + this.markForNotification(task) + + try { + await this.notifyParentSession(task) + log(`[background-agent] Task completed via ${source}:`, task.id) + } catch (err) { + log("[background-agent] Error in notifyParentSession:", { taskId: task.id, error: err }) + // Concurrency already released, notification failed but task is complete + } + + return true + } + + private async notifyParentSession(task: BackgroundTask): Promise { + // Note: Callers must release concurrency before calling this method + // to ensure slots are freed even if notification fails + const duration = this.formatDuration(task.startedAt, task.completedAt) log("[background-agent] notifyParentSession called for task:", task.id) @@ -715,10 +789,12 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea const taskId = task.id setTimeout(() => { - // Concurrency already released at completion - just cleanup notifications and task - this.clearNotificationsForTask(taskId) - this.tasks.delete(taskId) - log("[background-agent] Removed completed task from memory:", taskId) + // Guard: Only delete if task still exists (could have been deleted by session.deleted event) + if (this.tasks.has(taskId)) { + this.clearNotificationsForTask(taskId) + this.tasks.delete(taskId) + log("[background-agent] Removed completed task from memory:", taskId) + } }, 5 * 60 * 1000) } @@ -755,7 +831,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea task.completedAt = new Date() if (task.concurrencyKey) { this.concurrencyManager.release(task.concurrencyKey) - task.concurrencyKey = undefined // Prevent double-release + task.concurrencyKey = undefined } // Clean up pendingByParent to prevent stale entries this.cleanupPendingByParent(task) @@ -791,7 +867,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea for (const task of this.tasks.values()) { if (task.status !== "running") continue -try { + try { const sessionStatus = allStatuses[task.sessionID] // Don't skip if session not in status - fall through to message-based detection @@ -803,24 +879,16 @@ try { continue } + // Re-check status after async operation + if (task.status !== "running") continue + const hasIncompleteTodos = await this.checkSessionTodos(task.sessionID) if (hasIncompleteTodos) { log("[background-agent] Task has incomplete todos via polling, waiting:", task.id) continue } - task.status = "completed" - task.completedAt = new Date() - // Release concurrency immediately on completion - if (task.concurrencyKey) { - this.concurrencyManager.release(task.concurrencyKey) - task.concurrencyKey = undefined // Prevent double-release - } - // Clean up pendingByParent to prevent stale entries - this.cleanupPendingByParent(task) - this.markForNotification(task) - await this.notifyParentSession(task) - log("[background-agent] Task completed via polling:", task.id) + await this.tryCompleteTask(task, "polling (idle status)") continue } @@ -860,7 +928,7 @@ try { 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() } @@ -880,20 +948,12 @@ if (lastMessage) { continue } + // Re-check status after async operation + if (task.status !== "running") continue + const hasIncompleteTodos = await this.checkSessionTodos(task.sessionID) if (!hasIncompleteTodos) { - task.status = "completed" - task.completedAt = new Date() - // Release concurrency immediately on completion - if (task.concurrencyKey) { - this.concurrencyManager.release(task.concurrencyKey) - task.concurrencyKey = undefined // Prevent double-release - } - // Clean up pendingByParent to prevent stale entries - this.cleanupPendingByParent(task) - this.markForNotification(task) - await this.notifyParentSession(task) - log("[background-agent] Task completed via stability detection:", task.id) + await this.tryCompleteTask(task, "stability detection") continue } } @@ -912,6 +972,43 @@ if (lastMessage) { this.stopPolling() } } + + /** + * Shutdown the manager gracefully. + * Cancels all pending concurrency waiters and clears timers. + * Should be called when the plugin is unloaded. + */ + shutdown(): void { + if (this.shutdownTriggered) return + this.shutdownTriggered = true + log("[background-agent] Shutting down BackgroundManager") + this.stopPolling() + + // Release concurrency for all running tasks first + for (const task of this.tasks.values()) { + if (task.concurrencyKey) { + this.concurrencyManager.release(task.concurrencyKey) + task.concurrencyKey = undefined + } + } + + // Then clear all state (cancels any remaining waiters) + this.concurrencyManager.clear() + this.tasks.clear() + this.notifications.clear() + this.pendingByParent.clear() + log("[background-agent] Shutdown complete") + } +} + +function registerProcessSignal( + signal: NodeJS.Signals, + handler: () => void +): void { + process.on(signal, () => { + handler() + process.exit(0) + }) } function getMessageDir(sessionID: string): string | null { From c1246f61d1db3723e50d7602d5a56c8478b9c3a7 Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Wed, 14 Jan 2026 22:40:14 -0800 Subject: [PATCH 2/7] feat(background-agent): add concurrency group field Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- src/features/background-agent/types.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/features/background-agent/types.ts b/src/features/background-agent/types.ts index 8c384211..795ca89b 100644 --- a/src/features/background-agent/types.ts +++ b/src/features/background-agent/types.ts @@ -28,10 +28,13 @@ export interface BackgroundTask { progress?: TaskProgress parentModel?: { providerID: string; modelID: string } model?: { providerID: string; modelID: string; variant?: string } - /** Agent name used for concurrency tracking */ + /** Active concurrency slot key */ concurrencyKey?: string + /** Persistent key for re-acquiring concurrency on resume */ + concurrencyGroup?: 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 */ From 4ac0fa7bb08263d9a03905c32d22335f69b8bc7a Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Wed, 14 Jan 2026 22:40:16 -0800 Subject: [PATCH 3/7] fix(background-agent): preserve external concurrency keys Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- src/features/background-agent/manager.test.ts | 254 ++++++++++-------- src/features/background-agent/manager.ts | 42 ++- 2 files changed, 181 insertions(+), 115 deletions(-) diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 29e5b32e..dda7e1df 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -1,5 +1,10 @@ import { describe, test, expect, beforeEach } from "bun:test" +import { afterEach } from "bun:test" +import type { PluginInput } from "@opencode-ai/plugin" import type { BackgroundTask, ResumeInput } from "./types" +import { BackgroundManager } from "./manager" +import { ConcurrencyManager } from "./concurrency" + const TASK_TTL_MS = 30 * 60 * 1000 @@ -156,6 +161,32 @@ function createMockTask(overrides: Partial & { id: string; sessi } } +function createBackgroundManager(): BackgroundManager { + const client = { + session: { + prompt: async () => ({}), + }, + } + return new BackgroundManager({ client, directory: "C:\\tmp" } as unknown as PluginInput) +} + +function getConcurrencyManager(manager: BackgroundManager): ConcurrencyManager { + return (manager as unknown as { concurrencyManager: ConcurrencyManager }).concurrencyManager +} + +function getTaskMap(manager: BackgroundManager): Map { + return (manager as unknown as { tasks: Map }).tasks +} + +function stubNotifyParentSession(manager: BackgroundManager): void { + (manager as unknown as { notifyParentSession: (task: BackgroundTask) => Promise }).notifyParentSession = async () => {} +} + +async function tryCompleteTaskForTest(manager: BackgroundManager, task: BackgroundTask): Promise { + return (manager as unknown as { tryCompleteTask: (task: BackgroundTask, source: string) => Promise }).tryCompleteTask(task, "test") +} + + describe("BackgroundManager.getAllDescendantTasks", () => { let manager: MockBackgroundManager @@ -844,36 +875,25 @@ function buildNotificationPromptBody( return body } -describe("tryCompleteTask pattern - race condition prevention", () => { - /** - * These tests verify the tryCompleteTask pattern behavior - * by simulating the guard logic in a mock implementation. - */ +describe("BackgroundManager.tryCompleteTask", () => { + let manager: BackgroundManager - test("should prevent double completion when task already completed", () => { - // #given - task already completed - const task: BackgroundTask = { - id: "task-1", - sessionID: "session-1", - parentSessionID: "session-parent", - parentMessageID: "msg-1", - description: "test task", - prompt: "test", - agent: "explore", - status: "completed", - startedAt: new Date(), - completedAt: new Date(), - } - - // #when - try to complete again (simulating tryCompleteTask guard) - const canComplete = task.status === "running" - - // #then - should not allow completion - expect(canComplete).toBe(false) + beforeEach(() => { + // #given + manager = createBackgroundManager() + stubNotifyParentSession(manager) }) - test("should allow completion when task is running", () => { - // #given - task is running + afterEach(() => { + manager.shutdown() + }) + + test("should release concurrency and clear key on completion", async () => { + // #given + const concurrencyKey = "anthropic/claude-opus-4-5" + const concurrencyManager = getConcurrencyManager(manager) + await concurrencyManager.acquire(concurrencyKey) + const task: BackgroundTask = { id: "task-1", sessionID: "session-1", @@ -884,87 +904,25 @@ describe("tryCompleteTask pattern - race condition prevention", () => { agent: "explore", status: "running", startedAt: new Date(), - } - - // #when - check if can complete - const canComplete = task.status === "running" - - // #then - expect(canComplete).toBe(true) - }) - - test("should prevent completion when task is cancelled", () => { - // #given - task cancelled by session.deleted - const task: BackgroundTask = { - id: "task-1", - sessionID: "session-1", - parentSessionID: "session-parent", - parentMessageID: "msg-1", - description: "test task", - prompt: "test", - agent: "explore", - status: "cancelled", - startedAt: new Date(), + concurrencyKey, } // #when - const canComplete = task.status === "running" - - // #then - expect(canComplete).toBe(false) - }) - - test("should prevent completion when task errored", () => { - // #given - task errored - const task: BackgroundTask = { - id: "task-1", - sessionID: "session-1", - parentSessionID: "session-parent", - parentMessageID: "msg-1", - description: "test task", - prompt: "test", - agent: "explore", - status: "error", - error: "some error", - startedAt: new Date(), - } - - // #when - const canComplete = task.status === "running" - - // #then - expect(canComplete).toBe(false) - }) -}) - -describe("concurrencyKey management", () => { - test("concurrencyKey should be undefined after release", () => { - // #given - task with concurrency key - const task: BackgroundTask = { - id: "task-1", - sessionID: "session-1", - parentSessionID: "session-parent", - parentMessageID: "msg-1", - description: "test task", - prompt: "test", - agent: "explore", - status: "running", - startedAt: new Date(), - concurrencyKey: "anthropic/claude-sonnet-4-5", - } - - // #when - simulate release pattern (what tryCompleteTask does) - if (task.concurrencyKey) { - // concurrencyManager.release(task.concurrencyKey) would be called - task.concurrencyKey = undefined - } + const completed = await tryCompleteTaskForTest(manager, task) // #then + expect(completed).toBe(true) + expect(task.status).toBe("completed") expect(task.concurrencyKey).toBeUndefined() + expect(concurrencyManager.getCount(concurrencyKey)).toBe(0) }) - test("release should be idempotent with concurrencyKey guard", () => { - // #given - task with key already released + test("should prevent double completion and double release", async () => { + // #given + const concurrencyKey = "anthropic/claude-opus-4-5" + const concurrencyManager = getConcurrencyManager(manager) + await concurrencyManager.acquire(concurrencyKey) + const task: BackgroundTask = { id: "task-1", sessionID: "session-1", @@ -973,19 +931,95 @@ describe("concurrencyKey management", () => { description: "test task", prompt: "test", agent: "explore", - status: "completed", + status: "running", startedAt: new Date(), - concurrencyKey: undefined, // already released + concurrencyKey, } - // #when - try to release again (guard pattern) - let releaseCount = 0 - if (task.concurrencyKey) { - releaseCount++ - task.concurrencyKey = undefined - } + // #when + await tryCompleteTaskForTest(manager, task) + const secondAttempt = await tryCompleteTaskForTest(manager, task) - // #then - no double release - expect(releaseCount).toBe(0) + // #then + expect(secondAttempt).toBe(false) + expect(task.status).toBe("completed") + expect(concurrencyManager.getCount(concurrencyKey)).toBe(0) }) }) + +describe("BackgroundManager.registerExternalTask", () => { + let manager: BackgroundManager + + beforeEach(() => { + // #given + manager = createBackgroundManager() + stubNotifyParentSession(manager) + }) + + afterEach(() => { + manager.shutdown() + }) + + test("should not double acquire on duplicate registration", async () => { + // #given + const input = { + taskId: "task-1", + sessionID: "session-1", + parentSessionID: "parent-session", + description: "external task", + agent: "sisyphus_task", + concurrencyKey: "external-key", + } + + // #when + await manager.registerExternalTask(input) + await manager.registerExternalTask(input) + + // #then + const concurrencyManager = getConcurrencyManager(manager) + expect(concurrencyManager.getCount("external-key")).toBe(1) + expect(getTaskMap(manager).size).toBe(1) + }) +}) + +describe("BackgroundManager.resume concurrency key", () => { + let manager: BackgroundManager + + beforeEach(() => { + // #given + manager = createBackgroundManager() + stubNotifyParentSession(manager) + }) + + afterEach(() => { + manager.shutdown() + }) + + test("should re-acquire using external task concurrency key", async () => { + // #given + const task = await manager.registerExternalTask({ + taskId: "task-1", + sessionID: "session-1", + parentSessionID: "parent-session", + description: "external task", + agent: "sisyphus_task", + concurrencyKey: "external-key", + }) + + await tryCompleteTaskForTest(manager, task) + + // #when + await manager.resume({ + sessionId: "session-1", + prompt: "resume", + parentSessionID: "parent-session-2", + parentMessageID: "msg-2", + }) + + // #then + const concurrencyManager = getConcurrencyManager(manager) + expect(concurrencyManager.getCount("external-key")).toBe(1) + expect(task.concurrencyKey).toBe("external-key") + }) +}) + diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index ff0a4975..20185099 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -129,8 +129,10 @@ export class BackgroundManager { parentAgent: input.parentAgent, model: input.model, concurrencyKey, + concurrencyGroup: concurrencyKey, } + this.tasks.set(task.id, task) this.startPolling() @@ -189,8 +191,9 @@ export class BackgroundManager { existingTask.completedAt = new Date() if (existingTask.concurrencyKey) { this.concurrencyManager.release(existingTask.concurrencyKey) - existingTask.concurrencyKey = undefined + existingTask.concurrencyKey = undefined } + this.markForNotification(existingTask) this.notifyParentSession(existingTask).catch(err => { log("[background-agent] Failed to notify on error:", err) @@ -250,6 +253,33 @@ export class BackgroundManager { parentAgent?: string concurrencyKey?: string }): Promise { + const existingTask = this.tasks.get(input.taskId) + if (existingTask) { + if (input.parentSessionID !== existingTask.parentSessionID) { + existingTask.parentSessionID = input.parentSessionID + } + if (input.parentAgent !== undefined) { + existingTask.parentAgent = input.parentAgent + } + if (!existingTask.concurrencyGroup) { + existingTask.concurrencyGroup = input.concurrencyKey ?? existingTask.agent + } + + subagentSessions.add(existingTask.sessionID) + this.startPolling() + + // Track for batched notifications (external tasks need tracking too) + const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set() + pending.add(existingTask.id) + this.pendingByParent.set(input.parentSessionID, pending) + + log("[background-agent] External task already registered:", { taskId: existingTask.id, sessionID: existingTask.sessionID }) + + return existingTask + } + + const concurrencyGroup = input.concurrencyKey ?? input.agent ?? "sisyphus_task" + // Acquire concurrency slot if a key is provided if (input.concurrencyKey) { await this.concurrencyManager.acquire(input.concurrencyKey) @@ -271,12 +301,14 @@ export class BackgroundManager { }, parentAgent: input.parentAgent, concurrencyKey: input.concurrencyKey, + concurrencyGroup, } this.tasks.set(task.id, task) 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) @@ -301,12 +333,12 @@ export class BackgroundManager { return existingTask } - // Re-acquire concurrency using the agent name as the key (same as launch()). - // Note: existingTask.concurrencyKey is cleared when tasks complete, so we - // derive the key from task.agent which persists through completion. - const concurrencyKey = existingTask.agent + // Re-acquire concurrency using the persisted concurrency group + const concurrencyKey = existingTask.concurrencyGroup ?? existingTask.agent await this.concurrencyManager.acquire(concurrencyKey) existingTask.concurrencyKey = concurrencyKey + existingTask.concurrencyGroup = concurrencyKey + existingTask.status = "running" existingTask.completedAt = undefined From 7050d447cd31c4f4e401794c56acad4832549599 Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Wed, 14 Jan 2026 23:11:38 -0800 Subject: [PATCH 4/7] feat(background-agent): implement process cleanup for BackgroundManager Add functionality to manage process cleanup by registering and unregistering signal listeners. This ensures that BackgroundManager instances properly shut down and remove their listeners on process exit. Introduce tests to verify listener removal after shutdown. --- src/features/background-agent/manager.test.ts | 39 +++++++++- src/features/background-agent/manager.ts | 77 ++++++++++++++----- 2 files changed, 95 insertions(+), 21 deletions(-) diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index dda7e1df..ab6e8acc 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -1,5 +1,6 @@ import { describe, test, expect, beforeEach } from "bun:test" import { afterEach } from "bun:test" +import { tmpdir } from "node:os" import type { PluginInput } from "@opencode-ai/plugin" import type { BackgroundTask, ResumeInput } from "./types" import { BackgroundManager } from "./manager" @@ -167,7 +168,7 @@ function createBackgroundManager(): BackgroundManager { prompt: async () => ({}), }, } - return new BackgroundManager({ client, directory: "C:\\tmp" } as unknown as PluginInput) + return new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) } function getConcurrencyManager(manager: BackgroundManager): ConcurrencyManager { @@ -186,6 +187,18 @@ async function tryCompleteTaskForTest(manager: BackgroundManager, task: Backgrou return (manager as unknown as { tryCompleteTask: (task: BackgroundTask, source: string) => Promise }).tryCompleteTask(task, "test") } +function getCleanupSignals(): Array { + const signals: Array = ["SIGINT", "SIGTERM", "beforeExit", "exit"] + if (process.platform === "win32") { + signals.push("SIGBREAK") + } + return signals +} + +function getListenerCounts(signals: Array): Record { + return Object.fromEntries(signals.map((signal) => [signal, process.listenerCount(signal)])) +} + describe("BackgroundManager.getAllDescendantTasks", () => { let manager: MockBackgroundManager @@ -1023,3 +1036,27 @@ describe("BackgroundManager.resume concurrency key", () => { }) }) +describe("BackgroundManager process cleanup", () => { + test("should remove listeners after last shutdown", () => { + // #given + const signals = getCleanupSignals() + const baseline = getListenerCounts(signals) + const managerA = createBackgroundManager() + const managerB = createBackgroundManager() + + // #when + const afterCreate = getListenerCounts(signals) + managerA.shutdown() + const afterFirstShutdown = getListenerCounts(signals) + managerB.shutdown() + const afterSecondShutdown = getListenerCounts(signals) + + // #then + for (const signal of signals) { + expect(afterCreate[signal]).toBe(baseline[signal] + 1) + expect(afterFirstShutdown[signal]).toBe(baseline[signal] + 1) + expect(afterSecondShutdown[signal]).toBe(baseline[signal]) + } + }) +}) + diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 20185099..e1d6b8ff 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -18,8 +18,11 @@ import { join } from "node:path" 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 ProcessCleanupEvent = NodeJS.Signals | "beforeExit" | "exit" + type OpencodeClient = PluginInput["client"] + interface MessagePartInfo { sessionID?: string type?: string @@ -45,6 +48,10 @@ interface Todo { } export class BackgroundManager { + private static cleanupManagers = new Set() + private static cleanupRegistered = false + private static cleanupHandlers = new Map void>() + private tasks: Map private notifications: Map private pendingByParent: Map> // Track pending tasks per parent for batching @@ -52,9 +59,9 @@ export class BackgroundManager { private directory: string private pollingInterval?: ReturnType private concurrencyManager: ConcurrencyManager - private cleanupRegistered = false private shutdownTriggered = false + constructor(ctx: PluginInput, config?: BackgroundTaskConfig) { this.tasks = new Map() this.notifications = new Map() @@ -648,26 +655,48 @@ export class BackgroundManager { } private registerProcessCleanup(): void { - if (this.cleanupRegistered) return - this.cleanupRegistered = true + BackgroundManager.cleanupManagers.add(this) - const cleanup = () => { - try { - this.shutdown() - } catch (error) { - log("[background-agent] Error during shutdown cleanup:", error) + if (BackgroundManager.cleanupRegistered) return + BackgroundManager.cleanupRegistered = true + + const cleanupAll = () => { + for (const manager of BackgroundManager.cleanupManagers) { + try { + manager.shutdown() + } catch (error) { + log("[background-agent] Error during shutdown cleanup:", error) + } } } - registerProcessSignal("SIGINT", cleanup) - registerProcessSignal("SIGTERM", cleanup) - if (process.platform === "win32") { - registerProcessSignal("SIGBREAK", cleanup) + const registerSignal = (signal: ProcessCleanupEvent, exitAfter: boolean): void => { + const listener = registerProcessSignal(signal, cleanupAll, exitAfter) + BackgroundManager.cleanupHandlers.set(signal, listener) } - process.on("beforeExit", cleanup) - process.on("exit", cleanup) + + registerSignal("SIGINT", true) + registerSignal("SIGTERM", true) + if (process.platform === "win32") { + registerSignal("SIGBREAK", true) + } + registerSignal("beforeExit", false) + registerSignal("exit", false) } + private unregisterProcessCleanup(): void { + BackgroundManager.cleanupManagers.delete(this) + + if (BackgroundManager.cleanupManagers.size > 0) return + + for (const [signal, listener] of BackgroundManager.cleanupHandlers.entries()) { + process.off(signal, listener) + } + BackgroundManager.cleanupHandlers.clear() + BackgroundManager.cleanupRegistered = false + } + + /** * Get all running tasks (for compaction hook) */ @@ -1029,20 +1058,28 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea this.tasks.clear() this.notifications.clear() this.pendingByParent.clear() + this.unregisterProcessCleanup() log("[background-agent] Shutdown complete") + } } function registerProcessSignal( - signal: NodeJS.Signals, - handler: () => void -): void { - process.on(signal, () => { + signal: ProcessCleanupEvent, + handler: () => void, + exitAfter: boolean +): () => void { + const listener = () => { handler() - process.exit(0) - }) + if (exitAfter) { + process.exit(0) + } + } + process.on(signal, listener) + return listener } + function getMessageDir(sessionID: string): string | null { if (!existsSync(MESSAGE_STORAGE)) return null From 7168c2d904a1af12674b6c2000fcb1acdd015833 Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Wed, 14 Jan 2026 23:51:19 -0800 Subject: [PATCH 5/7] fix(background-agent): prevent stale entries in pending notifications Update BackgroundManager to track batched notifications only for running tasks. Implement cleanup for completed or cancelled tasks to avoid stale entries in pending notifications. Enhance logging to include task status for better debugging. --- src/features/background-agent/manager.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index e1d6b8ff..f166b476 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -275,12 +275,18 @@ export class BackgroundManager { subagentSessions.add(existingTask.sessionID) this.startPolling() - // Track for batched notifications (external tasks need tracking too) - const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set() - pending.add(existingTask.id) - this.pendingByParent.set(input.parentSessionID, pending) + // Track for batched notifications only if task is still running + // Don't add stale entries for completed tasks + if (existingTask.status === "running") { + const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set() + pending.add(existingTask.id) + this.pendingByParent.set(input.parentSessionID, pending) + } else { + // Don't re-add completed/cancelled tasks; clean any stale entry + this.cleanupPendingByParent(existingTask) + } - log("[background-agent] External task already registered:", { taskId: existingTask.id, sessionID: existingTask.sessionID }) + log("[background-agent] External task already registered:", { taskId: existingTask.id, sessionID: existingTask.sessionID, status: existingTask.status }) return existingTask } From b5bd837025c718ab3ab9d40363ca756a7b5958cd Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Thu, 15 Jan 2026 00:16:35 -0800 Subject: [PATCH 6/7] fix(background-agent): improve parent session ID handling in task management Enhance the BackgroundManager to properly clean up pending tasks when the parent session ID changes. This prevents stale entries in the pending notifications and ensures that the cleanup process is only executed when necessary, improving overall task management reliability. --- assets/oh-my-opencode.schema.json | 1 + src/features/background-agent/manager.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/assets/oh-my-opencode.schema.json b/assets/oh-my-opencode.schema.json index b215a7c8..308b177c 100644 --- a/assets/oh-my-opencode.schema.json +++ b/assets/oh-my-opencode.schema.json @@ -77,6 +77,7 @@ "claude-code-hooks", "auto-slash-command", "edit-error-recovery", + "sisyphus-task-retry", "prometheus-md-only", "start-work", "sisyphus-orchestrator" diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index f166b476..40870190 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -262,7 +262,11 @@ export class BackgroundManager { }): Promise { const existingTask = this.tasks.get(input.taskId) if (existingTask) { - if (input.parentSessionID !== existingTask.parentSessionID) { + // P2 fix: Clean up old parent's pending set BEFORE changing parent + // Otherwise cleanupPendingByParent would use the new parent ID + const parentChanged = input.parentSessionID !== existingTask.parentSessionID + if (parentChanged) { + this.cleanupPendingByParent(existingTask) // Clean from OLD parent existingTask.parentSessionID = input.parentSessionID } if (input.parentAgent !== undefined) { @@ -281,8 +285,8 @@ export class BackgroundManager { const pending = this.pendingByParent.get(input.parentSessionID) ?? new Set() pending.add(existingTask.id) this.pendingByParent.set(input.parentSessionID, pending) - } else { - // Don't re-add completed/cancelled tasks; clean any stale entry + } else if (!parentChanged) { + // Only clean up if parent didn't change (already cleaned above if it did) this.cleanupPendingByParent(existingTask) } From 8e2410f1a041e4ae6be2bf4bae00f83bbb1eb5e8 Mon Sep 17 00:00:00 2001 From: Jeremy Gollehon Date: Thu, 15 Jan 2026 10:53:08 -0800 Subject: [PATCH 7/7] refactor(background-agent): rename registerExternalTask to trackTask Update BackgroundManager to rename the method for tracking external tasks, improving clarity and consistency in task management. Adjust related tests to reflect the new method name. --- src/features/background-agent/manager.test.ts | 8 ++++---- src/features/background-agent/manager.ts | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index ab6e8acc..304cf99d 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -960,7 +960,7 @@ describe("BackgroundManager.tryCompleteTask", () => { }) }) -describe("BackgroundManager.registerExternalTask", () => { +describe("BackgroundManager.trackTask", () => { let manager: BackgroundManager beforeEach(() => { @@ -985,8 +985,8 @@ describe("BackgroundManager.registerExternalTask", () => { } // #when - await manager.registerExternalTask(input) - await manager.registerExternalTask(input) + await manager.trackTask(input) + await manager.trackTask(input) // #then const concurrencyManager = getConcurrencyManager(manager) @@ -1010,7 +1010,7 @@ describe("BackgroundManager.resume concurrency key", () => { test("should re-acquire using external task concurrency key", async () => { // #given - const task = await manager.registerExternalTask({ + const task = await manager.trackTask({ taskId: "task-1", sessionID: "session-1", parentSessionID: "parent-session", diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 40870190..6959f7b4 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -248,10 +248,10 @@ export class BackgroundManager { } /** - * Register an external task (e.g., from sisyphus_task) for notification tracking. - * This allows tasks created by external tools to receive the same toast/prompt notifications. + * Track a task created elsewhere (e.g., from sisyphus_task) for notification tracking. + * This allows tasks created by other tools to receive the same toast/prompt notifications. */ - async registerExternalTask(input: { + async trackTask(input: { taskId: string sessionID: string parentSessionID: string @@ -419,11 +419,11 @@ export class BackgroundManager { existingTask.error = errorMessage existingTask.completedAt = new Date() - // Release concurrency on error to prevent slot leaks - if (existingTask.concurrencyKey) { - this.concurrencyManager.release(existingTask.concurrencyKey) - existingTask.concurrencyKey = undefined - } + // Release concurrency on error to prevent slot leaks + if (existingTask.concurrencyKey) { + this.concurrencyManager.release(existingTask.concurrencyKey) + existingTask.concurrencyKey = undefined + } this.markForNotification(existingTask) this.notifyParentSession(existingTask).catch(err => { log("[background-agent] Failed to notify on resume error:", err)