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/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) +}