mirror of
https://github.com/vercel-labs/fx.git
synced 2026-10-28 10:15:17 +08:00
Allow up to 20 questions in one ask_user_question call
- The model can ask up to 20 questions in one ask_user_question call, so a related set of questions opens as one prompt instead of several prompts of 4. - The tool schema and description tell the model the new limit. - A call with more than 20 questions is rejected with "provide 1 to 20 questions", and no prompt opens. - Resumed sessions show every answer from a batch of up to 20 questions.
This commit is contained in:
7 files changed
+663
-9
No files matched your search
@@ -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);
|
||||
|
||||
@@ -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 = &.{} },
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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<Record<string, unknown>>) {
|
||||
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 () => {
|
||||
|
||||
@@ -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<ReturnType<typeof startFakeGateway>> = [];
|
||||
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<string, unknown>) =>
|
||||
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 () => {
|
||||
|
||||
Reference in new issue
Block a user