From 9bf9143a1729238529a2fba8f874ff04ad425038 Mon Sep 17 00:00:00 2001 From: "vercel-gh-bot-2[bot]" <282331341+vercel-gh-bot-2[bot]@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:47:22 +0000 Subject: [PATCH] fix(eve): resolve pending input on message follow-up Co-authored-by: ruiconti <1834568+ruiconti@users.noreply.github.com> --- .changeset/resolve-follow-up-inputs.md | 5 + docs/concepts/sessions-runs-and-streaming.md | 6 +- docs/tools/human-in-the-loop.md | 4 +- .../authored-always-unrelated-input.eval.ts | 33 +--- .../evals/hitl/text-approve.eval.ts | 28 +-- .../hitl/unrelated-message-queued.eval.ts | 36 +--- .../eve/src/harness/input-requests.test.ts | 165 +++++++++++++++--- packages/eve/src/harness/input-requests.ts | 106 +++++------ ...nerate-approval-resume.integration.test.ts | 109 ++++++++++++ packages/eve/src/harness/tool-loop.test.ts | 84 ++++----- packages/eve/src/harness/tool-loop.ts | 2 +- 11 files changed, 364 insertions(+), 214 deletions(-) create mode 100644 .changeset/resolve-follow-up-inputs.md diff --git a/.changeset/resolve-follow-up-inputs.md b/.changeset/resolve-follow-up-inputs.md new file mode 100644 index 000000000..dd564ebb2 --- /dev/null +++ b/.changeset/resolve-follow-up-inputs.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +Follow-up messages now resolve pending input before the conversation continues: unanswered tool approvals are denied and unanswered questions are dismissed instead of leaving the message queued. diff --git a/docs/concepts/sessions-runs-and-streaming.md b/docs/concepts/sessions-runs-and-streaming.md index 6e2a9fbb7..d41aa36c3 100644 --- a/docs/concepts/sessions-runs-and-streaming.md +++ b/docs/concepts/sessions-runs-and-streaming.md @@ -85,13 +85,13 @@ curl -X POST http://127.0.0.1:2000/eve/v1/session/ \ The follow-up reuses the same durable session: same history, same state. -If the session is waiting on a human-in-the-loop approval, a matching text reply such as `approve` or `deny` answers the approval. Other follow-up text is held until the approval is answered, so an unrelated message does not implicitly deny the pending tool call. +If the session is waiting on a human-in-the-loop approval, send an `inputResponses` entry to approve or deny it. A follow-up `message` denies each unanswered approval, then starts the follow-up turn. -If the session is waiting on `ask_question`, a follow-up message clears that pending request before the model continues. An exact option match or permitted freeform response answers the question; any other message marks the question unanswered and starts the follow-up turn. +If the session is waiting on `ask_question`, send an `inputResponses` entry to answer it. A follow-up `message` dismisses each unanswered question, then starts the follow-up turn. A response is stale when its request is no longer pending: the question or approval was already answered, cleared by a follow-up message, or cancelled. eve delivers a stale response to the model as a new user message, and the model decides whether the old selection still matters. A stale approval never authorizes the earlier tool call; the model must request the action and approval again if they are still needed. -Responses match pending requests by request ID, so a response to an older request stays a plain user message even while a different question or approval is pending. Like any follow-up message, a stale response clears a pending question and is held while an approval is pending. +Responses match pending requests by request ID, so a response to an older request stays a plain user message even while a different question or approval is pending. Like any follow-up message, a stale response clears a pending question and denies a pending approval. For deterministic ordering, send one follow-up at a time and wait for the next `session.waiting` event before sending another message to the same session. See [message delivery and queueing](./execution-model-and-durability#message-delivery-and-queueing) for the current runtime contract. diff --git a/docs/tools/human-in-the-loop.md b/docs/tools/human-in-the-loop.md index 3d0e1db4f..1e533f211 100644 --- a/docs/tools/human-in-the-loop.md +++ b/docs/tools/human-in-the-loop.md @@ -100,11 +100,11 @@ Approvals and questions share one protocol: 1. The model requests input (an approval, or an `ask_question`). 2. eve emits an `input.requested` stream event carrying the pending requests. 3. The turn parks at `session.waiting`, durably, for as long as it takes. -4. The client answers with `inputResponses` (structured, keyed by `requestId`) or a normal follow-up `message`. A follow-up whose text matches an option ID, option label, or numeric option index resolves automatically, including approval options such as `approve` and `deny`. +4. The client answers with `inputResponses`, structured and keyed by `requestId`. A normal follow-up `message` supersedes the pending batch instead: unanswered approvals are denied and unanswered questions are dismissed before the message continues the conversation. The run picks back up exactly where it parked. Because the pause is durable, nothing is held in memory while it waits — the process can restart and the parked turn survives. -For approval requests, unrelated follow-up text does not deny the tool call. eve keeps the approval pending and holds that text until the approval is answered, then replays it as the next message in the session. +A follow-up message never remains queued behind pending input. eve resolves unanswered approvals as denied and unanswered questions as ignored, then continues with the message. To answer a pending request, send an explicit `inputResponses` entry. See [Sessions, runs & streaming](/docs/concepts/sessions-runs-and-streaming) for the full event and resume contract that this builds on. diff --git a/e2e/fixtures/agent-tools-hitl/evals/hitl/authored-always-unrelated-input.eval.ts b/e2e/fixtures/agent-tools-hitl/evals/hitl/authored-always-unrelated-input.eval.ts index 4887add5f..5b88f72f5 100644 --- a/e2e/fixtures/agent-tools-hitl/evals/hitl/authored-always-unrelated-input.eval.ts +++ b/e2e/fixtures/agent-tools-hitl/evals/hitl/authored-always-unrelated-input.eval.ts @@ -3,44 +3,25 @@ import { defineEval } from "eve/evals"; const MARKER = "authored-always-unrelated-input-P7M2"; const TOOL_NAME = "gate"; -/** Regression reproduction for https://github.com/vercel/eve/issues/533. */ +/** Regression reproduction for https://github.com/vercel/eve/issues/1224. */ export default defineEval({ - description: - "HITL repro (#533): unrelated input does not replay an unresolved authored tool call.", + description: "HITL repro (#1224): unrelated input denies the authored tool call and continues.", async test(t) { const parked = await t.send(`Call the \`${TOOL_NAME}\` tool with marker "${MARKER}".`); parked.calledTool(TOOL_NAME, { status: "pending", count: 1 }); - const approval = t.requireInputRequest({ + t.requireInputRequest({ display: "confirmation", toolName: TOOL_NAME, }); - const unrelated = await t.send("Queue this unrelated note: ORBITAL-PINE-6C3R."); + const unrelated = await t.send("Reply with exactly ORBITAL-PINE-6C3R."); unrelated.expectOk(); - unrelated.notEvent("action.result", { - data: { result: { toolName: TOOL_NAME } }, - }); - unrelated.notEvent("step.started"); - unrelated.event("session.waiting", { count: 1 }); - - const approved = await t.respond({ - optionId: "approve", - requestId: approval.requestId, - }); - - approved.expectOk(); - approved.event("action.result", { - data: { - result: { - kind: "tool-result", - output: new RegExp(MARKER), - toolName: TOOL_NAME, - }, - status: "completed", - }, + unrelated.event("action.result", { + data: { result: { toolName: TOOL_NAME }, status: "rejected" }, count: 1, }); + unrelated.messageIncludes(/ORBITAL-PINE-6C3R/i); t.succeeded(); }, }); diff --git a/e2e/fixtures/agent-tools-hitl/evals/hitl/text-approve.eval.ts b/e2e/fixtures/agent-tools-hitl/evals/hitl/text-approve.eval.ts index 5ad2ff85c..04d46f535 100644 --- a/e2e/fixtures/agent-tools-hitl/evals/hitl/text-approve.eval.ts +++ b/e2e/fixtures/agent-tools-hitl/evals/hitl/text-approve.eval.ts @@ -1,37 +1,23 @@ import { defineEval } from "eve/evals"; -import { GUARDED_ECHO_TOKEN } from "./shared"; - /** - * HITL flow: a plain follow-up message whose text matches an approval option - * resolves the pending approval the same way as structured inputResponses. + * HITL flow: a plain follow-up message is not an approval response, even when + * its text matches the approve option. */ export default defineEval({ - description: "HITL smoke: text approve resolves a pending tool approval.", + description: "HITL smoke: text approve denies a pending tool approval.", async test(t) { const parked = await t.send('Call the guarded-echo tool with note "text-approve".'); parked.calledTool("guarded-echo", { status: "pending", count: 1 }); t.requireInputRequest({ display: "confirmation", toolName: "guarded-echo" }); - const approved = await t.send("approve"); - approved.expectOk(); - approved.event("action.result", { - data: { - result: { - kind: "tool-result", - output: new RegExp(GUARDED_ECHO_TOKEN), - toolName: "guarded-echo", - }, - status: "completed", - }, + const resumed = await t.send("approve"); + resumed.expectOk(); + resumed.event("action.result", { + data: { result: { toolName: "guarded-echo" }, status: "rejected" }, count: 1, }); t.succeeded(); - t.calledTool("guarded-echo", { - output: new RegExp(GUARDED_ECHO_TOKEN), - status: "completed", - count: 1, - }); }, }); diff --git a/e2e/fixtures/agent-tools-hitl/evals/hitl/unrelated-message-queued.eval.ts b/e2e/fixtures/agent-tools-hitl/evals/hitl/unrelated-message-queued.eval.ts index 2325bd472..990cdaf4e 100644 --- a/e2e/fixtures/agent-tools-hitl/evals/hitl/unrelated-message-queued.eval.ts +++ b/e2e/fixtures/agent-tools-hitl/evals/hitl/unrelated-message-queued.eval.ts @@ -1,48 +1,28 @@ import { defineEval } from "eve/evals"; -import { GUARDED_ECHO_TOKEN } from "./shared"; - /** - * HITL flow: unrelated text sent while an approval is pending must not deny - * the approval or disappear. The message is replayed after the approval - * receives an explicit terminal answer. + * HITL flow: unrelated text sent while an approval is pending denies the + * approval and replays the message immediately. */ export default defineEval({ - description: "HITL smoke: unrelated message during approval is queued.", + description: "HITL smoke: unrelated message denies approval and resumes.", async test(t) { const parked = await t.send('Call the guarded-echo tool with note "queued-approval".'); parked.calledTool("guarded-echo", { status: "pending", count: 1 }); - const request = t.requireInputRequest({ + t.requireInputRequest({ display: "confirmation", toolName: "guarded-echo", }); - const queued = await t.send( + const resumed = await t.send( "After the pending approval is resolved, reply with exactly QUEUED-HITL-OK.", ); - queued.expectOk(); - queued.notEvent("action.result", { + resumed.expectOk(); + resumed.event("action.result", { data: { result: { toolName: "guarded-echo" }, status: "rejected" }, - }); - queued.event("session.waiting", { count: 1 }); - - const approved = await t.respond({ - requestId: request.requestId, - optionId: "approve", - }); - approved.expectOk(); - approved.event("action.result", { - data: { - result: { - kind: "tool-result", - output: new RegExp(GUARDED_ECHO_TOKEN), - toolName: "guarded-echo", - }, - status: "completed", - }, count: 1, }); - approved.messageIncludes(/QUEUED-HITL-OK/i); + resumed.messageIncludes(/QUEUED-HITL-OK/i); t.succeeded(); }, diff --git a/packages/eve/src/harness/input-requests.test.ts b/packages/eve/src/harness/input-requests.test.ts index 65fc4586c..daf6246fc 100644 --- a/packages/eve/src/harness/input-requests.test.ts +++ b/packages/eve/src/harness/input-requests.test.ts @@ -192,7 +192,115 @@ describe("resolvePendingInput", () => { }); }); - it("resolves freeform question input from a follow-up message", () => { + it("denies authorization and dismisses input when a user message follows up", () => { + const session = setPendingInputBatch({ + event: { sequence: 7, stepIndex: 2, turnId: "turn_1" }, + requests: [ + { + action: { + callId: "question-call", + input: { prompt: "Pick one." }, + kind: "tool-call", + toolName: "ask_question", + }, + display: "text", + prompt: "Pick one.", + requestId: "question-1", + } satisfies InputRequest, + { + action: { + callId: "approval-call", + input: { command: "pwd" }, + kind: "tool-call", + toolName: "bash", + }, + allowFreeform: false, + display: "confirmation", + options: [ + { id: "approve", label: "Yes" }, + { id: "deny", label: "No" }, + ], + prompt: "Approve tool call: bash", + requestId: "approval-1", + } satisfies InputRequest, + ], + responseMessages: [], + session: createHarnessSession(), + }); + + const result = resolvePendingInput({ + stepInput: { message: "Do something else." }, + session, + }); + + expect(result.outcome).toBe("resolved"); + expect(result.messages.at(-1)).toMatchObject({ + content: expect.arrayContaining([ + expect.objectContaining({ + output: { + type: "json", + value: { status: "ignored" }, + }, + toolCallId: "question-call", + }), + expect.objectContaining({ approvalId: "approval-1", approved: false }), + ]), + role: "tool", + }); + expect(result.rejectedActions?.results).toHaveLength(1); + expect(result.deferredMessage).toBe(true); + expect(hasDeferredStepInput(result.session)).toBe(true); + expect(consumeDeferredStepInput({ session: result.session }).input).toEqual({ + message: "Do something else.", + }); + }); + + it("does not misclassify an approve-or-deny question as authorization", () => { + const session = setPendingInputBatch({ + requests: [ + { + action: { + callId: "question-call", + input: {}, + kind: "tool-call", + toolName: "ask_question", + }, + allowFreeform: false, + display: "select", + options: [ + { id: "approve", label: "Approve the plan" }, + { id: "deny", label: "Reject the plan" }, + ], + prompt: "Do you approve the plan?", + requestId: "question-1", + }, + ], + responseMessages: [], + session: createHarnessSession(), + }); + + const result = resolvePendingInput({ + stepInput: { message: "Let's discuss this first." }, + session, + }); + + expect(result.outcome).toBe("resolved"); + expect(result.deferredMessage).toBeUndefined(); + expect(result.rejectedActions).toBeUndefined(); + expect(result.messages.at(-1)).toEqual({ + content: [ + { + output: { type: "json", value: { status: "ignored" } }, + toolCallId: "question-call", + toolName: "ask_question", + type: "tool-result", + }, + ], + role: "tool", + }); + }); + + it("dismisses freeform question input when a user message follows up", () => { const session = setPendingInputBatch({ requests: [ { @@ -237,11 +345,7 @@ describe("resolvePendingInput", () => { { output: { type: "json", - value: { - optionId: undefined, - text: "Ignore that and continue.", - status: "answered", - }, + value: { status: "ignored" }, }, toolCallId: "question-call", toolName: "ask_question", @@ -293,19 +397,22 @@ describe("resolvePendingInput", () => { session: createHarnessSession(), }); - // Deliver an approval response AND a message simultaneously. const result = resolvePendingInput({ stepInput: { - inputResponses: [{ requestId: "approval-1", optionId: "deny" }], + inputResponses: [{ requestId: "approval-1", optionId: "approve" }], message: "Ignore that and say hi instead.", }, session, }); - // The approval should be resolved immediately. expect(result.outcome).toBe("resolved"); + expect(result.messages.at(-1)).toMatchObject({ + content: expect.arrayContaining([ + expect.objectContaining({ approvalId: "approval-1", approved: true }), + ]), + role: "tool", + }); - // The follow-up message should be deferred. expect(result.deferredMessage).toBe(true); expect(hasDeferredStepInput(result.session)).toBe(true); @@ -378,7 +485,7 @@ describe("resolvePendingInput", () => { expect(hasDeferredStepInput(deferred.session)).toBe(false); }); - it("resolves approval when follow-up text matches an option", () => { + it("denies authorization even when follow-up text matches an option", () => { const session = setPendingInputBatch({ requests: [ { @@ -425,21 +532,22 @@ describe("resolvePendingInput", () => { }); expect(result.outcome).toBe("resolved"); - expect(result.deferredMessage).toBeUndefined(); - expect(result.consumedMessage).toBe(true); - expect(result.messages.at(-1)).toEqual({ + expect(result.deferredMessage).toBe(true); + expect(result.messages.at(-1)).toMatchObject({ content: [ { approvalId: "approval-1", - approved: true, - reason: undefined, + approved: false, type: "tool-approval-response", }, + { toolCallId: "approval-call", type: "tool-result" }, ], role: "tool", }); - expect(getApprovedTools(result.session).has("bash")).toBe(true); - expect(hasDeferredStepInput(result.session)).toBe(false); + expect(getApprovedTools(result.session).has("bash")).toBe(false); + expect(consumeDeferredStepInput({ session: result.session }).input).toEqual({ + message: "approve", + }); }); it("records compound approval key when resolveApprovalKey is provided", () => { @@ -669,7 +777,7 @@ describe("resolvePendingInput", () => { expect(result.rejectedActions).toBeUndefined(); }); - it("keeps a pending approval and queues an unrelated follow-up message", () => { + it("denies a non-freeform approval when the user sends a follow-up message", () => { const session = setPendingInputBatch({ event: { sequence: 7, stepIndex: 2, turnId: "turn_1" }, requests: [ @@ -699,9 +807,22 @@ describe("resolvePendingInput", () => { session, }); - expect(result.outcome).toBe("unresolved"); - expect(result.rejectedActions).toBeUndefined(); - expect(result.messages).toEqual([{ content: "previous", role: "user" }]); + expect(result.outcome).toBe("resolved"); + expect(result.rejectedActions).toMatchObject({ + results: [ + { + callId: "approval-call", + output: { approval: { requestId: "approval-1", status: "denied" } }, + }, + ], + }); + expect(result.messages.at(-1)).toMatchObject({ + content: [ + { approvalId: "approval-1", approved: false, type: "tool-approval-response" }, + { toolCallId: "approval-call", type: "tool-result" }, + ], + role: "tool", + }); expect(hasDeferredStepInput(result.session)).toBe(true); const deferred = consumeDeferredStepInput({ session: result.session }); diff --git a/packages/eve/src/harness/input-requests.ts b/packages/eve/src/harness/input-requests.ts index 7d412e222..34fa599b0 100644 --- a/packages/eve/src/harness/input-requests.ts +++ b/packages/eve/src/harness/input-requests.ts @@ -4,8 +4,8 @@ import type { RuntimeToolCallActionRequest, RuntimeToolResultActionResult, } from "#runtime/actions/types.js"; +import { ASK_QUESTION_TOOL_NAME } from "#runtime/framework-tools/ask-question.js"; import type { InputRequest, InputResponse } from "#runtime/input/types.js"; -import { resolveTextToResponses } from "#channel/resolve-text.js"; import { coalesceTurnInputs } from "#harness/messages.js"; import { resolveToolCallInputObject } from "#harness/runtime-actions.js"; import { @@ -146,42 +146,18 @@ export function resolvePendingInput(input: { return { outcome: "continue", messages: baseHistory, session }; } - // Pending batch exists -- only resolve if we have actual responses. - const resolvedStepInput = resolveTextMessageInput(pendingBatch, stepInput); - const responses = resolvedStepInput?.inputResponses ?? []; + const responses = synthesizeFollowUpResponses(pendingBatch.requests, stepInput); const resolvesApprovalBatch = pendingBatch.requests.some((request) => isApprovalRequest(request)); - if (responses.length === 0 && resolvedStepInput?.message === undefined) { + if (responses.length === 0 && stepInput?.message === undefined) { return { outcome: "unresolved", messages: baseHistory, session }; } if (resolvesApprovalBatch && hasUnansweredApproval({ pendingBatch, responses })) { - session = queueDeferredStepInput(session, compactStepInput(resolvedStepInput)); + session = queueDeferredStepInput(session, compactStepInput(stepInput)); return { deferredMessage: true, outcome: "unresolved", messages: baseHistory, session }; } - if (responses.length === 0 && resolvedStepInput?.message !== undefined) { - // A follow-up message arrived for question-only input with no explicit - // responses. Keep the existing question semantics: mark unanswered - // question requests ignored so the model can continue with the message. - const toolParts = buildToolResponseParts(pendingBatch, []); - const messages: ModelMessage[] = [...baseHistory, ...pendingBatch.responseMessages]; - if (toolParts.length > 0) { - messages.push({ content: toolParts, role: "tool" }); - } - - const rejectedActions = buildRejectedActionBatch(pendingBatch, []); - session = clearPendingInputBatch(session); - - return { - consumedMessage: resolvedStepInput?.messageConsumed, - outcome: "resolved", - messages, - rejectedActions, - session, - }; - } - const limitContinuation = resolveSessionLimitContinuation({ requests: pendingBatch.requests, responses, @@ -214,18 +190,17 @@ export function resolvePendingInput(input: { context?: StepInput["context"]; message?: StepInput["message"]; } = {}; - if ((resolvedStepInput?.context?.length ?? 0) > 0) { - deferredInput.context = resolvedStepInput?.context; + if ((stepInput?.context?.length ?? 0) > 0) { + deferredInput.context = stepInput?.context; } - if (resolvedStepInput?.message !== undefined) { - deferredInput.message = resolvedStepInput.message; + if (stepInput?.message !== undefined) { + deferredInput.message = stepInput.message; } if (deferredInput.context !== undefined || deferredInput.message !== undefined) { session = queueDeferredStepInput(session, deferredInput); return { - consumedMessage: resolvedStepInput?.messageConsumed, deferredContext: deferredInput.context === undefined ? undefined : true, deferredMessage: deferredInput.message === undefined ? undefined : true, limitContinuation, @@ -238,7 +213,6 @@ export function resolvePendingInput(input: { } return { - consumedMessage: resolvedStepInput?.messageConsumed, limitContinuation, outcome: "resolved", messages, @@ -247,30 +221,7 @@ export function resolvePendingInput(input: { }; } -function resolveTextMessageInput( - pendingBatch: PendingInputBatch, - stepInput: StepInput | undefined, -): (StepInput & { readonly messageConsumed?: boolean }) | undefined { - if (typeof stepInput?.message !== "string" || (stepInput.inputResponses?.length ?? 0) > 0) { - return stepInput; - } - - const responses = resolveTextToResponses(stepInput.message, pendingBatch.requests); - if (responses.length === 0) { - return stepInput; - } - - return compactStepInput({ - ...stepInput, - inputResponses: responses, - messageConsumed: true, - message: undefined, - }); -} - -function compactStepInput( - input: (StepInput & { readonly messageConsumed?: boolean }) | undefined, -): StepInput & { readonly messageConsumed?: boolean } { +function compactStepInput(input: StepInput | undefined): StepInput { if (input === undefined) { return {}; } @@ -279,7 +230,6 @@ function compactStepInput( context?: StepInput["context"]; inputResponses?: StepInput["inputResponses"]; message?: StepInput["message"]; - messageConsumed?: boolean; outputSchema?: StepInput["outputSchema"]; } = {}; @@ -292,9 +242,6 @@ function compactStepInput( if (input.message !== undefined) { result.message = input.message; } - if (input.messageConsumed === true) { - result.messageConsumed = true; - } if (input.outputSchema !== undefined) { result.outputSchema = input.outputSchema; } @@ -313,7 +260,6 @@ function hasUnansweredApproval(input: { } type ResolvePendingInputResult = { - readonly consumedMessage?: boolean; readonly deferredContext?: boolean; readonly deferredMessage?: boolean; /** @@ -579,6 +525,35 @@ function buildToolResponseParts( return parts; } +/** + * Resolves every unanswered request when a user message supersedes the pending + * batch. Tool approvals are denied, questions are dismissed, and explicit + * responses always win. + */ +function synthesizeFollowUpResponses( + requests: readonly InputRequest[], + stepInput: StepInput | undefined, +): InputResponse[] { + const explicitResponses = stepInput?.inputResponses ?? []; + if (stepInput?.message === undefined) { + return [...explicitResponses]; + } + + const explicitResponseIds = new Set(explicitResponses.map((response) => response.requestId)); + return [ + ...explicitResponses, + ...requests.flatMap((request) => { + if (explicitResponseIds.has(request.requestId)) { + return []; + } + if (isApprovalRequest(request)) { + return [{ optionId: "deny", requestId: request.requestId }]; + } + return [{ requestId: request.requestId }]; + }), + ]; +} + function buildToolResponsePartsForRequest( request: InputRequest, response: InputResponse | undefined, @@ -632,7 +607,7 @@ function buildToolResponsePartsForRequest( output: { type: "json", value: - response !== undefined + response?.optionId !== undefined || response?.text !== undefined ? { optionId: response.optionId, text: response.text, status: "answered" } : { status: "ignored" }, }, @@ -643,9 +618,10 @@ function buildToolResponsePartsForRequest( ]; } -/** Shared approval predicate: a request whose options are exactly `approve` / `deny`. */ +/** Shared approval predicate for non-question requests with the approval option pair. */ export function isApprovalRequest(request: InputRequest): boolean { return ( + request.action.toolName !== ASK_QUESTION_TOOL_NAME && request.options?.length === 2 && request.options[0]?.id === "approve" && request.options[1]?.id === "deny" diff --git a/packages/eve/src/harness/tool-loop-generate-approval-resume.integration.test.ts b/packages/eve/src/harness/tool-loop-generate-approval-resume.integration.test.ts index bae7a600a..d8afc2185 100644 --- a/packages/eve/src/harness/tool-loop-generate-approval-resume.integration.test.ts +++ b/packages/eve/src/harness/tool-loop-generate-approval-resume.integration.test.ts @@ -174,4 +174,113 @@ describe("tool loop generate approval resume (real AI SDK)", () => { role: "assistant", }); }); + + it("denies a pending approval when a user sends a freeform follow-up", async () => { + const execute = vi.fn(async () => "/workspace"); + const model = new MockLanguageModelV4({ + doGenerate: { + content: [{ text: "I will not run that command.", type: "text" }], + finishReason: { raw: undefined, unified: "stop" }, + usage, + warnings: [], + }, + modelId: "generate-approval-resume-model", + provider: "eve-integration-mock", + }); + const config: ToolLoopHarnessConfig = { + mode: "conversation", + resolveModel: async (): Promise => model, + tools: new Map([ + [ + "bash", + { + description: "Run a shell command.", + execute, + inputSchema: jsonSchema({ type: "object" }), + name: "bash", + }, + ], + ]), + }; + + const session = setPendingInputBatch({ + requests: [ + { + action: { + callId: "delete-temp-directory", + input: { command: "rm -rf /tmp/eve-test" }, + kind: "tool-call", + toolName: "bash", + }, + allowFreeform: false, + display: "confirmation", + options: [ + { id: "approve", label: "Yes" }, + { id: "deny", label: "No" }, + ], + prompt: "Approve deleting the temporary directory.", + requestId: "delete-temp-directory-approval", + }, + ], + responseMessages: [ + { + content: [ + { + input: { command: "rm -rf /tmp/eve-test" }, + toolCallId: "delete-temp-directory", + toolName: "bash", + type: "tool-call", + }, + { + approvalId: "delete-temp-directory-approval", + toolCallId: "delete-temp-directory", + type: "tool-approval-request", + }, + ], + role: "assistant", + }, + ], + session: { + agent: { + modelReference: { id: "generate-approval-resume-model" }, + system: "You are a test assistant.", + tools: [ + { + description: "Run a shell command.", + inputSchema: { type: "object" }, + name: "bash", + }, + ], + }, + compaction: { recentWindowSize: 10, threshold: 100_000 }, + continuationToken: "http:freeform-approval-session", + history: [{ content: "Delete the temporary directory.", role: "user" }], + sessionId: "freeform-approval-session", + }, + }); + + const result = await createToolLoopHarness(config)(session, { + message: "Never mind, do something else.", + }); + + expect(execute).not.toHaveBeenCalled(); + expect(model.doGenerateCalls).toHaveLength(1); + expect(findPart(model.doGenerateCalls[0]?.prompt ?? [], "tool-result")).toMatchObject({ + output: { type: "execution-denied" }, + toolCallId: "delete-temp-directory", + toolName: "bash", + }); + expect(typeof result.next).toBe("function"); + if (typeof result.next !== "function") { + throw new TypeError("Expected the denial to continue with the deferred user message."); + } + + await result.next(result.session); + + expect(model.doGenerateCalls).toHaveLength(2); + expect(model.doGenerateCalls[1]?.prompt.at(-1)).toMatchObject({ + content: [{ text: "Never mind, do something else.", type: "text" }], + role: "user", + }); + }); }); diff --git a/packages/eve/src/harness/tool-loop.test.ts b/packages/eve/src/harness/tool-loop.test.ts index 74ea2c3ed..e8c8a636f 100644 --- a/packages/eve/src/harness/tool-loop.test.ts +++ b/packages/eve/src/harness/tool-loop.test.ts @@ -1244,7 +1244,7 @@ describe("createToolLoopHarness", () => { expect(resumed.session.history).toContainEqual({ content: "Hi again", role: "user" }); }); - it("grants the budget when the user types the continue option as plain text", async () => { + it("does not treat a follow-up message as a session-limit response", async () => { setupMockAgent({ finishReason: "stop", response: { messages: [{ content: "Hello!", role: "assistant" }] }, @@ -1258,13 +1258,11 @@ describe("createToolLoopHarness", () => { const parked = await runStep(createLimitReachedSession(), { message: "Hi again" }); - // Surfaces without buttons deliver the answer as a plain message; - // resolveTextToResponses maps it onto the pending option. const resumed = await runStep(parked.session, { message: "continue" }); - expect(vi.mocked(ToolLoopAgent)).toHaveBeenCalledTimes(1); + expect(vi.mocked(ToolLoopAgent)).not.toHaveBeenCalled(); expect(resumed.next).toBeNull(); - expect(getSessionTokenLimitViolation(resumed.session)).toBeNull(); + expect(getSessionTokenLimitViolation(resumed.session)).not.toBeNull(); }); it("cancels the turn when the user declines the limit continuation prompt", async () => { @@ -6527,7 +6525,7 @@ describe("createToolLoopHarness", () => { expect(events.filter((event) => event.type === "action.result")).toHaveLength(1); }); - it("queues a follow-up user message until the pending tool approval resolves", async () => { + it("denies a pending approval before replaying a follow-up user message", async () => { const generateCalls: unknown[] = []; const agentResults = [ { @@ -6641,15 +6639,9 @@ describe("createToolLoopHarness", () => { message: "Hi instead.", }); - expect(firstResult.next).toBeNull(); - expect(generateCalls).toEqual([]); + expect(typeof firstResult.next).toBe("function"); + expect(generateCalls).toHaveLength(1); expect(hasDeferredStepInput(firstResult.session)).toBe(true); - - const deniedResult = await createToolLoopHarness(config)(firstResult.session, { - inputResponses: [{ requestId: "approval-1", optionId: "deny" }], - }); - - expect(typeof deniedResult.next).toBe("function"); expect(generateCalls[0]).toEqual([ { content: [ @@ -6689,7 +6681,7 @@ describe("createToolLoopHarness", () => { }, ]); - const secondResult = await createToolLoopHarness(config)(deniedResult.session); + const secondResult = await createToolLoopHarness(config)(firstResult.session); expect(secondResult.next).toBeNull(); expect((generateCalls[1] as { role: string; content: unknown }[]).at(-1)).toEqual({ @@ -6706,7 +6698,7 @@ describe("createToolLoopHarness", () => { }); }); - it("consumes text approval shortcuts without appending them as user messages", async () => { + it("treats text approval shortcuts as follow-up messages", async () => { const generateCalls: unknown[] = []; vi.mocked(ToolLoopAgent).mockImplementation(function ( @@ -6806,8 +6798,10 @@ describe("createToolLoopHarness", () => { ]), }); - await createToolLoopHarness(config)(session, { message: "approve" }); + const firstResult = await createToolLoopHarness(config)(session, { message: "approve" }); + expect(typeof firstResult.next).toBe("function"); + expect(hasDeferredStepInput(firstResult.session)).toBe(true); expect(generateCalls[0]).toEqual([ { content: [ @@ -6829,14 +6823,29 @@ describe("createToolLoopHarness", () => { content: [ { approvalId: "approval-1", - approved: true, - reason: undefined, + approved: false, + reason: "Tool execution was denied.", type: "tool-approval-response", }, + { + output: { + type: "execution-denied", + reason: "Tool execution was denied.", + }, + toolCallId: "call-1", + toolName: "guarded_echo", + type: "tool-result", + }, ], role: "tool", }, ]); + + await createToolLoopHarness(config)(firstResult.session); + expect((generateCalls[1] as ModelMessage[]).at(-1)).toEqual({ + content: "approve", + role: "user", + }); }); it("keeps channel context after the approval-response model call", async () => { @@ -6932,11 +6941,7 @@ describe("createToolLoopHarness", () => { }); }); - it("deferred message lands as last non-system message after explicit approval denial", async () => { - // Step 1: pending approval + user sends a follow-up message. The approval - // remains pending and the message is deferred. Step 2: the user denies the - // approval. Step 3: the deferred message is consumed and appears as the - // last message the model sees. + it("deferred message lands last after automatic approval denial", async () => { const generateCalls: Array> = []; const agentResults = [ { @@ -7049,31 +7054,19 @@ describe("createToolLoopHarness", () => { ]), }); - // Step 1: user sends "Do something else" while approval is pending. - // Approval remains pending; message is deferred. const firstResult = await createToolLoopHarness(config)(session, { message: "Do something else", }); - expect(firstResult.next).toBeNull(); - expect(generateCalls).toEqual([]); - - // Step 2: user denies the approval; the deferred message is NOT in this call. - const deniedResult = await createToolLoopHarness(config)(firstResult.session, { - inputResponses: [{ requestId: "approval-1", optionId: "deny" }], - }); - expect(typeof deniedResult.next).toBe("function"); - const step2Last = generateCalls[0]?.at(-1); - expect(step2Last?.role).toBe("tool"); + expect(typeof firstResult.next).toBe("function"); + expect(generateCalls[0]?.at(-1)?.role).toBe("tool"); - // Step 3: harness consumes the deferred message. - const secondResult = await createToolLoopHarness(config)(deniedResult.session); + const secondResult = await createToolLoopHarness(config)(firstResult.session); expect(secondResult.next).toBeNull(); + expect(generateCalls[1]?.at(-1)).toEqual({ + content: "Do something else", + role: "user", + }); - // The deferred user message is the last message the model sees. - const step3Last = generateCalls[1]?.at(-1); - expect(step3Last).toEqual({ content: "Do something else", role: "user" }); - - // History reflects the full conversation. expect(secondResult.session.history.at(-1)).toEqual({ content: "Sure, here you go.", role: "assistant", @@ -7351,8 +7344,7 @@ describe("createToolLoopHarness", () => { }), }); - // The follow-up resolves question-1 as freeform; the turn completes with - // nothing pending. + // The follow-up dismisses question-1; the turn completes with nothing pending. const followupResult = await runStep(session, { message: "Use current context instead.", }); @@ -7378,7 +7370,7 @@ describe("createToolLoopHarness", () => { role: "user", }); // The stale selection must not append a second tool result for - // question-1: only the freeform answer from the follow-up turn exists. + // question-1: only the ignored result from the follow-up turn exists. expect(modelMessages?.filter((message: ModelMessage) => message.role === "tool")).toHaveLength( 1, ); diff --git a/packages/eve/src/harness/tool-loop.ts b/packages/eve/src/harness/tool-loop.ts index a9f205249..5fe0e68b8 100644 --- a/packages/eve/src/harness/tool-loop.ts +++ b/packages/eve/src/harness/tool-loop.ts @@ -646,7 +646,7 @@ export function createToolLoopHarness(config: ToolLoopHarnessConfig): StepFn { } const userContent = normalizeUserContent(effectiveStepInput?.message); - if (userContent !== undefined && !pending.deferredMessage && !pending.consumedMessage) { + if (userContent !== undefined && !pending.deferredMessage) { // Staging writes FilePart bytes into the sandbox and replaces // each part's `data` with a compact `eve-sandbox:` URL. The // `messages` array — and everything that flows into