diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 8098cdf26..59d5ba710 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -5643,6 +5643,7 @@ describe("BackgroundManager.handleEvent - session.error", () => { const client = { session: { status: async () => ({ data: { "parent-session-wake": { type: "idle" } } }), + messages: async () => ({ data: [] }), promptAsync: async (args: { path: { id: string }; body: Record }) => { promptCalls.push(args) return {} @@ -5699,6 +5700,7 @@ describe("BackgroundManager.handleEvent - session.error", () => { const client = { session: { status: async () => ({ data: { "parent-session-alias": { type: "idle" } } }), + messages: async () => ({ data: [] }), promptAsync: async (args: { path: { id: string }; body: Record }) => { promptCalls.push(args) return {} diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 125b6d352..74cac7b11 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -149,7 +149,7 @@ const PARENT_WAKE_TOOL_CALL_DEFER_MAX_MS = 5_000 * env. See issue #4120. */ const PARENT_WAKE_USER_MESSAGE_IN_PROGRESS_WINDOW_MS = 2_000 -const PARENT_WAKE_SESSION_ACTIVITY_IN_PROGRESS_WINDOW_MS = 2_000 +const PARENT_WAKE_SESSION_ACTIVITY_IN_PROGRESS_WINDOW_MS = PARENT_WAKE_TOOL_CALL_DEFER_MAX_MS interface EventProperties { sessionID?: string diff --git a/src/features/background-agent/parent-wake-activity-window.test.ts b/src/features/background-agent/parent-wake-activity-window.test.ts new file mode 100644 index 000000000..525b8481e --- /dev/null +++ b/src/features/background-agent/parent-wake-activity-window.test.ts @@ -0,0 +1,129 @@ +import { tmpdir } from "node:os" +import { afterEach, describe, expect, test } from "bun:test" +import type { PluginInput } from "@opencode-ai/plugin" +import { unsafeTestValue } from "../../../test-support/unsafe-test-value" +import { releaseAllPromptAsyncReservationsForTesting } from "../../hooks/shared/prompt-async-gate" +import { BackgroundManager } from "./manager" +import type { BackgroundTask } from "./types" + +type PromptAsyncCall = { + readonly path: { readonly id: string } + readonly body: { readonly parts?: readonly unknown[] } +} + +let managerUnderTest: BackgroundManager | undefined + +afterEach(() => { + managerUnderTest?.shutdown() + managerUnderTest = undefined + releaseAllPromptAsyncReservationsForTesting() +}) + +function createTask(): BackgroundTask { + return { + id: "task-a", + parentMessageId: "parent-message-id", + parentSessionId: "parent-1", + description: "task A", + prompt: "Prompt for task A", + agent: "test-agent", + status: "completed", + startedAt: new Date("2026-05-20T14:19:10.000Z"), + completedAt: new Date("2026-05-20T14:19:14.625Z"), + } +} + +function createManager(): { + readonly manager: BackgroundManager + readonly promptAsyncCalls: readonly PromptAsyncCall[] +} { + const promptAsyncCalls: PromptAsyncCall[] = [] + const client = unsafeTestValue({ + session: { + messages: async () => [], + status: async () => ({ data: { "parent-1": { type: "idle" } } }), + prompt: async () => ({}), + promptAsync: async (call: PromptAsyncCall) => { + promptAsyncCalls.push(call) + return {} + }, + abort: async () => ({}), + }, + }) + const manager = new BackgroundManager({ + pluginContext: { + client, + project: {}, + directory: tmpdir(), + worktree: tmpdir(), + serverUrl: new URL("http://localhost"), + $: {}, + }, + config: undefined, + enableParentSessionNotifications: true, + }) + return { manager, promptAsyncCalls } +} + +function getTasks(manager: BackgroundManager): Map { + return unsafeTestValue>(Reflect.get(manager, "tasks")) +} + +function getPendingByParent(manager: BackgroundManager): Map> { + return unsafeTestValue>>(Reflect.get(manager, "pendingByParent")) +} + +function getPendingParentWakes(manager: BackgroundManager): Map { + const notifier = unsafeTestValue<{ + readonly getPendingParentWakes: () => Map + }>(Reflect.get(manager, "parentWakeNotifier")) + return notifier.getPendingParentWakes() +} + +async function notifyParentSessionForTest(manager: BackgroundManager, task: BackgroundTask): Promise { + const notifyParentSession = unsafeTestValue<(task: BackgroundTask) => Promise>(Reflect.get(manager, "notifyParentSession")) + await notifyParentSession.call(manager, task) +} + +async function flushPendingParentWakeForTest(manager: BackgroundManager, sessionID: string): Promise { + const flushPendingParentWake = unsafeTestValue<(sessionID: string) => Promise>(Reflect.get(manager, "flushPendingParentWake")) + await flushPendingParentWake.call(manager, sessionID) +} + +describe("BackgroundManager parent wake activity window", () => { + test("#given parent tool activity is within the tool deferral window #when stale idle flushes a wake #then parent prompt stays deferred", async () => { + // given + const originalDateNow = Date.now + let now = 100_000 + Date.now = () => now + const { manager, promptAsyncCalls } = createManager() + managerUnderTest = manager + manager.handleEvent({ + type: "message.part.updated", + properties: { + sessionID: "parent-1", + part: { + sessionID: "parent-1", + type: "tool", + tool: "todowrite", + }, + }, + }) + now = 104_900 + const task = createTask() + getTasks(manager).set(task.id, task) + getPendingByParent(manager).set(task.parentSessionId, new Set([task.id])) + + try { + // when + await notifyParentSessionForTest(manager, task) + await flushPendingParentWakeForTest(manager, "parent-1") + + // then + expect(promptAsyncCalls).toHaveLength(0) + expect(getPendingParentWakes(manager).has("parent-1")).toBe(true) + } finally { + Date.now = originalDateNow + } + }) +}) diff --git a/src/features/background-agent/parent-wake-assistant-blocking.test.ts b/src/features/background-agent/parent-wake-assistant-blocking.test.ts index fa855e635..e70b0ed7b 100644 --- a/src/features/background-agent/parent-wake-assistant-blocking.test.ts +++ b/src/features/background-agent/parent-wake-assistant-blocking.test.ts @@ -19,138 +19,6 @@ type PromptAsyncCall = { type ParentWakeClient = ConstructorParameters[0]["client"] describe("ParentWakeNotifier — assistant turn blocking", () => { - test("#given stale unfinished assistant text has no pending tool call #when checking parent wake history #then parent wake dispatches after defer max", async () => { - // given - const originalDateNow = Date.now - Date.now = () => 100_000 - const client = unsafeTestValue({ - session: { - messages: async () => ({ - data: [ - { - info: { - role: "assistant", - finish: "unknown", - time: { created: 90_000 }, - }, - parts: [{ type: "text", text: "still streaming" }], - }, - ], - }), - status: async () => ({ data: { "parent-stale-text": { type: "idle" } } }), - promptAsync: async () => { - return { data: {} } - }, - }, - }) - const notifier = new ParentWakeNotifier( - { - client, - directory: "/tmp/test-omo", - enqueueNotificationForParent: async (_sessionID, operation) => { - await operation() - }, - }, - { - pendingRetryMs: 1_000, - acceptedMessageSkewMs: 5_000, - toolCallDeferMaxMs: 5_000, - failureRequeueWindowMs: 5_000, - userMessageInProgressWindowMs: 2_000, - }, - ) - notifier.queuePendingParentWake( - "parent-stale-text", - "task complete", - { agent: "sisyphus" }, - true, - ) - const pendingWake = notifier.getPendingParentWakes().get("parent-stale-text") - expect(pendingWake).toBeDefined() - if (!pendingWake) { - throw new Error("Missing pending parent wake") - } - pendingWake.toolCallDeferralStartedAt = 90_000 - - try { - // when - const decision = await notifier["shouldDeferParentWakeForSessionHistory"]("parent-stale-text", pendingWake) - - // then - expect(decision).toEqual({ defer: false, skipPromptGateToolStateCheck: false }) - } finally { - Date.now = originalDateNow - notifier.shutdown() - releaseAllPromptAsyncReservationsForTesting() - } - }) - - test("#given fresh unfinished assistant text has no pending tool call #when checking parent wake history #then parent wake continues deferring", async () => { - // given - const originalDateNow = Date.now - Date.now = () => 100_000 - const client = unsafeTestValue({ - session: { - messages: async () => ({ - data: [ - { - info: { - role: "assistant", - finish: "unknown", - time: { created: 99_000 }, - }, - parts: [{ type: "text", text: "still streaming" }], - }, - ], - }), - status: async () => ({ data: { "parent-fresh-text": { type: "idle" } } }), - promptAsync: async () => { - return { data: {} } - }, - }, - }) - const notifier = new ParentWakeNotifier( - { - client, - directory: "/tmp/test-omo", - enqueueNotificationForParent: async (_sessionID, operation) => { - await operation() - }, - }, - { - pendingRetryMs: 1_000, - acceptedMessageSkewMs: 5_000, - toolCallDeferMaxMs: 5_000, - failureRequeueWindowMs: 5_000, - userMessageInProgressWindowMs: 2_000, - }, - ) - notifier.queuePendingParentWake( - "parent-fresh-text", - "task complete", - { agent: "sisyphus" }, - true, - ) - const pendingWake = notifier.getPendingParentWakes().get("parent-fresh-text") - expect(pendingWake).toBeDefined() - if (!pendingWake) { - throw new Error("Missing pending parent wake") - } - pendingWake.toolCallDeferralStartedAt = 98_000 - - try { - // when - const decision = await notifier["shouldDeferParentWakeForSessionHistory"]("parent-fresh-text", pendingWake) - - // then - expect(decision).toEqual({ defer: true, skipPromptGateToolStateCheck: false }) - } finally { - Date.now = originalDateNow - notifier.shutdown() - releaseAllPromptAsyncReservationsForTesting() - } - }) - test("#given notifier sees an unfinished assistant but prompt gate message fetch fails #when flushing pending wake #then the wake stays pending", async () => { // given const promptAsyncCalls: PromptAsyncCall[] = [] @@ -216,4 +84,86 @@ describe("ParentWakeNotifier — assistant turn blocking", () => { notifier.shutdown() releaseAllPromptAsyncReservationsForTesting() }) + + test("#given stale completed assistant question tool has no real user answer #when flushing pending wake #then wake stays pending", async () => { + // given + const originalDateNow = Date.now + Date.now = () => 100_000 + const promptAsyncCalls: PromptAsyncCall[] = [] + const client = unsafeTestValue({ + session: { + messages: async () => ({ + data: [ + { + info: { + role: "user", + time: { created: 10_000 }, + }, + parts: [{ type: "text", text: "start work" }], + }, + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 20_000, completed: 99_000 }, + }, + parts: [ + { + type: "tool", + tool: "question", + state: { status: "error" }, + }, + ], + }, + ], + }), + status: async () => ({ data: { "parent-question-unanswered": { type: "idle" } } }), + promptAsync: async (call: PromptAsyncCall) => { + promptAsyncCalls.push(call) + return { data: {} } + }, + }, + }) + const notifier = new ParentWakeNotifier( + { + client, + directory: "/tmp/test-omo", + enqueueNotificationForParent: async (_sessionID, operation) => { + await operation() + }, + }, + { + pendingRetryMs: 1_000, + acceptedMessageSkewMs: 5_000, + toolCallDeferMaxMs: 5_000, + failureRequeueWindowMs: 5_000, + userMessageInProgressWindowMs: 2_000, + }, + ) + notifier.queuePendingParentWake( + "parent-question-unanswered", + "task complete", + { agent: "sisyphus" }, + true, + ) + const pendingWake = notifier.getPendingParentWakes().get("parent-question-unanswered") + expect(pendingWake).toBeDefined() + if (!pendingWake) { + throw new Error("Missing pending parent wake") + } + pendingWake.toolCallDeferralStartedAt = 1_000 + + try { + // when + await notifier.flushPendingParentWake("parent-question-unanswered") + + // then + expect(promptAsyncCalls).toHaveLength(0) + expect(notifier.getPendingParentWakes().has("parent-question-unanswered")).toBe(true) + } finally { + Date.now = originalDateNow + notifier.shutdown() + releaseAllPromptAsyncReservationsForTesting() + } + }) }) diff --git a/src/features/background-agent/parent-wake-history-deferral.test.ts b/src/features/background-agent/parent-wake-history-deferral.test.ts new file mode 100644 index 000000000..c73450aa3 --- /dev/null +++ b/src/features/background-agent/parent-wake-history-deferral.test.ts @@ -0,0 +1,268 @@ +/// + +import { describe, expect, test } from "bun:test" +import { releaseAllPromptAsyncReservationsForTesting } from "../../hooks/shared/prompt-async-gate" +import { unsafeTestValue } from "../../../test-support/unsafe-test-value" +import { ParentWakeNotifier } from "./parent-wake-notifier" + +type ParentWakeClient = ConstructorParameters[0]["client"] + +describe("ParentWakeNotifier — assistant history deferral", () => { + test("#given stale unfinished assistant text has no pending tool call #when checking parent wake history #then parent wake dispatches after defer max", async () => { + // given + const originalDateNow = Date.now + Date.now = () => 100_000 + const client = unsafeTestValue({ + session: { + messages: async () => ({ + data: [ + { + info: { + role: "assistant", + finish: "unknown", + time: { created: 90_000 }, + }, + parts: [{ type: "text", text: "still streaming" }], + }, + ], + }), + status: async () => ({ data: { "parent-stale-text": { type: "idle" } } }), + promptAsync: async () => { + return { data: {} } + }, + }, + }) + const notifier = new ParentWakeNotifier( + { + client, + directory: "/tmp/test-omo", + enqueueNotificationForParent: async (_sessionID, operation) => { + await operation() + }, + }, + { + pendingRetryMs: 1_000, + acceptedMessageSkewMs: 5_000, + toolCallDeferMaxMs: 5_000, + failureRequeueWindowMs: 5_000, + userMessageInProgressWindowMs: 2_000, + }, + ) + notifier.queuePendingParentWake( + "parent-stale-text", + "task complete", + { agent: "sisyphus" }, + true, + ) + const pendingWake = notifier.getPendingParentWakes().get("parent-stale-text") + expect(pendingWake).toBeDefined() + if (!pendingWake) { + throw new Error("Missing pending parent wake") + } + pendingWake.toolCallDeferralStartedAt = 90_000 + + try { + // when + const decision = await notifier["shouldDeferParentWakeForSessionHistory"]("parent-stale-text", pendingWake) + + // then + expect(decision).toEqual({ defer: false, skipPromptGateToolStateCheck: false }) + } finally { + Date.now = originalDateNow + notifier.shutdown() + releaseAllPromptAsyncReservationsForTesting() + } + }) + + test("#given fresh unfinished assistant text has no pending tool call #when checking parent wake history #then parent wake continues deferring", async () => { + // given + const originalDateNow = Date.now + Date.now = () => 100_000 + const client = unsafeTestValue({ + session: { + messages: async () => ({ + data: [ + { + info: { + role: "assistant", + finish: "unknown", + time: { created: 99_000 }, + }, + parts: [{ type: "text", text: "still streaming" }], + }, + ], + }), + status: async () => ({ data: { "parent-fresh-text": { type: "idle" } } }), + promptAsync: async () => { + return { data: {} } + }, + }, + }) + const notifier = new ParentWakeNotifier( + { + client, + directory: "/tmp/test-omo", + enqueueNotificationForParent: async (_sessionID, operation) => { + await operation() + }, + }, + { + pendingRetryMs: 1_000, + acceptedMessageSkewMs: 5_000, + toolCallDeferMaxMs: 5_000, + failureRequeueWindowMs: 5_000, + userMessageInProgressWindowMs: 2_000, + }, + ) + notifier.queuePendingParentWake( + "parent-fresh-text", + "task complete", + { agent: "sisyphus" }, + true, + ) + const pendingWake = notifier.getPendingParentWakes().get("parent-fresh-text") + expect(pendingWake).toBeDefined() + if (!pendingWake) { + throw new Error("Missing pending parent wake") + } + pendingWake.toolCallDeferralStartedAt = 98_000 + + try { + // when + const decision = await notifier["shouldDeferParentWakeForSessionHistory"]("parent-fresh-text", pendingWake) + + // then + expect(decision).toEqual({ defer: true, skipPromptGateToolStateCheck: false }) + } finally { + Date.now = originalDateNow + notifier.shutdown() + releaseAllPromptAsyncReservationsForTesting() + } + }) + + test("#given parent session messages cannot be inspected #when checking parent wake history #then parent wake stays deferred", async () => { + // given + const client = unsafeTestValue({ + session: { + messages: async () => { + throw new Error("message endpoint failed") + }, + status: async () => ({ data: { "parent-message-error": { type: "idle" } } }), + promptAsync: async () => { + return { data: {} } + }, + }, + }) + const notifier = new ParentWakeNotifier( + { + client, + directory: "/tmp/test-omo", + enqueueNotificationForParent: async (_sessionID, operation) => { + await operation() + }, + }, + { + pendingRetryMs: 1_000, + acceptedMessageSkewMs: 5_000, + toolCallDeferMaxMs: 5_000, + failureRequeueWindowMs: 5_000, + userMessageInProgressWindowMs: 2_000, + }, + ) + notifier.queuePendingParentWake( + "parent-message-error", + "task complete", + { agent: "sisyphus" }, + true, + ) + const pendingWake = notifier.getPendingParentWakes().get("parent-message-error") + expect(pendingWake).toBeDefined() + if (!pendingWake) { + throw new Error("Missing pending parent wake") + } + + try { + // when + const decision = await notifier["shouldDeferParentWakeForSessionHistory"]("parent-message-error", pendingWake) + + // then + expect(decision).toEqual({ defer: true, skipPromptGateToolStateCheck: false }) + } finally { + notifier.shutdown() + releaseAllPromptAsyncReservationsForTesting() + } + }) + + test("#given old assistant turn has recent running tool activity #when checking parent wake history #then stale tool escape stays deferred", async () => { + // given + const originalDateNow = Date.now + Date.now = () => 100_000 + const client = unsafeTestValue({ + session: { + messages: async () => ({ + data: [ + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 80_000 }, + }, + parts: [ + { + type: "tool", + tool: "bash", + time: { start: 99_000, end: 99_500 }, + state: { status: "running" }, + }, + ], + }, + ], + }), + status: async () => ({ data: { "parent-fresh-tool-activity": { type: "idle" } } }), + promptAsync: async () => { + return { data: {} } + }, + }, + }) + const notifier = new ParentWakeNotifier( + { + client, + directory: "/tmp/test-omo", + enqueueNotificationForParent: async (_sessionID, operation) => { + await operation() + }, + }, + { + pendingRetryMs: 1_000, + acceptedMessageSkewMs: 5_000, + toolCallDeferMaxMs: 5_000, + failureRequeueWindowMs: 5_000, + userMessageInProgressWindowMs: 2_000, + }, + ) + notifier.queuePendingParentWake( + "parent-fresh-tool-activity", + "task complete", + { agent: "sisyphus" }, + true, + ) + const pendingWake = notifier.getPendingParentWakes().get("parent-fresh-tool-activity") + expect(pendingWake).toBeDefined() + if (!pendingWake) { + throw new Error("Missing pending parent wake") + } + pendingWake.toolCallDeferralStartedAt = 90_000 + + try { + // when + const decision = await notifier["shouldDeferParentWakeForSessionHistory"]("parent-fresh-tool-activity", pendingWake) + + // then + expect(decision).toEqual({ defer: true, skipPromptGateToolStateCheck: false }) + } finally { + Date.now = originalDateNow + notifier.shutdown() + releaseAllPromptAsyncReservationsForTesting() + } + }) +}) diff --git a/src/features/background-agent/parent-wake-message-activity.ts b/src/features/background-agent/parent-wake-message-activity.ts new file mode 100644 index 000000000..9de5bd63a --- /dev/null +++ b/src/features/background-agent/parent-wake-message-activity.ts @@ -0,0 +1,78 @@ +type ParentWakeMessageTime = { + readonly created?: unknown + readonly updated?: unknown + readonly completed?: unknown + readonly start?: unknown + readonly end?: unknown +} + +type ParentWakeMessageActivityPart = { + readonly time?: ParentWakeMessageTime + readonly state?: { + readonly time?: ParentWakeMessageTime + } +} + +type ParentWakeMessageActivity = { + readonly info?: { + readonly time?: ParentWakeMessageTime + } + readonly time?: ParentWakeMessageTime + readonly parts?: readonly ParentWakeMessageActivityPart[] +} + +function timestampFromUnknown(value: unknown): number | undefined { + if (typeof value === "number" && Number.isFinite(value)) { + return value + } + if (typeof value === "string") { + const parsed = Date.parse(value) + return Number.isFinite(parsed) ? parsed : undefined + } + if (value instanceof Date) { + return value.getTime() + } + return undefined +} + +function latestTimestamp(...values: readonly unknown[]): number | undefined { + let latest: number | undefined + for (const value of values) { + const timestamp = timestampFromUnknown(value) + if (timestamp === undefined) { + continue + } + if (latest === undefined || timestamp > latest) { + latest = timestamp + } + } + return latest +} + +function latestTimeActivity(time: ParentWakeMessageTime | undefined): number | undefined { + if (!time) { + return undefined + } + return latestTimestamp(time.created, time.updated, time.completed, time.start, time.end) +} + +export function getParentWakeMessageCreatedAt(message: ParentWakeMessageActivity): number | undefined { + return timestampFromUnknown(message.info?.time?.created ?? message.time?.created) +} + +export function getParentWakeMessageActivityAt(message: ParentWakeMessageActivity): number | undefined { + let latest = latestTimestamp( + latestTimeActivity(message.info?.time), + latestTimeActivity(message.time), + ) + for (const part of message.parts ?? []) { + const partActivity = latestTimestamp( + latestTimeActivity(part.time), + latestTimeActivity(part.state?.time), + ) + if (partActivity !== undefined && (latest === undefined || partActivity > latest)) { + latest = partActivity + } + } + return latest +} diff --git a/src/features/background-agent/parent-wake-notifier.ts b/src/features/background-agent/parent-wake-notifier.ts index c3800ac8d..fa6c80ce1 100644 --- a/src/features/background-agent/parent-wake-notifier.ts +++ b/src/features/background-agent/parent-wake-notifier.ts @@ -7,8 +7,12 @@ import { } from "../../shared" import { isSessionActive as isOpenCodeSessionActive, settleAfterSessionIdle } from "../../hooks/shared/session-idle-settle" import { dispatchInternalPrompt, isInternalPromptDispatchAccepted } from "../../hooks/shared/prompt-async-gate" +import { isPromptMessageInspectionAborted } from "../../shared/prompt-async-gate/message-inspection-error" import type { PromptDispatchClient } from "../../shared/prompt-async-gate/types" -import { latestAssistantTurnBlocksInternalPrompt } from "../../shared/prompt-async-gate/pending-tool-turn" +import { + latestAssistantTurnBlocksInternalPrompt, + latestAssistantTurnHasUnansweredQuestion, +} from "../../shared/prompt-async-gate/pending-tool-turn" import type { PluginInput } from "@opencode-ai/plugin" import { cloneParentWake, @@ -17,6 +21,7 @@ import { type ParentWakePromptContext, type PendingParentWake, } from "./parent-wake-dedupe" +import { getParentWakeMessageActivityAt, getParentWakeMessageCreatedAt } from "./parent-wake-message-activity" type OpencodeClient = PluginInput["client"] type ParentWakeNotifierClient = PromptDispatchClient & { @@ -32,18 +37,20 @@ type ParentWakeSessionMessage = { info?: { role?: string finish?: string - time?: { created?: unknown } + time?: { created?: unknown; updated?: unknown; completed?: unknown; start?: unknown; end?: unknown } } role?: string finish?: string - time?: { created?: unknown } + time?: { created?: unknown; updated?: unknown; completed?: unknown; start?: unknown; end?: unknown } parts?: Array<{ type?: string text?: string synthetic?: boolean content?: unknown + time?: { created?: unknown; updated?: unknown; completed?: unknown; start?: unknown; end?: unknown } state?: { status?: unknown + time?: { created?: unknown; updated?: unknown; completed?: unknown; start?: unknown; end?: unknown } } }> } @@ -393,7 +400,7 @@ export class ParentWakeNotifier { this.dispatchedParentWakeTimers.set(sessionID, timer) } - private async loadParentWakeSessionMessages(sessionID: string): Promise { + private async loadParentWakeSessionMessages(sessionID: string): Promise { try { const messagesResp = await this.deps.client.session.messages({ path: { id: sessionID }, @@ -405,7 +412,7 @@ export class ParentWakeNotifier { sessionID, error, }) - return [] + return isPromptMessageInspectionAborted(error) ? [] : undefined } } @@ -417,21 +424,6 @@ export class ParentWakeNotifier { return message.info?.finish ?? message.finish } - private getParentWakeMessageCreatedAt(message: ParentWakeSessionMessage): number | undefined { - const value = message.info?.time?.created ?? message.time?.created - if (typeof value === "number" && Number.isFinite(value)) { - return value - } - if (typeof value === "string") { - const parsed = Date.parse(value) - return Number.isFinite(parsed) ? parsed : undefined - } - if (value instanceof Date) { - return value.getTime() - } - return undefined - } - private parentWakePartIsWaitingOnTool(part: NonNullable[number]): boolean { if ( part.type !== "tool" @@ -448,7 +440,7 @@ export class ParentWakeNotifier { private latestAssistantToolWaitState(messages: ParentWakeSessionMessage[]): { waiting: boolean - createdAt?: number + activityAt?: number } { for (let index = messages.length - 1; index >= 0; index--) { const message = messages[index] @@ -460,7 +452,7 @@ export class ParentWakeNotifier { const waiting = this.getParentWakeMessageFinish(message) === "tool-calls" || message.parts?.some((part) => this.parentWakePartIsWaitingOnTool(part)) === true return waiting - ? { waiting: true, createdAt: this.getParentWakeMessageCreatedAt(message) } + ? { waiting: true, activityAt: getParentWakeMessageActivityAt(message) } : { waiting: false } } if (role === "user") { @@ -522,6 +514,9 @@ export class ParentWakeNotifier { return false } const messages = await this.loadParentWakeSessionMessages(sessionID) + if (!messages) { + return true + } for (let index = messages.length - 1; index >= 0; index--) { const message = messages[index] if (!message) { @@ -532,7 +527,7 @@ export class ParentWakeNotifier { if (isSyntheticOrInternalUserMessage(message)) { continue } - const createdAt = this.getParentWakeMessageCreatedAt(message) + const createdAt = getParentWakeMessageCreatedAt(message) if (createdAt === undefined) { return false } @@ -552,7 +547,14 @@ export class ParentWakeNotifier { wake: PendingParentWake, ): Promise { const messages = await this.loadParentWakeSessionMessages(sessionID) + if (!messages) { + log("[background-agent] Deferred parent wake because parent messages could not be inspected:", { + sessionID, + }) + return { defer: true, skipPromptGateToolStateCheck: false } + } const latestAssistantBlocksPrompt = latestAssistantTurnBlocksInternalPrompt(messages) + const latestAssistantHasUnansweredQuestion = latestAssistantTurnHasUnansweredQuestion(messages) const toolWaitState = this.latestAssistantToolWaitState(messages) if (!latestAssistantBlocksPrompt) { delete wake.toolCallDeferralStartedAt @@ -560,9 +562,15 @@ export class ParentWakeNotifier { } const now = Date.now() wake.toolCallDeferralStartedAt ??= now - const latestToolWaitAgeMs = toolWaitState.createdAt === undefined + if (latestAssistantHasUnansweredQuestion) { + log("[background-agent] Deferred parent wake because latest assistant question awaits user response:", { + sessionID, + }) + return { defer: true, skipPromptGateToolStateCheck: false } + } + const latestToolWaitAgeMs = toolWaitState.activityAt === undefined ? 0 - : now - toolWaitState.createdAt + : now - toolWaitState.activityAt const deferAge = now - wake.toolCallDeferralStartedAt if ( wake.shouldReply @@ -594,8 +602,11 @@ export class ParentWakeNotifier { } const dispatchedAt = wake.dispatchedAt const messages = await this.loadParentWakeSessionMessages(sessionID) + if (!messages) { + return false + } return messages.some((message) => { - const createdAt = this.getParentWakeMessageCreatedAt(message) + const createdAt = getParentWakeMessageCreatedAt(message) if (createdAt === undefined) { return false } diff --git a/src/features/background-agent/parent-wake-user-message-race.test.ts b/src/features/background-agent/parent-wake-user-message-race.test.ts index ffcc72568..feb5ec35e 100644 --- a/src/features/background-agent/parent-wake-user-message-race.test.ts +++ b/src/features/background-agent/parent-wake-user-message-race.test.ts @@ -581,7 +581,7 @@ describe("ParentWakeNotifier — user message race guard (issue #4120)", () => { } }) - test("#given only an internal user tail is fresh #when flushing pending wake #then user race guard does not defer", async () => { + test("#given only an internal user tail is fresh #when flushing pending wake #then parent wake remains deferred", async () => { // given const originalDateNow = Date.now Date.now = () => 100_000 @@ -615,8 +615,8 @@ describe("ParentWakeNotifier — user message race guard (issue #4120)", () => { await notifier.flushPendingParentWake("parent-internal-tail-user-race") // then - expect(promptAsyncCalls).toHaveLength(1) - expect(notifier.getPendingParentWakes().has("parent-internal-tail-user-race")).toBe(false) + expect(promptAsyncCalls).toHaveLength(0) + expect(notifier.getPendingParentWakes().has("parent-internal-tail-user-race")).toBe(true) } finally { Date.now = originalDateNow notifier.shutdown() diff --git a/src/hooks/shared/prompt-async-gate-message-fetch.test.ts b/src/hooks/shared/prompt-async-gate-message-fetch.test.ts new file mode 100644 index 000000000..15ad3bc5e --- /dev/null +++ b/src/hooks/shared/prompt-async-gate-message-fetch.test.ts @@ -0,0 +1,79 @@ +/// + +import { afterEach, describe, expect, test } from "bun:test" +import { + _setPromptGateMessagesFetchTimeoutMsForTesting, + dispatchInternalPrompt, + releaseAllPromptAsyncReservationsForTesting, +} from "./prompt-async-gate" + +describe("dispatchInternalPrompt message fetch safety", () => { + afterEach(() => { + // then + _setPromptGateMessagesFetchTimeoutMsForTesting(undefined) + releaseAllPromptAsyncReservationsForTesting() + }) + + test("#given latest-message fetch hangs #when an internal promptAsync is requested #then no prompt is sent", async () => { + // given + _setPromptGateMessagesFetchTimeoutMsForTesting(5) + let promptCalls = 0 + const client = { + session: { + status: async () => ({ data: { ses_messages_hang: { type: "idle" } } }), + messages: async () => new Promise(() => {}), + promptAsync: async () => { + promptCalls += 1 + }, + }, + } + + // when + const result = await dispatchInternalPrompt({ + mode: "async", + client, + sessionID: "ses_messages_hang", + input: { path: { id: "ses_messages_hang" }, body: { parts: [] } }, + source: "test:messages-hang", + settleMs: 0, + postDispatchHoldMs: 0, + dispatchTimeoutMs: 50, + }) + + // then + expect(result.status).toBe("queued") + expect(promptCalls).toBe(0) + }) + + test("#given latest-message fetch throws #when an internal promptAsync is requested #then no prompt is sent", async () => { + // given + let promptCalls = 0 + const client = { + session: { + status: async () => ({ data: { ses_messages_throw: { type: "idle" } } }), + messages: async () => { + throw new Error("message endpoint failed") + }, + promptAsync: async () => { + promptCalls += 1 + }, + }, + } + + // when + const result = await dispatchInternalPrompt({ + mode: "async", + client, + sessionID: "ses_messages_throw", + input: { path: { id: "ses_messages_throw" }, body: { parts: [] } }, + source: "test:messages-throw", + settleMs: 0, + postDispatchHoldMs: 0, + dispatchTimeoutMs: 50, + }) + + // then + expect(result.status).toBe("queued") + expect(promptCalls).toBe(0) + }) +}) diff --git a/src/hooks/shared/prompt-async-gate-question.test.ts b/src/hooks/shared/prompt-async-gate-question.test.ts new file mode 100644 index 000000000..0d665b799 --- /dev/null +++ b/src/hooks/shared/prompt-async-gate-question.test.ts @@ -0,0 +1,53 @@ +import { afterEach, describe, expect, test } from "bun:test" + +import { + dispatchInternalPrompt, + releaseAllPromptAsyncReservationsForTesting, +} from "./prompt-async-gate" + +describe("dispatchInternalPrompt question tool gating", () => { + afterEach(() => { + releaseAllPromptAsyncReservationsForTesting() + }) + + test("#given completed assistant question has no real user answer #when an internal promptAsync is requested #then no prompt is sent", async () => { + // given + let promptCalls = 0 + const client = { + session: { + status: async () => ({ data: { ses_completed_question: { type: "idle" } } }), + messages: async () => ({ + data: [ + { + info: { + id: "msg_assistant", + role: "assistant", + finish: "tool-calls", + time: { completed: 1_762_000_000_000 }, + }, + parts: [{ type: "tool", tool: "question", state: { status: "error" } }], + }, + ], + }), + promptAsync: async () => { + promptCalls += 1 + }, + }, + } + + // when + const result = await dispatchInternalPrompt({ + mode: "async", + client, + sessionID: "ses_completed_question", + input: { path: { id: "ses_completed_question" }, body: { parts: [] } }, + source: "test:completed-question", + settleMs: 0, + postDispatchHoldMs: 0, + }) + + // then + expect(result.status).toBe("queued") + expect(promptCalls).toBe(0) + }) +}) diff --git a/src/hooks/shared/prompt-async-gate.test.ts b/src/hooks/shared/prompt-async-gate.test.ts index f3b61d881..efdfb4ca4 100644 --- a/src/hooks/shared/prompt-async-gate.test.ts +++ b/src/hooks/shared/prompt-async-gate.test.ts @@ -1001,37 +1001,6 @@ describe("dispatchInternalPrompt shared gate behavior", () => { expect(promptCalls).toBe(1) }) - test("#given latest-message fetch hangs #when an internal promptAsync is requested #then the tool-state check times out and dispatch continues", async () => { - // given - _setPromptGateMessagesFetchTimeoutMsForTesting(5) - let promptCalls = 0 - const client = { - session: { - status: async () => ({ data: { ses_messages_hang: { type: "idle" } } }), - messages: async () => new Promise(() => {}), - promptAsync: async () => { - promptCalls += 1 - }, - }, - } - - // when - const result = await dispatchInternalPrompt({ - mode: "async", - client, - sessionID: "ses_messages_hang", - input: { path: { id: "ses_messages_hang" }, body: { parts: [] } }, - source: "test:messages-hang", - settleMs: 0, - postDispatchHoldMs: 0, - dispatchTimeoutMs: 50, - }) - - // then - expect(result.status).toBe("dispatched") - expect(promptCalls).toBe(1) - }) - test("#given dispatch hold has expired #when the same session prompts again #then the next promptAsync is accepted", async () => { // given let promptCalls = 0 diff --git a/src/hooks/todo-continuation-enforcer/continuation-injection-agent-name.test.ts b/src/hooks/todo-continuation-enforcer/continuation-injection-agent-name.test.ts index 093ad886e..fcf509cf4 100644 --- a/src/hooks/todo-continuation-enforcer/continuation-injection-agent-name.test.ts +++ b/src/hooks/todo-continuation-enforcer/continuation-injection-agent-name.test.ts @@ -45,4 +45,38 @@ describe("injectContinuation agent names", () => { // then expect(capturedAgent).toBe("Hephaestus - Deep Agent") }) + + test("#given resolved agent is an invisible-prefixed config key #when continuation is injected #then promptAsync receives the display name", async () => { + // given + let capturedAgent: string | undefined + const ctx = unsafeTestValue[0]["ctx"]>({ + directory: "/tmp/test", + client: { + session: { + todo: async () => ({ data: [{ id: "1", content: "todo", status: "pending", priority: "high" }] }), + promptAsync: async (input: { body: { agent?: string } }) => { + capturedAgent = input.body.agent + return {} + }, + }, + }, + }) + const sessionStateStore = unsafeTestValue[0]["sessionStateStore"]>({ + getExistingState: () => ({ inFlight: false, lastInjectedAt: 0, consecutiveFailures: 0 }), + }) + + // when + await injectContinuation({ + ctx, + sessionID: "ses_invisible_lowercase_builtin_agent", + resolvedInfo: { + agent: "\u200Bhephaestus", + model: { providerID: "openai", modelID: "gpt-5.5" }, + }, + sessionStateStore, + }) + + // then + expect(capturedAgent).toBe("Hephaestus - Deep Agent") + }) }) diff --git a/src/hooks/todo-continuation-enforcer/continuation-injection.ts b/src/hooks/todo-continuation-enforcer/continuation-injection.ts index 8b799595d..af79a62a2 100644 --- a/src/hooks/todo-continuation-enforcer/continuation-injection.ts +++ b/src/hooks/todo-continuation-enforcer/continuation-injection.ts @@ -20,7 +20,7 @@ import { log } from "../../shared/logger" import { isSqliteBackend } from "../../shared/opencode-storage-detection" import { getAgentConfigKey, - normalizeAgentForPromptKey, + normalizeAgentForPrompt, stripAgentListSortPrefix, } from "../../shared/agent-display-names" import { dispatchInternalPrompt, isInternalPromptDispatchAccepted } from "../shared/prompt-async-gate" @@ -132,7 +132,11 @@ export async function injectContinuation(args: { tools = tools ?? previousMessage?.tools } - const promptAgent = resolveRegisteredAgentName(agentName) ?? normalizeAgentForPromptKey(agentName) + const agentConfigKey = getAgentConfigKey(agentName ?? "") + const registeredAgentName = resolveRegisteredAgentName(agentName) + const promptAgent = registeredAgentName !== undefined && registeredAgentName !== agentConfigKey + ? registeredAgentName + : normalizeAgentForPrompt(agentName) const launchAgent = promptAgent ? stripAgentListSortPrefix(promptAgent).trim() || undefined : undefined if (promptAgent && skipAgents.some(s => getAgentConfigKey(s) === getAgentConfigKey(promptAgent))) { diff --git a/src/hooks/todo-continuation-enforcer/idle-event.test.ts b/src/hooks/todo-continuation-enforcer/idle-event.test.ts index 989b0b378..5563f45d3 100644 --- a/src/hooks/todo-continuation-enforcer/idle-event.test.ts +++ b/src/hooks/todo-continuation-enforcer/idle-event.test.ts @@ -40,7 +40,18 @@ function createStateStore(): { resetContinuationProgress: (sessionID: string) => { resetCalls.push(sessionID) }, - cancelCountdown: () => {}, + cancelCountdown: () => { + if (state.countdownTimer) { + clearTimeout(state.countdownTimer) + state.countdownTimer = undefined + } + if (state.countdownInterval) { + clearInterval(state.countdownInterval) + state.countdownInterval = undefined + } + state.countdownStartedAt = undefined + state.inFlight = false + }, cleanup: () => {}, cancelAllCountdowns: () => {}, shutdown: () => {}, @@ -136,4 +147,85 @@ describe("handleSessionIdle", () => { // reset is still called only once (from the first idle) expect(resetCalls).toHaveLength(1) }) + + it("skips todo continuation when the previous internal continuation has only an empty unknown assistant turn", async () => { + // given + const sessionID = "ses_internal_noop_tail" + const { store, trackCalls, state } = createStateStore() + const ctx = { + client: { + session: { + messages: async () => ({ + data: [ + { + info: { role: "user" }, + parts: [{ type: "text", text: "continue\n", synthetic: true }], + }, + { + info: { role: "assistant", finish: "unknown", time: { completed: Date.now() } }, + parts: [{ type: "step-start" }, { type: "step-finish", reason: "unknown" }], + }, + ], + }), + todo: async () => ({ + data: [ + { id: "todo-1", content: "Finish init-deep", status: "pending", priority: "high" }, + ], + }), + }, + }, + directory: "/tmp/test", + } + + try { + // when + await handleSessionIdle({ + ctx: ctx as never, + sessionID, + sessionStateStore: store, + }) + + // then + expect(trackCalls).toEqual([]) + expect(state.countdownStartedAt).toBeUndefined() + } finally { + store.cancelCountdown(sessionID) + } + }) + + it("skips todo continuation when session messages cannot be inspected", async () => { + // given + const sessionID = "ses_messages_fetch_fails" + const { store, trackCalls, state } = createStateStore() + const ctx = { + client: { + session: { + messages: async () => { + throw new Error("message endpoint failed") + }, + todo: async () => ({ + data: [ + { id: "todo-1", content: "Finish init-deep", status: "pending", priority: "high" }, + ], + }), + }, + }, + directory: "/tmp/test", + } + + try { + // when + await handleSessionIdle({ + ctx: ctx as never, + sessionID, + sessionStateStore: store, + }) + + // then + expect(trackCalls).toEqual([]) + expect(state.countdownStartedAt).toBeUndefined() + } finally { + store.cancelCountdown(sessionID) + } + }) }) diff --git a/src/hooks/todo-continuation-enforcer/idle-event.ts b/src/hooks/todo-continuation-enforcer/idle-event.ts index bd4306096..1288dd9b3 100644 --- a/src/hooks/todo-continuation-enforcer/idle-event.ts +++ b/src/hooks/todo-continuation-enforcer/idle-event.ts @@ -4,6 +4,7 @@ import { getSessionAgent } from "../../features/claude-code-session-state" import { normalizeSDKResponse } from "../../shared" import { getAgentConfigKey } from "../../shared/agent-display-names" import { log } from "../../shared/logger" +import { latestAssistantTurnBlocksInternalPrompt } from "../../shared/prompt-async-gate/pending-tool-turn" import { isLastAssistantMessageAborted } from "./abort-detection" import { acknowledgeCompactionGuard, isCompactionGuardActive } from "./compaction-guard" @@ -92,8 +93,13 @@ export async function handleSessionIdle(args: { log(`[${HOOK_NAME}] Skipped: pending question awaiting user response`, { sessionID }) return } + if (latestAssistantTurnBlocksInternalPrompt(prefetchedMessages)) { + log(`[${HOOK_NAME}] Skipped: pending internal continuation response`, { sessionID }) + return + } } catch (error) { - log(`[${HOOK_NAME}] Messages fetch failed, continuing`, { sessionID, error: String(error) }) + log(`[${HOOK_NAME}] Messages fetch failed, skipping continuation`, { sessionID, error: String(error) }) + return } let todos: Todo[] = [] diff --git a/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts b/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts index 62c2a8179..ff9e463f8 100644 --- a/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts +++ b/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts @@ -39,6 +39,44 @@ describe("hasUnansweredQuestion", () => { expect(hasUnansweredQuestion(messages)).toBe(true) }) + test("#given last assistant message with OpenCode question tool field #when checking pending question #then returns true", () => { + const messages = [ + { info: { role: "user" } }, + { + info: { role: "assistant" }, + parts: [ + { type: "tool", tool: "question" }, + ], + }, + ] + expect(hasUnansweredQuestion(messages)).toBe(true) + }) + + test("#given last assistant message with OpenCode ask_user_question tool field #when checking pending question #then returns true", () => { + const messages = [ + { info: { role: "user" } }, + { + info: { role: "assistant" }, + parts: [ + { type: "tool", tool: "ask_user_question" }, + ], + }, + ] + expect(hasUnansweredQuestion(messages)).toBe(true) + }) + + test("#given completed OpenCode question tool #when checking pending question #then returns false", () => { + const messages = [ + { + info: { role: "assistant" }, + parts: [ + { type: "tool", tool: "question", state: { status: "completed" } }, + ], + }, + ] + expect(hasUnansweredQuestion(messages)).toBe(false) + }) + test("given user message after question (answered), returns false", () => { const messages = [ { diff --git a/src/hooks/todo-continuation-enforcer/pending-question-detection.ts b/src/hooks/todo-continuation-enforcer/pending-question-detection.ts index f9bd4881e..2217ff6b3 100644 --- a/src/hooks/todo-continuation-enforcer/pending-question-detection.ts +++ b/src/hooks/todo-continuation-enforcer/pending-question-detection.ts @@ -5,7 +5,9 @@ import { HOOK_NAME } from "./constants" interface MessagePart { type?: string name?: string + tool?: string toolName?: string + state?: { status?: string } text?: string synthetic?: boolean } @@ -16,6 +18,20 @@ interface Message { parts?: MessagePart[] } +const QUESTION_TOOL_NAMES = new Set(["question", "ask_user_question", "askuserquestion"]) + +function getToolName(part: MessagePart): string | undefined { + return part.name ?? part.tool ?? part.toolName +} + +function isUnansweredQuestionTool(part: MessagePart): boolean { + const toolName = getToolName(part) + if (!QUESTION_TOOL_NAMES.has(toolName?.toLowerCase() ?? "")) { + return false + } + return part.state?.status !== "completed" +} + export function hasUnansweredQuestion(messages: Message[]): boolean { if (!messages || messages.length === 0) return false @@ -33,8 +49,8 @@ export function hasUnansweredQuestion(messages: Message[]): boolean { if (role === "assistant" && msg.parts) { const hasQuestion = msg.parts.some( (part) => - (part.type === "tool_use" || part.type === "tool-invocation") && - (part.name === "question" || part.toolName === "question"), + (part.type === "tool" || part.type === "tool_use" || part.type === "tool-invocation") && + isUnansweredQuestionTool(part), ) if (hasQuestion) { log(`[${HOOK_NAME}] Detected pending question tool in last assistant message`) diff --git a/src/shared/prompt-async-gate/message-inspection-error.ts b/src/shared/prompt-async-gate/message-inspection-error.ts new file mode 100644 index 000000000..c737eef6f --- /dev/null +++ b/src/shared/prompt-async-gate/message-inspection-error.ts @@ -0,0 +1,8 @@ +import { isRecord } from "../record-type-guard" + +export function isPromptMessageInspectionAborted(error: unknown): boolean { + if (error instanceof Error && error.name === "MessageAbortedError") { + return true + } + return isRecord(error) && error.name === "MessageAbortedError" +} diff --git a/src/shared/prompt-async-gate/pending-tool-turn-metadata.test.ts b/src/shared/prompt-async-gate/pending-tool-turn-metadata.test.ts new file mode 100644 index 000000000..9dc7bd76a --- /dev/null +++ b/src/shared/prompt-async-gate/pending-tool-turn-metadata.test.ts @@ -0,0 +1,56 @@ +/// + +import { describe, expect, test } from "bun:test" +import { latestAssistantTurnBlocksInternalPrompt } from "./pending-tool-turn" + +describe("latestAssistantTurnBlocksInternalPrompt metadata-only messages", () => { + test("#given empty unknown assistant turn has no parts loaded #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "user", + time: { created: 1000 }, + }, + }, + { + info: { + role: "assistant", + finish: "unknown", + time: { created: 2000, completed: 3000 }, + }, + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given completed tool-calls assistant turn has no parts loaded #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "user", + time: { created: 1000 }, + }, + }, + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) +}) diff --git a/src/shared/prompt-async-gate/pending-tool-turn.test.ts b/src/shared/prompt-async-gate/pending-tool-turn.test.ts new file mode 100644 index 000000000..5b0d496db --- /dev/null +++ b/src/shared/prompt-async-gate/pending-tool-turn.test.ts @@ -0,0 +1,286 @@ +import { describe, expect, test } from "bun:test" +import { latestAssistantTurnBlocksInternalPrompt } from "./pending-tool-turn" + +describe("latestAssistantTurnBlocksInternalPrompt", () => { + test("#given completed assistant question tool has no real user answer #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "user", + time: { created: 1000 }, + }, + parts: [{ type: "text", text: "start" }], + }, + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { + type: "tool_use", + name: "question", + state: { status: "error" }, + }, + ], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given internal wake follows an unanswered question #when checking prompt safety #then the internal wake does not count as an answer", () => { + // given + const messages = [ + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { + type: "tool-invocation", + toolName: "question", + state: { status: "error" }, + }, + ], + }, + { + info: { + role: "user", + time: { created: 4000 }, + }, + parts: [{ type: "text", text: "wake\n" }], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given opencode question tool field has no real user answer #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { + type: "tool", + tool: "question", + state: { status: "error" }, + }, + ], + }, + { + info: { + role: "user", + time: { created: 4000 }, + }, + parts: [{ type: "text", text: "wake\n" }], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given opencode ask-user-question tool field has no real user answer #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { + type: "tool", + tool: "ask_user_question", + state: { status: "error" }, + }, + ], + }, + { + info: { + role: "user", + time: { created: 4000 }, + }, + parts: [{ type: "text", text: "wake\n" }], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given answered question tool completed #when checking prompt safety #then internal prompts are not blocked by that question", () => { + // given + const messages = [ + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { + type: "tool", + tool: "question", + state: { + status: "completed", + output: "User has answered your questions: \"format\"=\"Flat codex:sess_abc\".", + }, + }, + ], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(false) + }) + + test("#given real user answer follows a question #when checking prompt safety #then internal prompts are not blocked by that question", () => { + // given + const messages = [ + { + info: { + role: "assistant", + finish: "tool-calls", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { + type: "tool_use", + name: "question", + state: { status: "error" }, + }, + ], + }, + { + info: { + role: "user", + time: { created: 4000 }, + }, + parts: [{ type: "text", text: "continue without the question tool" }], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(false) + }) + + test("#given latest message is an internal continuation user turn #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "assistant", + finish: "stop", + time: { created: 1000, completed: 2000 }, + }, + parts: [{ type: "text", text: "working" }], + }, + { + info: { + role: "user", + time: { created: 3000 }, + }, + parts: [{ type: "text", text: "continue\n", synthetic: true }], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given internal continuation gets only an empty unknown assistant turn #when checking prompt safety #then internal prompts stay blocked", () => { + // given + const messages = [ + { + info: { + role: "user", + time: { created: 1000 }, + }, + parts: [{ type: "text", text: "continue\n", synthetic: true }], + }, + { + info: { + role: "assistant", + finish: "unknown", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { type: "step-start" }, + { type: "step-finish", reason: "unknown" }, + ], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(true) + }) + + test("#given internal continuation receives assistant text #when checking prompt safety #then internal prompts are not blocked", () => { + // given + const messages = [ + { + info: { + role: "user", + time: { created: 1000 }, + }, + parts: [{ type: "text", text: "continue\n", synthetic: true }], + }, + { + info: { + role: "assistant", + finish: "unknown", + time: { created: 2000, completed: 3000 }, + }, + parts: [ + { type: "step-start" }, + { type: "text", text: "I will keep working." }, + { type: "step-finish", reason: "unknown" }, + ], + }, + ] + + // when + const blocks = latestAssistantTurnBlocksInternalPrompt(messages) + + // then + expect(blocks).toBe(false) + }) +}) diff --git a/src/shared/prompt-async-gate/pending-tool-turn.ts b/src/shared/prompt-async-gate/pending-tool-turn.ts index 6d1d007b5..9c5363a4a 100644 --- a/src/shared/prompt-async-gate/pending-tool-turn.ts +++ b/src/shared/prompt-async-gate/pending-tool-turn.ts @@ -1,10 +1,16 @@ import { log } from "../logger" import { - isSyntheticOrInternalUserMessage, - type InternalInitiatorMessageLike, - type InternalInitiatorTextPartLike, -} from "../internal-initiator-marker" + messageCompleted, + messageFinish, + messageHasQuestionTool, + messageHasSubstantiveAssistantOutput, + messageHasUnresolvedTool, + messageHasWaitingTool, + messageIsSyntheticOrInternalUser, + messageRole, +} from "./prompt-message-state" import { isRecord } from "../record-type-guard" +import { isPromptMessageInspectionAborted } from "./message-inspection-error" import { withDispatchTimeout } from "./timing" import type { PromptDispatchClient, PromptMessagesQuery, PromptSessionName } from "./types" @@ -36,120 +42,46 @@ function getMessagesData(response: unknown): unknown[] { return Array.isArray(response) ? response : [] } -function messageRole(message: unknown): string | undefined { - if (!isRecord(message)) { - return undefined - } - const info = message.info - if (isRecord(info) && typeof info.role === "string") { - return info.role - } - return typeof message.role === "string" ? message.role : undefined -} - -function messageFinish(message: unknown): string | true | undefined { - if (!isRecord(message)) { - return undefined - } - const info = message.info - if (isRecord(info)) { - if (info.finish === true) { - return true - } - if (typeof info.finish === "string" && info.finish.length > 0) { - return info.finish - } - } - if (message.finish === true) { - return true - } - return typeof message.finish === "string" && message.finish.length > 0 ? message.finish : undefined -} - -function messageCompleted(message: unknown): boolean { - if (!isRecord(message)) { - return false - } - const info = message.info - const time = isRecord(info) && isRecord(info.time) ? info.time : undefined - const completed = time?.completed - if (typeof completed === "number" && Number.isFinite(completed)) { - return true - } - return typeof completed === "string" && completed.length > 0 -} - -function toInternalInitiatorTextPartLike(part: unknown): InternalInitiatorTextPartLike { - const result: InternalInitiatorTextPartLike = {} - if (!isRecord(part)) { - return result - } - - if (typeof part.type === "string") { - result.type = part.type - } - if (typeof part.text === "string") { - result.text = part.text - } - if (typeof part.synthetic === "boolean") { - result.synthetic = part.synthetic - } - return result -} - -function toInternalInitiatorMessageLike(message: unknown): InternalInitiatorMessageLike | undefined { - if (!isRecord(message)) { - return undefined - } - - const result: InternalInitiatorMessageLike = {} - const info = message.info - if (isRecord(info) && typeof info.role === "string") { - result.info = { role: info.role } - } - if (typeof message.role === "string") { - result.role = message.role - } - if (Array.isArray(message.parts)) { - result.parts = message.parts.map(toInternalInitiatorTextPartLike) - } - return result -} - -function messageIsSyntheticOrInternalUser(message: unknown): boolean { - const initiatorMessage = toInternalInitiatorMessageLike(message) - return initiatorMessage !== undefined && isSyntheticOrInternalUserMessage(initiatorMessage) -} - -function partIsWaitingOnTool(part: unknown): boolean { - if (!isRecord(part)) { - return false - } - if ( - part.type !== "tool" - && part.type !== "tool_use" - && part.type !== "tool-call" - && part.type !== "tool-invocation" - ) { - return false - } - - const state = part.state - if (!isRecord(state)) { - return false - } - return state.status === "pending" || state.status === "running" -} - -export function latestAssistantTurnBlocksInternalPrompt(messages: unknown[]): boolean { +export function latestAssistantTurnHasUnansweredQuestion(messages: unknown[]): boolean { for (let index = messages.length - 1; index >= 0; index--) { const message = messages[index] const role = messageRole(message) if (role === "assistant") { + return messageHasQuestionTool(message) + } + if (role === "user") { + if (messageIsSyntheticOrInternalUser(message)) { + continue + } + return false + } + } + return false +} + +export function latestAssistantTurnBlocksInternalPrompt(messages: unknown[]): boolean { + let sawAssistantAfterLatestUser = false + for (let index = messages.length - 1; index >= 0; index--) { + const message = messages[index] + const role = messageRole(message) + if (role === "assistant") { + sawAssistantAfterLatestUser = true + if (messageHasQuestionTool(message)) { + return true + } + const finish = messageFinish(message) + if (finish === "tool-calls") { + return !isRecord(message) || !Array.isArray(message.parts) || messageHasUnresolvedTool(message) + } + if ( + (finish === undefined || finish === "unknown") + && !messageHasSubstantiveAssistantOutput(message) + ) { + return true + } if (messageCompleted(message)) { return false } - const finish = messageFinish(message) if (finish === true) { return false } @@ -159,10 +91,13 @@ export function latestAssistantTurnBlocksInternalPrompt(messages: unknown[]): bo if (!isRecord(message) || !Array.isArray(message.parts)) { return finish === "tool-calls" } - return finish === "tool-calls" || message.parts.some(partIsWaitingOnTool) + return messageHasWaitingTool(message) } if (role === "user") { if (messageIsSyntheticOrInternalUser(message)) { + if (!sawAssistantAfterLatestUser) { + return true + } continue } return false @@ -201,6 +136,6 @@ export async function sessionLatestAssistantBlocksInternalPrompt(args: { source: args.source, error: String(error), }) - return false + return !isPromptMessageInspectionAborted(error) } } diff --git a/src/shared/prompt-async-gate/prompt-message-state.ts b/src/shared/prompt-async-gate/prompt-message-state.ts new file mode 100644 index 000000000..ffa17d377 --- /dev/null +++ b/src/shared/prompt-async-gate/prompt-message-state.ts @@ -0,0 +1,186 @@ +import { + isSyntheticOrInternalUserMessage, + type InternalInitiatorMessageLike, + type InternalInitiatorTextPartLike, +} from "../internal-initiator-marker" +import { isRecord } from "../record-type-guard" + +export function messageRole(message: unknown): string | undefined { + if (!isRecord(message)) { + return undefined + } + const info = message.info + if (isRecord(info) && typeof info.role === "string") { + return info.role + } + return typeof message.role === "string" ? message.role : undefined +} + +export function messageFinish(message: unknown): string | true | undefined { + if (!isRecord(message)) { + return undefined + } + const info = message.info + if (isRecord(info)) { + if (info.finish === true) { + return true + } + if (typeof info.finish === "string" && info.finish.length > 0) { + return info.finish + } + } + if (message.finish === true) { + return true + } + return typeof message.finish === "string" && message.finish.length > 0 ? message.finish : undefined +} + +export function messageCompleted(message: unknown): boolean { + if (!isRecord(message)) { + return false + } + const info = message.info + const time = isRecord(info) && isRecord(info.time) ? info.time : undefined + const completed = time?.completed + if (typeof completed === "number" && Number.isFinite(completed)) { + return true + } + return typeof completed === "string" && completed.length > 0 +} + +function toInternalInitiatorTextPartLike(part: unknown): InternalInitiatorTextPartLike { + const result: InternalInitiatorTextPartLike = {} + if (!isRecord(part)) { + return result + } + + if (typeof part.type === "string") { + result.type = part.type + } + if (typeof part.text === "string") { + result.text = part.text + } + if (typeof part.synthetic === "boolean") { + result.synthetic = part.synthetic + } + return result +} + +function toInternalInitiatorMessageLike(message: unknown): InternalInitiatorMessageLike | undefined { + if (!isRecord(message)) { + return undefined + } + + const result: InternalInitiatorMessageLike = {} + const info = message.info + if (isRecord(info) && typeof info.role === "string") { + result.info = { role: info.role } + } + if (typeof message.role === "string") { + result.role = message.role + } + if (Array.isArray(message.parts)) { + result.parts = message.parts.map(toInternalInitiatorTextPartLike) + } + return result +} + +export function messageIsSyntheticOrInternalUser(message: unknown): boolean { + const initiatorMessage = toInternalInitiatorMessageLike(message) + return initiatorMessage !== undefined && isSyntheticOrInternalUserMessage(initiatorMessage) +} + +const QUESTION_TOOL_NAMES = new Set(["question", "ask_user_question", "askuserquestion"]) + +function partToolName(part: Record): string | undefined { + if (typeof part.name === "string") { + return part.name + } + if (typeof part.tool === "string") { + return part.tool + } + return typeof part.toolName === "string" ? part.toolName : undefined +} + +function partIsToolCall(part: Record): boolean { + return ( + part.type === "tool" + || part.type === "tool_use" + || part.type === "tool-call" + || part.type === "tool-invocation" + ) +} + +function partIsQuestionTool(part: unknown): boolean { + if (!isRecord(part) || !partIsToolCall(part)) { + return false + } + const toolName = partToolName(part) + return toolName !== undefined && QUESTION_TOOL_NAMES.has(toolName.toLowerCase()) +} + +function partIsUnansweredQuestionTool(part: unknown): boolean { + if (!partIsQuestionTool(part) || !isRecord(part)) { + return false + } + const state = part.state + if (!isRecord(state)) { + return true + } + return state.status !== "completed" +} + +function partIsWaitingOnTool(part: unknown): boolean { + if (!isRecord(part)) { + return false + } + if (!partIsToolCall(part)) { + return false + } + + const state = part.state + if (!isRecord(state)) { + return false + } + return state.status === "pending" || state.status === "running" +} + +function partIsUnresolvedTool(part: unknown): boolean { + if (!isRecord(part) || !partIsToolCall(part)) { + return false + } + const state = part.state + if (!isRecord(state)) { + return true + } + return state.status !== "completed" +} + +function partHasSubstantiveAssistantOutput(part: unknown): boolean { + if (!isRecord(part)) { + return false + } + if (part.type === "step-start" || part.type === "step-finish") { + return false + } + if (part.type === "text") { + return typeof part.text === "string" && part.text.trim().length > 0 + } + return typeof part.type === "string" && part.type.length > 0 +} + +export function messageHasQuestionTool(message: unknown): boolean { + return isRecord(message) && Array.isArray(message.parts) && message.parts.some(partIsUnansweredQuestionTool) +} + +export function messageHasWaitingTool(message: unknown): boolean { + return isRecord(message) && Array.isArray(message.parts) && message.parts.some(partIsWaitingOnTool) +} + +export function messageHasUnresolvedTool(message: unknown): boolean { + return isRecord(message) && Array.isArray(message.parts) && message.parts.some(partIsUnresolvedTool) +} + +export function messageHasSubstantiveAssistantOutput(message: unknown): boolean { + return isRecord(message) && Array.isArray(message.parts) && message.parts.some(partHasSubstantiveAssistantOutput) +} diff --git a/src/tools/delegate-task/sync-continuation.ts b/src/tools/delegate-task/sync-continuation.ts index ce6bf0e99..73a52fdd2 100644 --- a/src/tools/delegate-task/sync-continuation.ts +++ b/src/tools/delegate-task/sync-continuation.ts @@ -172,6 +172,7 @@ export async function executeSyncContinuation( }, }, { queueBehavior: "defer", + checkToolState: false, }) } catch (promptError) { if (toastManager) {