- BackgroundManager.shutdown() now aborts all running child sessions via client.session.abort() before clearing state, preventing orphaned opencode processes when parent exits - Add onShutdown callback to BackgroundManager constructor, used to trigger TmuxSessionManager.cleanup() on process exit signals - Interactive bash session hook now aborts tracked subagent opencode sessions when killing tmux sessions (defense-in-depth) - Add 4 tests verifying shutdown abort behavior and callback invocation Closes #1240
This commit is contained in:
parent
6d50fbe563
commit
b4973954e3
@ -170,6 +170,7 @@ function createBackgroundManager(): BackgroundManager {
|
|||||||
const client = {
|
const client = {
|
||||||
session: {
|
session: {
|
||||||
prompt: async () => ({}),
|
prompt: async () => ({}),
|
||||||
|
abort: async () => ({}),
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
return new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
return new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||||
@ -1053,6 +1054,7 @@ describe("BackgroundManager.resume model persistence", () => {
|
|||||||
promptCalls.push(args)
|
promptCalls.push(args)
|
||||||
return {}
|
return {}
|
||||||
},
|
},
|
||||||
|
abort: async () => ({}),
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||||
@ -1926,3 +1928,162 @@ describe("BackgroundManager.checkAndInterruptStaleTasks", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("BackgroundManager.shutdown session abort", () => {
|
||||||
|
test("should call session.abort for all running tasks during shutdown", () => {
|
||||||
|
// #given
|
||||||
|
const abortedSessionIDs: string[] = []
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
prompt: async () => ({}),
|
||||||
|
abort: async (args: { path: { id: string } }) => {
|
||||||
|
abortedSessionIDs.push(args.path.id)
|
||||||
|
return {}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||||
|
|
||||||
|
const task1: BackgroundTask = {
|
||||||
|
id: "task-1",
|
||||||
|
sessionID: "session-1",
|
||||||
|
parentSessionID: "parent-1",
|
||||||
|
parentMessageID: "msg-1",
|
||||||
|
description: "Running task 1",
|
||||||
|
prompt: "Test",
|
||||||
|
agent: "test-agent",
|
||||||
|
status: "running",
|
||||||
|
startedAt: new Date(),
|
||||||
|
}
|
||||||
|
const task2: BackgroundTask = {
|
||||||
|
id: "task-2",
|
||||||
|
sessionID: "session-2",
|
||||||
|
parentSessionID: "parent-2",
|
||||||
|
parentMessageID: "msg-2",
|
||||||
|
description: "Running task 2",
|
||||||
|
prompt: "Test",
|
||||||
|
agent: "test-agent",
|
||||||
|
status: "running",
|
||||||
|
startedAt: new Date(),
|
||||||
|
}
|
||||||
|
|
||||||
|
getTaskMap(manager).set(task1.id, task1)
|
||||||
|
getTaskMap(manager).set(task2.id, task2)
|
||||||
|
|
||||||
|
// #when
|
||||||
|
manager.shutdown()
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(abortedSessionIDs).toContain("session-1")
|
||||||
|
expect(abortedSessionIDs).toContain("session-2")
|
||||||
|
expect(abortedSessionIDs).toHaveLength(2)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should not call session.abort for completed or cancelled tasks", () => {
|
||||||
|
// #given
|
||||||
|
const abortedSessionIDs: string[] = []
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
prompt: async () => ({}),
|
||||||
|
abort: async (args: { path: { id: string } }) => {
|
||||||
|
abortedSessionIDs.push(args.path.id)
|
||||||
|
return {}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||||
|
|
||||||
|
const completedTask: BackgroundTask = {
|
||||||
|
id: "task-completed",
|
||||||
|
sessionID: "session-completed",
|
||||||
|
parentSessionID: "parent-1",
|
||||||
|
parentMessageID: "msg-1",
|
||||||
|
description: "Completed task",
|
||||||
|
prompt: "Test",
|
||||||
|
agent: "test-agent",
|
||||||
|
status: "completed",
|
||||||
|
startedAt: new Date(),
|
||||||
|
completedAt: new Date(),
|
||||||
|
}
|
||||||
|
const cancelledTask: BackgroundTask = {
|
||||||
|
id: "task-cancelled",
|
||||||
|
sessionID: "session-cancelled",
|
||||||
|
parentSessionID: "parent-2",
|
||||||
|
parentMessageID: "msg-2",
|
||||||
|
description: "Cancelled task",
|
||||||
|
prompt: "Test",
|
||||||
|
agent: "test-agent",
|
||||||
|
status: "cancelled",
|
||||||
|
startedAt: new Date(),
|
||||||
|
completedAt: new Date(),
|
||||||
|
}
|
||||||
|
const pendingTask: BackgroundTask = {
|
||||||
|
id: "task-pending",
|
||||||
|
parentSessionID: "parent-3",
|
||||||
|
parentMessageID: "msg-3",
|
||||||
|
description: "Pending task",
|
||||||
|
prompt: "Test",
|
||||||
|
agent: "test-agent",
|
||||||
|
status: "pending",
|
||||||
|
queuedAt: new Date(),
|
||||||
|
}
|
||||||
|
|
||||||
|
getTaskMap(manager).set(completedTask.id, completedTask)
|
||||||
|
getTaskMap(manager).set(cancelledTask.id, cancelledTask)
|
||||||
|
getTaskMap(manager).set(pendingTask.id, pendingTask)
|
||||||
|
|
||||||
|
// #when
|
||||||
|
manager.shutdown()
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(abortedSessionIDs).toHaveLength(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should call onShutdown callback during shutdown", () => {
|
||||||
|
// #given
|
||||||
|
let shutdownCalled = false
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
prompt: async () => ({}),
|
||||||
|
abort: async () => ({}),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const manager = new BackgroundManager(
|
||||||
|
{ client, directory: tmpdir() } as unknown as PluginInput,
|
||||||
|
undefined,
|
||||||
|
{
|
||||||
|
onShutdown: () => {
|
||||||
|
shutdownCalled = true
|
||||||
|
},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
// #when
|
||||||
|
manager.shutdown()
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(shutdownCalled).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should not throw when onShutdown callback throws", () => {
|
||||||
|
// #given
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
prompt: async () => ({}),
|
||||||
|
abort: async () => ({}),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const manager = new BackgroundManager(
|
||||||
|
{ client, directory: tmpdir() } as unknown as PluginInput,
|
||||||
|
undefined,
|
||||||
|
{
|
||||||
|
onShutdown: () => {
|
||||||
|
throw new Error("cleanup failed")
|
||||||
|
},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
// #when / #then
|
||||||
|
expect(() => manager.shutdown()).not.toThrow()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
|||||||
@ -79,6 +79,7 @@ export class BackgroundManager {
|
|||||||
private config?: BackgroundTaskConfig
|
private config?: BackgroundTaskConfig
|
||||||
private tmuxEnabled: boolean
|
private tmuxEnabled: boolean
|
||||||
private onSubagentSessionCreated?: OnSubagentSessionCreated
|
private onSubagentSessionCreated?: OnSubagentSessionCreated
|
||||||
|
private onShutdown?: () => void
|
||||||
|
|
||||||
private queuesByKey: Map<string, QueueItem[]> = new Map()
|
private queuesByKey: Map<string, QueueItem[]> = new Map()
|
||||||
private processingKeys: Set<string> = new Set()
|
private processingKeys: Set<string> = new Set()
|
||||||
@ -89,6 +90,7 @@ export class BackgroundManager {
|
|||||||
options?: {
|
options?: {
|
||||||
tmuxConfig?: TmuxConfig
|
tmuxConfig?: TmuxConfig
|
||||||
onSubagentSessionCreated?: OnSubagentSessionCreated
|
onSubagentSessionCreated?: OnSubagentSessionCreated
|
||||||
|
onShutdown?: () => void
|
||||||
}
|
}
|
||||||
) {
|
) {
|
||||||
this.tasks = new Map()
|
this.tasks = new Map()
|
||||||
@ -100,6 +102,7 @@ export class BackgroundManager {
|
|||||||
this.config = config
|
this.config = config
|
||||||
this.tmuxEnabled = options?.tmuxConfig?.enabled ?? false
|
this.tmuxEnabled = options?.tmuxConfig?.enabled ?? false
|
||||||
this.onSubagentSessionCreated = options?.onSubagentSessionCreated
|
this.onSubagentSessionCreated = options?.onSubagentSessionCreated
|
||||||
|
this.onShutdown = options?.onShutdown
|
||||||
this.registerProcessCleanup()
|
this.registerProcessCleanup()
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1346,7 +1349,25 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
|
|||||||
log("[background-agent] Shutting down BackgroundManager")
|
log("[background-agent] Shutting down BackgroundManager")
|
||||||
this.stopPolling()
|
this.stopPolling()
|
||||||
|
|
||||||
// Release concurrency for all running tasks first
|
// Abort all running sessions to prevent zombie processes (#1240)
|
||||||
|
for (const task of this.tasks.values()) {
|
||||||
|
if (task.status === "running" && task.sessionID) {
|
||||||
|
this.client.session.abort({
|
||||||
|
path: { id: task.sessionID },
|
||||||
|
}).catch(() => {})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Notify shutdown listeners (e.g., tmux cleanup)
|
||||||
|
if (this.onShutdown) {
|
||||||
|
try {
|
||||||
|
this.onShutdown()
|
||||||
|
} catch (error) {
|
||||||
|
log("[background-agent] Error in onShutdown callback:", error)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Release concurrency for all running tasks
|
||||||
for (const task of this.tasks.values()) {
|
for (const task of this.tasks.values()) {
|
||||||
if (task.concurrencyKey) {
|
if (task.concurrencyKey) {
|
||||||
this.concurrencyManager.release(task.concurrencyKey)
|
this.concurrencyManager.release(task.concurrencyKey)
|
||||||
|
|||||||
@ -6,6 +6,7 @@ import {
|
|||||||
} from "./storage";
|
} from "./storage";
|
||||||
import { OMO_SESSION_PREFIX, buildSessionReminderMessage } from "./constants";
|
import { OMO_SESSION_PREFIX, buildSessionReminderMessage } from "./constants";
|
||||||
import type { InteractiveBashSessionState } from "./types";
|
import type { InteractiveBashSessionState } from "./types";
|
||||||
|
import { subagentSessions } from "../../features/claude-code-session-state";
|
||||||
|
|
||||||
interface ToolExecuteInput {
|
interface ToolExecuteInput {
|
||||||
tool: string;
|
tool: string;
|
||||||
@ -146,7 +147,7 @@ function findSubcommand(tokens: string[]): string {
|
|||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createInteractiveBashSessionHook(_ctx: PluginInput) {
|
export function createInteractiveBashSessionHook(ctx: PluginInput) {
|
||||||
const sessionStates = new Map<string, InteractiveBashSessionState>();
|
const sessionStates = new Map<string, InteractiveBashSessionState>();
|
||||||
|
|
||||||
function getOrCreateState(sessionID: string): InteractiveBashSessionState {
|
function getOrCreateState(sessionID: string): InteractiveBashSessionState {
|
||||||
@ -178,6 +179,10 @@ export function createInteractiveBashSessionHook(_ctx: PluginInput) {
|
|||||||
await proc.exited;
|
await proc.exited;
|
||||||
} catch {}
|
} catch {}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
for (const sessionId of subagentSessions) {
|
||||||
|
ctx.client.session.abort({ path: { id: sessionId } }).catch(() => {})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const toolExecuteAfter = async (
|
const toolExecuteAfter = async (
|
||||||
|
|||||||
@ -264,6 +264,11 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
|
|||||||
});
|
});
|
||||||
log("[index] onSubagentSessionCreated callback completed");
|
log("[index] onSubagentSessionCreated callback completed");
|
||||||
},
|
},
|
||||||
|
onShutdown: () => {
|
||||||
|
tmuxSessionManager.cleanup().catch((error) => {
|
||||||
|
log("[index] tmux cleanup error during shutdown:", error)
|
||||||
|
})
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const atlasHook = isHookEnabled("atlas")
|
const atlasHook = isHookEnabled("atlas")
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user