diff --git a/src/builtins/tools.zig b/src/builtins/tools.zig index ce210a9be..f354dd92c 100644 --- a/src/builtins/tools.zig +++ b/src/builtins/tools.zig @@ -15,6 +15,7 @@ const compactor = @import("../core/compactor/compactor.zig"); const capability_retrieval = @import("../core/tooling/capability_retrieval.zig"); const permission_gate = @import("../core/permissions/permission_gate.zig"); const ask_user_question_impl = @import("../tools/agent/ask_user_question.zig"); +const question_answer = @import("../core/agent/question_answer.zig"); const subagent_impl = @import("../tools/agent/subagent.zig"); const vision_impl = @import("../tools/agent/vision.zig"); const edit_file_impl = @import("../tools/filesystem/edit_file.zig"); @@ -101,8 +102,10 @@ const install_skill_description = "Install a reusable skill from a supported source into fx managed skill storage. When to use: the user asks to install a skill or pastes a skills install command. When NOT to use: no installation is required, install packages, fetch unrelated repos, or modify project code."; const mcp_select_tool_description = "Exact-select one configured MCP/dynamic tool by name so its executable schema is advertised on the next model step. When to use: after discovering the exact specialized tool name in configured metadata. When NOT to use: guessing partial names, selecting built-in tools, or executing the dynamic tool directly."; -const ask_user_question_description = - "Ask the user 1-4 multiple-choice questions in interactive runs only when a concrete decision blocks progress after local files, git state, or tool output cannot answer it. When to use: choose among precise, mutually exclusive paths before acting, especially user-preference decisions. When NOT to use: safety-review escalation, discoverable facts, GitHub handles unless account/private-access specific, gh/auth/tool blockers, trivial yes/no checks, open-ended discussion, or noninteractive runs; noninteractive runs should surface a blocker in freeform text instead."; +const ask_user_question_description = std.fmt.comptimePrint( + "Ask the user 1-{d} multiple-choice questions in interactive runs only when a concrete decision blocks progress after local files, git state, or tool output cannot answer it. When to use: choose among precise, mutually exclusive paths before acting, especially user-preference decisions. When NOT to use: safety-review escalation, discoverable facts, GitHub handles unless account/private-access specific, gh/auth/tool blockers, trivial yes/no checks, open-ended discussion, or noninteractive runs; noninteractive runs should surface a blocker in freeform text instead.", + .{question_answer.max_questions}, +); const ask_user_question_option_schema = model_tool_schema.ObjectSchema{ .properties = &.{ .{ .name = "label", .json_type = .string, .description = "Short precise action label, 1-5 words." }, @@ -587,7 +590,7 @@ pub const ask_user_question = ToolSpec{ .description = ask_user_question_description, .input_schema = .{ .properties = &.{ - .{ .name = "questions", .json_type = .array, .bounds = &.{ .min_items = 1, .max_items = 4 }, .shape = &.{ .array_objects = &ask_user_question_question_schema } }, + .{ .name = "questions", .json_type = .array, .bounds = &.{ .min_items = 1, .max_items = question_answer.max_questions }, .shape = &.{ .array_objects = &ask_user_question_question_schema } }, }, .required = &.{"questions"}, }, @@ -807,7 +810,7 @@ test "built-in model-facing tool contract stays byte exact" { const actual_hex = std.fmt.bytesToHex(hasher.finalResult(), .lower); try std.testing.expectEqualStrings( - "921f1c5cf785e28616633763f261d942f8cd646ba76731d85f6f98febe66b737", + "bb643f827a9ffa27608b14c0e26b3b0a195f2909e7f1f7763c161f585adb6449", &actual_hex, ); } @@ -1438,7 +1441,8 @@ test "built-in ask_user_question owns product metadata schema and callbacks" { try std.testing.expect(std.mem.find(u8, ask_user_question.description, "interactive runs") != null); try std.testing.expect(std.mem.find(u8, ask_user_question.description, "noninteractive runs should surface a blocker in freeform text") != null); try std.testing.expect(std.mem.find(u8, schema_json, "\"name\":\"ask_user_question\"") != null); - try std.testing.expect(std.mem.find(u8, schema_json, "\"questions\":{\"type\":\"array\",\"minItems\":1,\"maxItems\":4") != null); + try std.testing.expect(std.mem.startsWith(u8, ask_user_question.description, "Ask the user 1-20 multiple-choice questions")); + try std.testing.expect(std.mem.find(u8, schema_json, "\"questions\":{\"type\":\"array\",\"minItems\":1,\"maxItems\":20") != null); try std.testing.expect(std.mem.find(u8, schema_json, "\"options\":{\"type\":\"array\",\"minItems\":2,\"maxItems\":6") != null); try std.testing.expect(std.mem.find(u8, schema_json, "Specific blocking decision shown to the user") != null); try std.testing.expect(std.mem.find(u8, schema_json, "do not ask for facts tools can inspect") != null); diff --git a/src/core/agent/question_answer.zig b/src/core/agent/question_answer.zig index d73171dbe..010518bc4 100644 --- a/src/core/agent/question_answer.zig +++ b/src/core/agent/question_answer.zig @@ -4,6 +4,10 @@ const types = @import("../shared/types.zig"); const Allocator = std.mem.Allocator; +/// Most questions one `ask_user_question` call may ask. The tool schema, the +/// argument parser, and saved-result decoding all share this bound. +pub const max_questions: u32 = 20; + const EncodeError = Allocator.Error || std.Io.Writer.Error || error{ AnswerCountMismatch, }; @@ -52,7 +56,7 @@ pub fn decodeJson( if (root != .array) return null; const items = root.array.items; - if (items.len < 1 or items.len > 4) return null; + if (items.len < 1 or items.len > max_questions) return null; const answers = try arena.alloc(types.QuestionAnswer, items.len); for (items, 0..) |item, index| { @@ -106,6 +110,36 @@ test "question answer decoding rejects non-result payloads" { try std.testing.expect((try decodeJson(arena, "[{\"question\":\"Q\",\"answer\":\"A\"}] trailing")) == null); } +test "question answers round-trip a full batch and reject one more" { + const alloc = std.testing.allocator; + const count = max_questions + 1; + var question_bufs: [count][24]u8 = undefined; + var answer_bufs: [count][24]u8 = undefined; + var entries: [count]types.QuestionBatchEntry = undefined; + var answers: [count][]const u8 = undefined; + for (&entries, &answers, &question_bufs, &answer_bufs, 0..) |*entry, *answer, *question_buf, *answer_buf, index| { + entry.* = .{ .question = try std.mem.print(question_buf, "Question {d}?", .{index + 1}), .options = &.{} }; + answer.* = try std.mem.print(answer_buf, "Answer {d}", .{index + 1}); + } + + var arena_state = std.heap.ArenaAllocator.init(alloc); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const full = try encodeJson(alloc, entries[0..max_questions], answers[0..max_questions]); + defer alloc.free(full); + const decoded = (try decodeJson(arena, full)).?; + try std.testing.expectEqual(@as(usize, max_questions), decoded.len); + for (decoded, entries[0..max_questions], answers[0..max_questions]) |pair, entry, answer| { + try std.testing.expectEqualStrings(entry.question, pair.question); + try std.testing.expectEqualStrings(answer, pair.answer); + } + + const oversized = try encodeJson(alloc, &entries, &answers); + defer alloc.free(oversized); + try std.testing.expect((try decodeJson(arena, oversized)) == null); +} + test "question answer codec exposes structural and allocation failures" { const entries = [_]types.QuestionBatchEntry{ .{ .question = "Continue?", .options = &.{} }, diff --git a/src/core/agent/question_prompt.zig b/src/core/agent/question_prompt.zig index 7070270de..26903df83 100644 --- a/src/core/agent/question_prompt.zig +++ b/src/core/agent/question_prompt.zig @@ -1695,3 +1695,116 @@ test "compact submission allocation failure preserves the unanswered input" { try std.testing.expectEqual(.all_decided, try prompt.apply(alloc, .submit)); try std.testing.expectEqualStrings("{\"value\":\"x\"}", prompt.entries.items[0].answer.?); } + +test "question prompt keeps a full ask batch consistent under seeded random input" { + const question_answer = @import("question_answer.zig"); + const batch_len = question_answer.max_questions; + // Pages are indexed with this type, and the prompt parks at the batch + // length once every page is answered. + comptime std.debug.assert(batch_len <= std.math.maxInt(@FieldType(QuestionPrompt, "current_index"))); + const option_count = 6; + // The synthetic freeform slot follows the model's options; two more + // ordinals exercise out-of-range selection. + const ordinal_count = option_count + 3; + + const alloc = std.testing.allocator; + var label_bufs: [batch_len][option_count][16]u8 = undefined; + var options: [batch_len][option_count]types.QuestionOption = undefined; + var question_bufs: [batch_len][16]u8 = undefined; + var entries: [batch_len]types.QuestionBatchEntry = undefined; + for (&entries, &options, &label_bufs, &question_bufs, 0..) |*entry, *entry_options, *entry_labels, *question_buf, question| { + for (entry_options, entry_labels, 0..) |*option, *label_buf, index| { + option.* = .{ .label = try std.mem.print(label_buf, "Q{d}:O{d}", .{ question + 1, index + 1 }) }; + } + entry.* = .{ + .question = try std.mem.print(question_buf, "Question {d}", .{question + 1}), + .options = entry_options, + }; + } + + const Walk = struct { + fn answeredCount(prompt: *const QuestionPrompt) usize { + var count: usize = 0; + for (prompt.entries.items) |entry| count += @intFromBool(entry.answer != null); + return count; + } + + /// Checks one event against the prompt and returns true once the + /// batch is fully decided. + fn check(prompt: *const QuestionPrompt, event: QuestionInputEvent, answered: *usize) !bool { + const len = prompt.entries.items.len; + const now = answeredCount(prompt); + // Navigation and re-answering never drop an earlier answer. + try std.testing.expect(now >= answered.*); + answered.* = now; + switch (event) { + .all_decided => { + try std.testing.expectEqual(len, now); + try std.testing.expectEqual(len, @as(usize, prompt.current_index)); + return true; + }, + .decision => |index| { + try std.testing.expect(index < len); + try std.testing.expect(prompt.entries.items[index].answer != null); + try std.testing.expect(prompt.current_index < len); + try std.testing.expect(prompt.entries.items[prompt.current_index].answer == null); + }, + .none, .redraw => try std.testing.expect(prompt.current_index < len), + .cancelled, .limit_exceeded => return error.TestUnexpectedResult, + } + try std.testing.expect(now < len); + return false; + } + }; + + var prompt: QuestionPrompt = .{}; + defer prompt.deinit(alloc); + var seed: u64 = 0; + while (seed < 64) : (seed += 1) { + var prng: std.Random.DefaultPrng = .init(seed); + const random = prng.random(); + try prompt.syncFrom(alloc, &entries); + try std.testing.expectEqual(@as(usize, batch_len), prompt.projection().?.entry_count); + + var answered: usize = 0; + var decided = false; + var step: usize = 0; + while (!decided and step < 400) : (step += 1) { + const event = switch (random.uintLessThan(u8, 8)) { + 0 => try prompt.apply(alloc, .next_entry), + 1 => try prompt.apply(alloc, .{ .move_choice = if (random.boolean()) .previous else .next }), + 2, 3 => try prompt.apply(alloc, .{ .select_ordinal = random.uintLessThan(u8, ordinal_count) }), + 4 => try prompt.apply(alloc, .submit), + 5 => try prompt.apply(alloc, .{ .insert_ascii = 'a' + random.uintLessThan(u8, 26) }), + 6 => try prompt.apply(alloc, .backspace), + else => prompt.retreatToPrevious(), + }; + decided = try Walk.check(&prompt, event, &answered); + } + // Pages the walk left unanswered take their first option. The walk + // may have parked on an answered page, which costs one extra + // selection before each selection answers a new page. + var remaining = batch_len - answered + 1; + while (!decided) : (remaining -= 1) { + try std.testing.expect(remaining > 0); + decided = try Walk.check(&prompt, try prompt.apply(alloc, .{ .select_ordinal = 0 }), &answered); + } + + for (prompt.entries.items, 0..) |entry, page| { + const option = entry.options.items[entry.confirmed_choice_index.?]; + if (option.is_freeform_slot) { + try std.testing.expectEqualStrings(entry.confirmed_freeform_buffer.items, entry.answer.?); + } else { + var prefix_buf: [8]u8 = undefined; + try std.testing.expectEqualStrings(option.label, entry.answer.?); + try std.testing.expect(std.mem.startsWith( + u8, + entry.answer.?, + try std.mem.print(&prefix_buf, "Q{d}:", .{page + 1}), + )); + } + } + prompt.resetAfterSubmission(alloc); + try std.testing.expect(!prompt.isActive()); + } +} diff --git a/src/core/tooling/tool_runtime.zig b/src/core/tooling/tool_runtime.zig index c2b985a54..c8b8512f5 100644 --- a/src/core/tooling/tool_runtime.zig +++ b/src/core/tooling/tool_runtime.zig @@ -3245,6 +3245,120 @@ test "ask_user_question worker bridge preserves allocator ownership" { try std.testing.expectEqualStrings("Run tests", state.answers.?[0]); } +const FullQuestionBatchExecution = struct { + ctx: Context, + arena: Allocator, + call: ToolCall, + max_tool_result_bytes: usize, + result: ?ToolExecutionResult = null, + err: ?anyerror = null, + + fn run(self: *FullQuestionBatchExecution) void { + self.result = executeToolCallAuthorized(self.ctx, .{ + .call_allocator = self.arena, + .result_allocator = self.arena, + .call = self.call, + .authority = .ordinary, + .session_grants = &.{}, + .advertised_dynamic_tool_names = &.{}, + .max_tool_result_bytes = self.max_tool_result_bytes, + }) catch |err| { + self.err = err; + return; + }; + } +}; + +test "ask_user_question carries a full batch from model arguments through the worker prompt to the model result" { + const question_answer = @import("../agent/question_answer.zig"); + const question_prompt = @import("../agent/question_prompt.zig"); + const alloc = std.testing.allocator; + const batch_len = question_answer.max_questions; + var arena_state = std.heap.ArenaAllocator.init(alloc); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var args: std.Io.Writer.Allocating = .init(arena); + try args.writer.writeAll("{\"questions\":["); + for (0..batch_len) |question| { + if (question > 0) try args.writer.writeByte(','); + try args.writer.print( + "{{\"question\":\"Batch question {d}?\",\"options\":[{{\"label\":\"Keep {d}\"}},{{\"label\":\"Change {d}\",\"description\":\"Apply the change.\"}}]}}", + .{ question + 1, question + 1, question + 1 }, + ); + } + try args.writer.writeAll("]}"); + + const tools = [_]tool_dispatch.Tool{test_builtin_tools.ask_user_question}; + var rt = TestRuntime{ .tool_registry = .{ .tools = tools[0..] } }; + // The worker side of the question bridge allocates with `c_allocator`. + defer { + rt.worker.deinit(std.heap.c_allocator); + rt.session.deinit(alloc); + } + var execution = FullQuestionBatchExecution{ + .ctx = rt.context(), + .arena = arena, + .call = .{ .id = "ask-full-batch", .name = "ask_user_question", .arguments_json = args.written() }, + .max_tool_result_bytes = rt.max_tool_result_bytes, + }; + const thread = try std.Thread.spawn(.{}, FullQuestionBatchExecution.run, .{&execution}); + var thread_joined = false; + defer if (!thread_joined) { + rt.worker.requestStop(); + thread.join(); + }; + + // The interactive side receives the whole batch as one prompt. + var snapshot = try waitForQuestionBridgeSnapshot(&rt.worker); + defer snapshot.deinit(alloc); + try std.testing.expectEqual(@as(usize, batch_len), snapshot.entries.len); + var prompt: question_prompt.QuestionPrompt = .{}; + defer prompt.deinit(alloc); + try prompt.syncFrom(alloc, snapshot.entries); + + // Even pages keep their first option and odd pages take the second. + for (0..batch_len) |page| { + const projection = prompt.projection().?; + try std.testing.expectEqual(page, projection.current_index); + var question_buf: [32]u8 = undefined; + try std.testing.expectEqualStrings( + try std.mem.print(&question_buf, "Batch question {d}?", .{page + 1}), + projection.current_entry.?.question, + ); + const event = try prompt.apply(alloc, .{ .select_ordinal = @intCast(page % 2) }); + if (page + 1 < batch_len) { + try std.testing.expectEqual(question_prompt.QuestionInputEvent{ .decision = @intCast(page) }, event); + } else { + try std.testing.expectEqual(question_prompt.QuestionInputEvent.all_decided, event); + } + } + var labels: [batch_len][]const u8 = undefined; + for (&labels, prompt.entries.items) |*label, entry| label.* = entry.answer.?; + try rt.worker.submitQuestionBatchAnswer(std.heap.c_allocator, &labels); + thread.join(); + thread_joined = true; + + if (execution.err) |err| return err; + const result = execution.result.?; + try std.testing.expectEqual(tool_contracts.ToolExecutionStatus.success, result.status); + const answers = (try question_answer.decodeJson(arena, result.model_output)).?; + try std.testing.expectEqual(@as(usize, batch_len), answers.len); + for (answers, 0..) |pair, page| { + var question_buf: [32]u8 = undefined; + var answer_buf: [32]u8 = undefined; + try std.testing.expectEqualStrings( + try std.mem.print(&question_buf, "Batch question {d}?", .{page + 1}), + pair.question, + ); + try std.testing.expectEqualStrings( + try std.mem.print(&answer_buf, "{s} {d}", .{ if (page % 2 == 0) "Keep" else "Change", page + 1 }), + pair.answer, + ); + } + try std.testing.expect(try rt.worker.snapshotPendingQuestionBatch(alloc) == null); +} + test "ask_user_question execution uses supplied registry entry and interactive host capability" { const alloc = std.testing.allocator; var arena_state = std.heap.ArenaAllocator.init(alloc); diff --git a/src/tools/agent/ask_user_question.zig b/src/tools/agent/ask_user_question.zig index 8a719404f..5d56a2400 100644 --- a/src/tools/agent/ask_user_question.zig +++ b/src/tools/agent/ask_user_question.zig @@ -2,6 +2,7 @@ const std = @import("std"); const observation = @import("../../core/agent/runtime/observation.zig"); const question_answer = @import("../../core/agent/question_answer.zig"); const core_types = @import("../../core/shared/types.zig"); +const testing_allocator = @import("../../core/shared/testing_allocator.zig"); const text_utils = @import("../../core/shared/text_utils.zig"); const tool_dispatch = @import("../../core/tooling/tool_dispatch.zig"); @@ -15,6 +16,10 @@ pub const cancel_sentinel = "(user cancelled the question)"; pub const not_available_sentinel = "(ask_user_question is only available in the interactive shell; ask the user freeform instead)"; const invalid_args_sentinel = "(ask_user_question: invalid arguments; provide {questions})"; +const question_count_sentinel = std.fmt.comptimePrint( + "(ask_user_question: provide 1 to {d} questions)", + .{question_answer.max_questions}, +); const QuestionBatchParseError = Allocator.Error || error{ InvalidArguments, @@ -183,7 +188,7 @@ fn parseQuestionBatch(arena: Allocator, args_json: []const u8) QuestionBatchPars if (questions_value != .array) return error.QuestionsNotArray; const questions_arr = questions_value.array; - if (questions_arr.items.len < 1 or questions_arr.items.len > 4) return error.QuestionCount; + if (questions_arr.items.len < 1 or questions_arr.items.len > question_answer.max_questions) return error.QuestionCount; var entries_buf: std.ArrayList(core_types.QuestionBatchEntry) = .empty; try entries_buf.ensureTotalCapacity(arena, questions_arr.items.len); @@ -253,7 +258,7 @@ fn invalidArgsBody(alloc: Allocator, err: QuestionBatchParseError) Allocator.Err error.InvalidArguments => invalid_args_sentinel, error.MissingQuestions => "(ask_user_question: missing required array \"questions\")", error.QuestionsNotArray => "(ask_user_question: \"questions\" must be an array)", - error.QuestionCount => "(ask_user_question: provide 1 to 4 questions)", + error.QuestionCount => question_count_sentinel, error.QuestionNotObject => "(ask_user_question: each question must be an object with a \"question\" and \"options\")", error.MissingQuestionText => "(ask_user_question: each question requires a \"question\" string)", error.QuestionTextNotString => "(ask_user_question: question \"question\" must be a string)", @@ -368,7 +373,7 @@ test "ask_user_question validates arguments and returns active sentinels" { try expectRequesterOutput("[]", invalid_args_sentinel); try expectRequesterOutput("{}", "(ask_user_question: missing required array \"questions\")"); try expectRequesterOutput("{\"questions\":{}}", "(ask_user_question: \"questions\" must be an array)"); - try expectRequesterOutput("{\"questions\":[]}", "(ask_user_question: provide 1 to 4 questions)"); + try expectRequesterOutput("{\"questions\":[]}", "(ask_user_question: provide 1 to 20 questions)"); try expectRequesterOutput("{\"questions\":[1]}", "(ask_user_question: each question must be an object with a \"question\" and \"options\")"); try expectRequesterOutput("{\"questions\":[{}]}", "(ask_user_question: each question requires a \"question\" string)"); try expectRequesterOutput("{\"questions\":[{\"question\":1}]}", "(ask_user_question: question \"question\" must be a string)"); @@ -424,6 +429,140 @@ test "ask_user_question flattens model-controlled prompt text for terminal displ ); } +const batch_option_count = 6; + +/// Writes `ask_user_question` arguments with `count` questions of six options +/// each. The caller owns the returned slice. +fn questionBatchArgs(alloc: Allocator, count: usize) ![]u8 { + var out: std.Io.Writer.Allocating = .init(alloc); + defer out.deinit(); + try out.writer.writeAll("{\"questions\":["); + for (0..count) |question| { + if (question > 0) try out.writer.writeByte(','); + try out.writer.print("{{\"question\":\"Question {d}?\",\"options\":[", .{question + 1}); + for (0..batch_option_count) |option| { + if (option > 0) try out.writer.writeByte(','); + try out.writer.print( + "{{\"label\":\"Q{d} option {d}\",\"description\":\"Consequence {d}.{d}\"}}", + .{ question + 1, option + 1, question + 1, option + 1 }, + ); + } + try out.writer.writeAll("]}"); + } + try out.writer.writeAll("]}"); + return out.toOwnedSlice(); +} + +/// Answers question `i` with option `i % 6` and records whether every entry +/// arrived in order with all of its options. +const FullBatchRequester = struct { + calls: usize = 0, + entry_count: usize = 0, + entries_in_order: bool = true, + + fn request(raw_ctx: ?*anyopaque, alloc: Allocator, entries: []const core_types.QuestionBatchEntry) anyerror!?[][]u8 { + const self: *FullBatchRequester = @ptrCast(@alignCast(raw_ctx.?)); + self.calls += 1; + self.entry_count = entries.len; + for (entries, 0..) |entry, index| { + var expected_buf: [32]u8 = undefined; + const expected = try std.mem.print(&expected_buf, "Question {d}?", .{index + 1}); + if (!std.mem.eql(u8, entry.question, expected) or entry.options.len != batch_option_count) { + self.entries_in_order = false; + } + } + + const answers = try alloc.alloc([]u8, entries.len); + errdefer alloc.free(answers); + var filled: usize = 0; + errdefer for (answers[0..filled]) |answer| alloc.free(answer); + while (filled < entries.len) : (filled += 1) { + answers[filled] = try alloc.dupe(u8, entries[filled].options[filled % batch_option_count].label); + } + return answers; + } +}; + +test "ask_user_question accepts a full batch and rejects one more question" { + const alloc = std.testing.allocator; + const full_args = try questionBatchArgs(alloc, question_answer.max_questions); + defer alloc.free(full_args); + + var requester = FullBatchRequester{}; + const output = try executeWithRequester(alloc, full_args, .{ + .ctx = &requester, + .response_alloc = alloc, + .request = FullBatchRequester.request, + }); + defer alloc.free(output); + + try std.testing.expectEqual(@as(usize, 1), requester.calls); + try std.testing.expectEqual(@as(usize, question_answer.max_questions), requester.entry_count); + try std.testing.expect(requester.entries_in_order); + + var arena_state = std.heap.ArenaAllocator.init(alloc); + defer arena_state.deinit(); + const answers = (try question_answer.decodeJson(arena_state.allocator(), output)).?; + try std.testing.expectEqual(@as(usize, question_answer.max_questions), answers.len); + for (answers, 0..) |pair, index| { + var question_buf: [32]u8 = undefined; + var answer_buf: [32]u8 = undefined; + try std.testing.expectEqualStrings( + try std.mem.print(&question_buf, "Question {d}?", .{index + 1}), + pair.question, + ); + try std.testing.expectEqualStrings( + try std.mem.print(&answer_buf, "Q{d} option {d}", .{ index + 1, index % batch_option_count + 1 }), + pair.answer, + ); + } + + const oversized_args = try questionBatchArgs(alloc, question_answer.max_questions + 1); + defer alloc.free(oversized_args); + var oversized_requester = FullBatchRequester{}; + const rejected = try executeWithRequester(alloc, oversized_args, .{ + .ctx = &oversized_requester, + .response_alloc = alloc, + .request = FullBatchRequester.request, + }); + defer alloc.free(rejected); + try std.testing.expectEqualStrings("(ask_user_question: provide 1 to 20 questions)", rejected); + try std.testing.expectEqual(@as(usize, 0), oversized_requester.calls); +} + +test "ask_user_question full batch frees every allocation when any allocation fails" { + const args = try questionBatchArgs(std.testing.allocator, question_answer.max_questions); + defer std.testing.allocator.free(args); + + // Sweep each allocation of a full batch, parse through answer encoding, + // until a run completes without an injected failure. + var fail_index: usize = 0; + while (true) : (fail_index += 1) { + var failing = std.testing.FailingAllocator.init(testing_allocator.no_resize, .{ .fail_index = fail_index }); + const alloc = failing.allocator(); + var requester = FullBatchRequester{}; + const completed = if (executeWithRequester(alloc, args, .{ + .ctx = &requester, + .response_alloc = alloc, + .request = FullBatchRequester.request, + })) |output| blk: { + alloc.free(output); + break :blk !failing.has_induced_failure; + } else |err| switch (err) { + // The answer encoder reports allocation failure as a write failure. + error.OutOfMemory, error.WriteFailed => blk: { + try std.testing.expect(failing.has_induced_failure); + break :blk false; + }, + else => return err, + }; + try std.testing.expectEqual(failing.allocated_bytes, failing.freed_bytes); + if (completed) break; + } + // The requester alone allocates one answer per question. + try std.testing.expect(fail_index > question_answer.max_questions); +} + test "ask_user_question cancellation returns sentinel" { const alloc = std.testing.allocator; const output = try executeWithRequester( diff --git a/tests/e2e/tui-decision-prompts.test.ts b/tests/e2e/tui-decision-prompts.test.ts index 75a521fb4..395a5e4c5 100644 --- a/tests/e2e/tui-decision-prompts.test.ts +++ b/tests/e2e/tui-decision-prompts.test.ts @@ -311,6 +311,46 @@ function outerQuestionBatchCall() { ]); } +const FULL_QUESTION_BATCH = 20; +const QUESTION_COUNT_REJECTION = + `(ask_user_question: provide 1 to ${FULL_QUESTION_BATCH} questions)`; + +function batchQuestionText(index: number) { + return `Batch decision ${String(index + 1).padStart(2, "0")}?`; +} + +function batchQuestions(count: number) { + return Array.from({ length: count }, (_, index) => ({ + question: batchQuestionText(index), + options: [ + { label: `Keep ${index + 1}`, description: "Leave this setting as it is." }, + { label: `Change ${index + 1}`, description: "Apply the proposed change." }, + ], + })); +} + +function outerBatchQuestionCall(id: string, count: number) { + return outerToolCalls([ + { id, name: "ask_user_question", input: { questions: batchQuestions(count) } }, + ]); +} + +function toolResultText(body: string, callId: string): string { + const prompt = (JSON.parse(body) as { prompt: Array<{ content: unknown }> }).prompt; + for (const message of prompt) { + if (!Array.isArray(message.content)) continue; + for (const part of message.content as Array>) { + if (part.type !== "tool-result" || part.toolCallId !== callId) continue; + const output = part.output as { value?: unknown } | undefined; + if (typeof output?.value !== "string") { + throw new Error(`Tool result ${callId} has no text output`); + } + return output.value; + } + } + throw new Error(`Missing tool result for ${callId}`); +} + function outerLongQuestionAnswerCall() { return outerToolCalls([ { @@ -2487,6 +2527,98 @@ describe.skipIf(SKIP)("tui: decision prompt input isolation", () => { TIMEOUT, ); + test( + "a full 20-question batch opens as one prompt, survives navigation and resize, and returns every answer in order", + async () => { + const finalMarker = "full question batch handled"; + const ctx = await launchScenario([ + outerBatchQuestionCall("question_batch_oversized", FULL_QUESTION_BATCH + 1), + outerBatchQuestionCall("question_batch_full", FULL_QUESTION_BATCH), + outerText(finalMarker), + ]); + const waitForPage = (page: number) => + waitForPaneState( + ctx.session, + `full batch page ${page} of ${FULL_QUESTION_BATCH}`, + (value) => + value.includes(batchQuestionText(page - 1)) && + value.includes(`Question ${page} of ${FULL_QUESTION_BATCH}`), + TIMEOUT, + ); + + await ctx.session.sendText("Ask every setup decision at once."); + await waitForPage(1); + + // The model is offered the full batch size, and one more question is + // rejected without opening a prompt. + const tools = (JSON.parse(ctx.gateway.requests[0]!.body) as { + tools: Array<{ name: string; description: string; inputSchema: any }>; + }).tools; + const askTool = tools.find((tool) => tool.name === "ask_user_question"); + expect(askTool?.inputSchema.properties.questions.maxItems).toBe(FULL_QUESTION_BATCH); + expect(askTool?.description).toStartWith(`Ask the user 1-${FULL_QUESTION_BATCH} `); + expect(ctx.gateway.requests).toHaveLength(2); + expect(toolResultText(ctx.gateway.requests[1]!.body, "question_batch_oversized")) + .toBe(QUESTION_COUNT_REJECTION); + + // Bursts of tab keys land on the right page and wrap past the last one. + await ctx.session.sendKeys(Array(13).fill("Tab").join(" ")); + await waitForPage(14); + await ctx.session.sendKeys(Array(FULL_QUESTION_BATCH - 13).fill("Tab").join(" ")); + await waitForPage(1); + await ctx.session.sendKeys(Array(13).fill("Tab").join(" ")); + await waitForPage(14); + + // A narrow terminal keeps the current page, and the counter returns + // once the hint row has room again. + await ctx.session.resizeWindow(48, 24); + await waitForPaneState( + ctx.session, + "narrow full batch page", + (value) => value.includes(batchQuestionText(13)) && value.includes("Change 14"), + TIMEOUT, + ); + await ctx.session.resizeWindow(120, 40); + await waitForPage(14); + await ctx.session.sendKeys(Array(FULL_QUESTION_BATCH - 13).fill("Tab").join(" ")); + await waitForPage(1); + + // One burst answers every page: even pages keep, odd pages change. + const choices = Array.from({ length: FULL_QUESTION_BATCH }, (_, index) => + index % 2 === 0 ? "1" : "2" + ); + await ctx.session.sendLiteralText(choices.join("")); + await ctx.session.waitForText(finalMarker, TIMEOUT); + + expect(ctx.gateway.requests).toHaveLength(3); + const answers = JSON.parse( + toolResultText(ctx.gateway.requests[2]!.body, "question_batch_full"), + ) as Array<{ question: string; answer: string }>; + expect(answers).toEqual( + Array.from({ length: FULL_QUESTION_BATCH }, (_, index) => ({ + question: batchQuestionText(index), + answer: `${index % 2 === 0 ? "Keep" : "Change"} ${index + 1}`, + })), + ); + + // The transcript lists every answer once, hanging under its numbered + // question even after the number grows to two digits. + const scrollback = await ctx.session.captureFullScrollback(); + expect(scrollback).not.toContain(`of ${FULL_QUESTION_BATCH + 1}`); + const lines = scrollback.split("\n").map((line) => line.trimEnd()); + for (let index = 0; index < FULL_QUESTION_BATCH; index += 1) { + const prefix = ` ${index + 1}) `; + const resolution = `${prefix}${batchQuestionText(index)}`; + expect(lines.filter((line) => line === resolution)).toHaveLength(1); + expect(lines[lines.indexOf(resolution) + 1]).toBe( + `${" ".repeat(prefix.length)}${answers[index]!.answer}`, + ); + } + await assertProcessAliveAndClean(ctx); + }, + TIMEOUT * 2, + ); + test( "raw Escape cancels a question turn before a new prompt", async () => { diff --git a/tests/e2e/tui-resume.test.ts b/tests/e2e/tui-resume.test.ts index 4c55d65bc..09e24c726 100644 --- a/tests/e2e/tui-resume.test.ts +++ b/tests/e2e/tui-resume.test.ts @@ -5895,6 +5895,124 @@ test.skipIf(!tmuxAvailable())( TIMEOUT * 4, ); +test.skipIf(!tmuxAvailable())( + "a full 20-question batch replays every answer after resume", + async () => { + const root = realpathSync(mkdtempSync(join(tmpdir(), "fx-tui-resume-question-batch-"))); + const home = join(root, "home"); + const workspace = join(root, "workspace"); + const stderrPath = join(root, "stderr.log"); + mkdirSync(home); + mkdirSync(workspace); + const workspaceRoot = realpathSync(workspace); + const batchSize = 20; + const question = (index: number) => + `RESUME_BATCH_QUESTION_${String(index + 1).padStart(2, "0")}`; + const answer = (index: number) => + `${index % 2 === 0 ? "Keep" : "Change"} ${index + 1}`; + const completion = "RESUME_QUESTION_BATCH_COMPLETED"; + const followUp = "RESUME_QUESTION_BATCH_FOLLOW_UP"; + const gateways: Array> = []; + let active: TmuxSession | null = null; + + // Each answer hangs under its numbered question, aligned with the + // question text even once the number has two digits. + const expectEveryAnswer = (scrollback: string) => { + const lines = scrollback.split("\n").map((line) => line.trimEnd()); + for (let index = 0; index < batchSize; index += 1) { + const prefix = ` ${index + 1}) `; + const row = lines.indexOf(`${prefix}${question(index)}`); + expect(row).toBeGreaterThanOrEqual(0); + expect(lines[row + 1]).toBe(`${" ".repeat(prefix.length)}${answer(index)}`); + } + expect(scrollback).not.toContain("● Asked"); + }; + + try { + const initialGateway = startFakeGateway([ + fakeGatewayToolCall("resume_question_batch", "ask_user_question", { + questions: Array.from({ length: batchSize }, (_, index) => ({ + question: question(index), + options: [ + { label: `Keep ${index + 1}`, description: "Leave it as it is." }, + { label: `Change ${index + 1}`, description: "Apply the change." }, + ], + })), + }), + fakeGatewayFinalText(completion), + ]); + gateways.push(initialGateway); + writeFileSync(stderrPath, ""); + active = await TmuxSession.create({ + cmd: FX_BIN, + cwd: workspaceRoot, + env: gatewayEnv(home, initialGateway), + stderrPath, + width: 100, + height: 32, + }); + await active.waitForComposer(TIMEOUT); + await active.sendText("Ask the prepared batch of questions."); + await active.waitForText(`Question 1 of ${batchSize}`, TIMEOUT); + await active.sendLiteralText( + Array.from({ length: batchSize }, (_, index) => (index % 2 === 0 ? "1" : "2")).join(""), + ); + expectEveryAnswer(await waitForScrollback(active, completion)); + await active.sendText("/quit"); + expect(await active.waitForSessionEnd()).toBe(true); + await active.kill(); + active = null; + + let resumedBody = ""; + const resumedGateway = startFakeGateway([ + (body: string) => { + resumedBody = body; + return fakeGatewayFinalText(followUp); + }, + ]); + gateways.push(resumedGateway); + active = await TmuxSession.create({ + cmd: `${FX_BIN} --resume-last`, + cwd: workspaceRoot, + env: gatewayEnv(home, resumedGateway), + stderrPath, + width: 100, + height: 32, + }); + await active.waitForComposer(TIMEOUT); + expectEveryAnswer(await waitForScrollback(active, completion)); + + await active.sendText("Continue after the resumed batch."); + await active.waitForText(followUp, TIMEOUT); + const resumedPrompt = (JSON.parse(resumedBody) as { + prompt: Array<{ content: unknown }>; + }).prompt; + const replayed = resumedPrompt + .flatMap((message) => Array.isArray(message.content) ? message.content : []) + .find((part: Record) => + part.type === "tool-result" && part.toolCallId === "resume_question_batch" + ) as { output: { value: string } } | undefined; + expect(JSON.parse(replayed!.output.value)).toEqual( + Array.from({ length: batchSize }, (_, index) => ({ + question: question(index), + answer: answer(index), + })), + ); + expect(active.isPaneAlive()).toBe(true); + expect(readFileSync(stderrPath, "utf8")).toBe(""); + await active.sendText("/quit"); + expect(await active.waitForSessionEnd()).toBe(true); + await active.kill(); + active = null; + } finally { + if (active) await active.kill(); + for (const gateway of gateways) gateway.stop(); + rmSync(root, { recursive: true, force: true }); + } + }, + TIMEOUT * 3, +); + test.skipIf(!tmuxAvailable())( "recorded file diffs survive resume and retain their Ctrl-O detail", async () => {