Merge pull request #1870 from dankochetov/fix/background-notification-hook-gate
fix(background-agent): honor disabled background-notification for system reminders
This commit is contained in:
commit
5aa9ecdd5d
@ -22,8 +22,9 @@ export function createManagers(args: {
|
|||||||
pluginConfig: OhMyOpenCodeConfig
|
pluginConfig: OhMyOpenCodeConfig
|
||||||
tmuxConfig: TmuxConfig
|
tmuxConfig: TmuxConfig
|
||||||
modelCacheState: ModelCacheState
|
modelCacheState: ModelCacheState
|
||||||
|
backgroundNotificationHookEnabled: boolean
|
||||||
}): Managers {
|
}): Managers {
|
||||||
const { ctx, pluginConfig, tmuxConfig, modelCacheState } = args
|
const { ctx, pluginConfig, tmuxConfig, modelCacheState, backgroundNotificationHookEnabled } = args
|
||||||
|
|
||||||
const tmuxSessionManager = new TmuxSessionManager(ctx, tmuxConfig)
|
const tmuxSessionManager = new TmuxSessionManager(ctx, tmuxConfig)
|
||||||
|
|
||||||
@ -57,6 +58,7 @@ export function createManagers(args: {
|
|||||||
log("[index] tmux cleanup error during shutdown:", error)
|
log("[index] tmux cleanup error during shutdown:", error)
|
||||||
})
|
})
|
||||||
},
|
},
|
||||||
|
enableParentSessionNotifications: backgroundNotificationHookEnabled,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@ -1003,6 +1003,52 @@ describe("BackgroundManager.notifyParentSession - aborted parent", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("BackgroundManager.notifyParentSession - notifications toggle", () => {
|
||||||
|
test("should skip parent prompt injection when notifications are disabled", async () => {
|
||||||
|
//#given
|
||||||
|
let promptCalled = false
|
||||||
|
const promptMock = async () => {
|
||||||
|
promptCalled = true
|
||||||
|
return {}
|
||||||
|
}
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
prompt: promptMock,
|
||||||
|
promptAsync: promptMock,
|
||||||
|
abort: async () => ({}),
|
||||||
|
messages: async () => ({ data: [] }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const manager = new BackgroundManager(
|
||||||
|
{ client, directory: tmpdir() } as unknown as PluginInput,
|
||||||
|
undefined,
|
||||||
|
{ enableParentSessionNotifications: false },
|
||||||
|
)
|
||||||
|
const task: BackgroundTask = {
|
||||||
|
id: "task-no-parent-notification",
|
||||||
|
sessionID: "session-child",
|
||||||
|
parentSessionID: "session-parent",
|
||||||
|
parentMessageID: "msg-parent",
|
||||||
|
description: "task notifications disabled",
|
||||||
|
prompt: "test",
|
||||||
|
agent: "explore",
|
||||||
|
status: "completed",
|
||||||
|
startedAt: new Date(),
|
||||||
|
completedAt: new Date(),
|
||||||
|
}
|
||||||
|
getPendingByParent(manager).set("session-parent", new Set([task.id]))
|
||||||
|
|
||||||
|
//#when
|
||||||
|
await (manager as unknown as { notifyParentSession: (task: BackgroundTask) => Promise<void> })
|
||||||
|
.notifyParentSession(task)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(promptCalled).toBe(false)
|
||||||
|
|
||||||
|
manager.shutdown()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
function buildNotificationPromptBody(
|
function buildNotificationPromptBody(
|
||||||
task: BackgroundTask,
|
task: BackgroundTask,
|
||||||
currentMessage: CurrentMessage | null
|
currentMessage: CurrentMessage | null
|
||||||
|
|||||||
@ -92,6 +92,7 @@ export class BackgroundManager {
|
|||||||
private completionTimers: Map<string, ReturnType<typeof setTimeout>> = new Map()
|
private completionTimers: Map<string, ReturnType<typeof setTimeout>> = new Map()
|
||||||
private idleDeferralTimers: Map<string, ReturnType<typeof setTimeout>> = new Map()
|
private idleDeferralTimers: Map<string, ReturnType<typeof setTimeout>> = new Map()
|
||||||
private notificationQueueByParent: Map<string, Promise<void>> = new Map()
|
private notificationQueueByParent: Map<string, Promise<void>> = new Map()
|
||||||
|
private enableParentSessionNotifications: boolean
|
||||||
readonly taskHistory = new TaskHistory()
|
readonly taskHistory = new TaskHistory()
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
@ -101,6 +102,7 @@ export class BackgroundManager {
|
|||||||
tmuxConfig?: TmuxConfig
|
tmuxConfig?: TmuxConfig
|
||||||
onSubagentSessionCreated?: OnSubagentSessionCreated
|
onSubagentSessionCreated?: OnSubagentSessionCreated
|
||||||
onShutdown?: () => void
|
onShutdown?: () => void
|
||||||
|
enableParentSessionNotifications?: boolean
|
||||||
}
|
}
|
||||||
) {
|
) {
|
||||||
this.tasks = new Map()
|
this.tasks = new Map()
|
||||||
@ -113,6 +115,7 @@ export class BackgroundManager {
|
|||||||
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.onShutdown = options?.onShutdown
|
||||||
|
this.enableParentSessionNotifications = options?.enableParentSessionNotifications ?? true
|
||||||
this.registerProcessCleanup()
|
this.registerProcessCleanup()
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1203,19 +1206,22 @@ export class BackgroundManager {
|
|||||||
allComplete = true
|
allComplete = true
|
||||||
}
|
}
|
||||||
|
|
||||||
const statusText = task.status === "completed" ? "COMPLETED" : task.status === "interrupt" ? "INTERRUPTED" : "CANCELLED"
|
const completedTasks = allComplete
|
||||||
const errorInfo = task.error ? `\n**Error:** ${task.error}` : ""
|
? Array.from(this.tasks.values())
|
||||||
|
|
||||||
let notification: string
|
|
||||||
let completedTasks: BackgroundTask[] = []
|
|
||||||
if (allComplete) {
|
|
||||||
completedTasks = Array.from(this.tasks.values())
|
|
||||||
.filter(t => t.parentSessionID === task.parentSessionID && t.status !== "running" && t.status !== "pending")
|
.filter(t => t.parentSessionID === task.parentSessionID && t.status !== "running" && t.status !== "pending")
|
||||||
const completedTasksText = completedTasks
|
: []
|
||||||
.map(t => `- \`${t.id}\`: ${t.description}`)
|
|
||||||
.join("\n")
|
|
||||||
|
|
||||||
notification = `<system-reminder>
|
if (this.enableParentSessionNotifications) {
|
||||||
|
const statusText = task.status === "completed" ? "COMPLETED" : task.status === "interrupt" ? "INTERRUPTED" : "CANCELLED"
|
||||||
|
const errorInfo = task.error ? `\n**Error:** ${task.error}` : ""
|
||||||
|
|
||||||
|
let notification: string
|
||||||
|
if (allComplete) {
|
||||||
|
const completedTasksText = completedTasks
|
||||||
|
.map(t => `- \`${t.id}\`: ${t.description}`)
|
||||||
|
.join("\n")
|
||||||
|
|
||||||
|
notification = `<system-reminder>
|
||||||
[ALL BACKGROUND TASKS COMPLETE]
|
[ALL BACKGROUND TASKS COMPLETE]
|
||||||
|
|
||||||
**Completed:**
|
**Completed:**
|
||||||
@ -1223,9 +1229,9 @@ ${completedTasksText || `- \`${task.id}\`: ${task.description}`}
|
|||||||
|
|
||||||
Use \`background_output(task_id="<id>")\` to retrieve each result.
|
Use \`background_output(task_id="<id>")\` to retrieve each result.
|
||||||
</system-reminder>`
|
</system-reminder>`
|
||||||
} else {
|
} else {
|
||||||
// Individual completion - silent notification
|
// Individual completion - silent notification
|
||||||
notification = `<system-reminder>
|
notification = `<system-reminder>
|
||||||
[BACKGROUND TASK ${statusText}]
|
[BACKGROUND TASK ${statusText}]
|
||||||
**ID:** \`${task.id}\`
|
**ID:** \`${task.id}\`
|
||||||
**Description:** ${task.description}
|
**Description:** ${task.description}
|
||||||
@ -1236,70 +1242,76 @@ Do NOT poll - continue productive work.
|
|||||||
|
|
||||||
Use \`background_output(task_id="${task.id}")\` to retrieve this result when ready.
|
Use \`background_output(task_id="${task.id}")\` to retrieve this result when ready.
|
||||||
</system-reminder>`
|
</system-reminder>`
|
||||||
}
|
}
|
||||||
|
|
||||||
let agent: string | undefined = task.parentAgent
|
let agent: string | undefined = task.parentAgent
|
||||||
let model: { providerID: string; modelID: string } | undefined
|
let model: { providerID: string; modelID: string } | undefined
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const messagesResp = await this.client.session.messages({ path: { id: task.parentSessionID } })
|
const messagesResp = await this.client.session.messages({ path: { id: task.parentSessionID } })
|
||||||
const messages = normalizeSDKResponse(messagesResp, [] as Array<{
|
const messages = normalizeSDKResponse(messagesResp, [] as Array<{
|
||||||
info?: { agent?: string; model?: { providerID: string; modelID: string }; modelID?: string; providerID?: string }
|
info?: { agent?: string; model?: { providerID: string; modelID: string }; modelID?: string; providerID?: string }
|
||||||
}>)
|
}>)
|
||||||
for (let i = messages.length - 1; i >= 0; i--) {
|
for (let i = messages.length - 1; i >= 0; i--) {
|
||||||
const info = messages[i].info
|
const info = messages[i].info
|
||||||
if (info?.agent || info?.model || (info?.modelID && info?.providerID)) {
|
if (info?.agent || info?.model || (info?.modelID && info?.providerID)) {
|
||||||
agent = info.agent ?? task.parentAgent
|
agent = info.agent ?? task.parentAgent
|
||||||
model = info.model ?? (info.providerID && info.modelID ? { providerID: info.providerID, modelID: info.modelID } : undefined)
|
model = info.model ?? (info.providerID && info.modelID ? { providerID: info.providerID, modelID: info.modelID } : undefined)
|
||||||
break
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (error) {
|
||||||
|
if (this.isAbortedSessionError(error)) {
|
||||||
|
log("[background-agent] Parent session aborted while loading messages; using messageDir fallback:", {
|
||||||
|
taskId: task.id,
|
||||||
|
parentSessionID: task.parentSessionID,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
const messageDir = getMessageDir(task.parentSessionID)
|
||||||
|
const currentMessage = messageDir ? findNearestMessageWithFields(messageDir) : null
|
||||||
|
agent = currentMessage?.agent ?? task.parentAgent
|
||||||
|
model = currentMessage?.model?.providerID && currentMessage?.model?.modelID
|
||||||
|
? { providerID: currentMessage.model.providerID, modelID: currentMessage.model.modelID }
|
||||||
|
: undefined
|
||||||
|
}
|
||||||
|
|
||||||
|
log("[background-agent] notifyParentSession context:", {
|
||||||
|
taskId: task.id,
|
||||||
|
resolvedAgent: agent,
|
||||||
|
resolvedModel: model,
|
||||||
|
})
|
||||||
|
|
||||||
|
try {
|
||||||
|
await this.client.session.promptAsync({
|
||||||
|
path: { id: task.parentSessionID },
|
||||||
|
body: {
|
||||||
|
noReply: !allComplete,
|
||||||
|
...(agent !== undefined ? { agent } : {}),
|
||||||
|
...(model !== undefined ? { model } : {}),
|
||||||
|
...(task.parentTools ? { tools: task.parentTools } : {}),
|
||||||
|
parts: [{ type: "text", text: notification }],
|
||||||
|
},
|
||||||
|
})
|
||||||
|
log("[background-agent] Sent notification to parent session:", {
|
||||||
|
taskId: task.id,
|
||||||
|
allComplete,
|
||||||
|
noReply: !allComplete,
|
||||||
|
})
|
||||||
|
} catch (error) {
|
||||||
|
if (this.isAbortedSessionError(error)) {
|
||||||
|
log("[background-agent] Parent session aborted while sending notification; continuing cleanup:", {
|
||||||
|
taskId: task.id,
|
||||||
|
parentSessionID: task.parentSessionID,
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
log("[background-agent] Failed to send notification:", error)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} else {
|
||||||
if (this.isAbortedSessionError(error)) {
|
log("[background-agent] Parent session notifications disabled, skipping prompt injection:", {
|
||||||
log("[background-agent] Parent session aborted while loading messages; using messageDir fallback:", {
|
|
||||||
taskId: task.id,
|
|
||||||
parentSessionID: task.parentSessionID,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
const messageDir = getMessageDir(task.parentSessionID)
|
|
||||||
const currentMessage = messageDir ? findNearestMessageWithFields(messageDir) : null
|
|
||||||
agent = currentMessage?.agent ?? task.parentAgent
|
|
||||||
model = currentMessage?.model?.providerID && currentMessage?.model?.modelID
|
|
||||||
? { providerID: currentMessage.model.providerID, modelID: currentMessage.model.modelID }
|
|
||||||
: undefined
|
|
||||||
}
|
|
||||||
|
|
||||||
log("[background-agent] notifyParentSession context:", {
|
|
||||||
taskId: task.id,
|
|
||||||
resolvedAgent: agent,
|
|
||||||
resolvedModel: model,
|
|
||||||
})
|
|
||||||
|
|
||||||
try {
|
|
||||||
await this.client.session.promptAsync({
|
|
||||||
path: { id: task.parentSessionID },
|
|
||||||
body: {
|
|
||||||
noReply: !allComplete,
|
|
||||||
...(agent !== undefined ? { agent } : {}),
|
|
||||||
...(model !== undefined ? { model } : {}),
|
|
||||||
...(task.parentTools ? { tools: task.parentTools } : {}),
|
|
||||||
parts: [{ type: "text", text: notification }],
|
|
||||||
},
|
|
||||||
})
|
|
||||||
log("[background-agent] Sent notification to parent session:", {
|
|
||||||
taskId: task.id,
|
taskId: task.id,
|
||||||
allComplete,
|
parentSessionID: task.parentSessionID,
|
||||||
noReply: !allComplete,
|
|
||||||
})
|
})
|
||||||
} catch (error) {
|
|
||||||
if (this.isAbortedSessionError(error)) {
|
|
||||||
log("[background-agent] Parent session aborted while sending notification; continuing cleanup:", {
|
|
||||||
taskId: task.id,
|
|
||||||
parentSessionID: task.parentSessionID,
|
|
||||||
})
|
|
||||||
} else {
|
|
||||||
log("[background-agent] Failed to send notification:", error)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (allComplete) {
|
if (allComplete) {
|
||||||
|
|||||||
@ -44,6 +44,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
|
|||||||
pluginConfig,
|
pluginConfig,
|
||||||
tmuxConfig,
|
tmuxConfig,
|
||||||
modelCacheState,
|
modelCacheState,
|
||||||
|
backgroundNotificationHookEnabled: isHookEnabled("background-notification"),
|
||||||
})
|
})
|
||||||
|
|
||||||
const toolsResult = await createTools({
|
const toolsResult = await createTools({
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user