From da3082aff89c282030687993d4e4046b18144565 Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Wed, 26 Aug 2026 09:52:21 +0800 Subject: [PATCH] Ask a group of decisions at once, not one at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `request_user_input` took one question. A turn that needed three raised them one after another, each hidden until the one before it was answered — and a second question you cannot see is a second question that cannot inform the first. It now takes up to three, drawn as one form with one Continue. The earlier message on `2ec0e12` gave the wrong reason for not doing this. It said several questions per call "would need a queue and a multi-question card", as though the runtime's single `pendingUserInput` forbade it. It never did: a call carrying three questions is still one blocking request and one continuation, which is exactly what a queue is not. That was a preference dressed as a constraint. The real cost is the two things below — per-question drafts, and a form that can outgrow its strip — and neither was a reason to refuse. The queue that *did* arrive in `22258e3` does not cover it either, for a reason worth writing down. `request_user_input` is `.sequential`, and a batch runs in parallel only when every call in it is, so two questions in one turn execute one after the other and `pendingUserInputs` never holds more than one. The queue and its `1 of N` are live for approvals and purely defensive for questions. Sequential is right for this tool — racing a question against other calls means output arriving under something still being read — so batching inside one call is the only way several questions reach a person together. The queue stays as it is. A question may be left unanswered, and the tool reports that as a null rather than an error. That needs saying in the tool's own description or a model reads the null as a failure and asks the same thing again, which puts the user in a loop they answered their way out of once already. So the description now says plainly: a null is the user declining to decide that one — choose, say which assumption you used, and do not ask again. Continue needs one answer, not all of them; it needs at least one because answering nothing is Cancel, and reporting that as success would hand the model a result whose only content is that it was not refused. Drafts become per question. Sharing one text field and one selection set across a form meant typing into the third question cleared the first, and the mutual exclusion between an option and a typed answer — which is per-question by definition — reached across all of them. `AgentUserQuestion` carries the id the model chose, so answers travel under the model's own names rather than an index we assign, and every question is reported back whether answered or not: an absent key would read as a bug where a null reads as a decision. Two things fell out of rendering it. A bare `ScrollView` capped at the form's maximum height is greedy, so a one-question card was handed 320pt of box for 120pt of question; `ViewThatFits` prefers the natural stack and falls back to scrolling only when the questions genuinely overflow. And `reported` is not private, because that shape is the contract with the model and a test should be able to hold it still — including that a secret, which `request_user_input` never asks for, is reported as skipped rather than echoed if one ever arrives there. Co-Authored-By: Claude Opus 5 (1M context) --- .../Application/Agent/AgentBuiltInTools.swift | 216 ++++++--- Termind/Application/Agent/AgentSecurity.swift | 75 ++- Termind/Design/DesignTokens.swift | 7 + Termind/Features/AI/ConversationModel.swift | 117 +++-- .../AI/ConversationTranscriptView.swift | 162 ++++--- TermindTests/AgentApprovalQueueTests.swift | 6 +- TermindTests/AgentUserInputTests.swift | 441 ++++++++++++++---- 7 files changed, 761 insertions(+), 263 deletions(-) diff --git a/Termind/Application/Agent/AgentBuiltInTools.swift b/Termind/Application/Agent/AgentBuiltInTools.swift index dd32150d..d3d4e9ab 100644 --- a/Termind/Application/Agent/AgentBuiltInTools.swift +++ b/Termind/Application/Agent/AgentBuiltInTools.swift @@ -715,77 +715,115 @@ nonisolated enum AgentBuiltInTools { AnyAgentTool(descriptor: descriptor( "request_user_input", """ - Ask the user to decide something you cannot decide yourself, and wait for the answer. \ + Ask the user to decide things you cannot decide yourself, and wait for the answers. \ Use it only when the answer is genuinely theirs: a preference, a tradeoff with no \ defensible default, or a fact only they hold — which host is production, where a \ credential lives. If the repository, the request, or an obvious convention already \ answers it, choose, say which assumption you made, and keep going; a question the \ user has to stop and think about costs more than a decision you make and flag. Never \ - use it to ask whether to proceed. Give 2-4 options, each with a description saying \ - what it costs rather than restating the label, and put your recommendation on the \ - option itself instead of in its wording. A free-form field is offered alongside them \ - unless you set allow_custom false, so never write an "Other" option yourself. Omit \ - options entirely for a question no menu can answer, such as a URL or a name. + use it to ask whether to proceed. Ask a whole group of related decisions in one \ + call, up to three, rather than one at a time — a second question raised only after \ + the first is answered was hidden at the moment it would have informed it. One \ + question is still the common case; three is a ceiling, not a target. Give each \ + question 2-4 options with a description saying what it costs rather than restating \ + the label, and put your recommendation on the option itself instead of in its \ + wording. A free-form field is offered alongside them unless you set allow_custom \ + false, so never write an "Other" option yourself. Omit options entirely for a \ + question no menu can answer, such as a URL or a name. A question may come back with \ + a null answer: that is the user declining to decide that one, not an error and not \ + a reason to ask again — decide it yourself and say which assumption you used. """, properties: [ - "prompt": string(max: 4_096), - "header": string(max: 24), - "options": array( + "questions": array( items: object( properties: [ "id": string(max: 64), - "label": string(max: 120), - "description": string(max: 400), - "recommended": boolean(), + "header": string(max: 24), + "prompt": string(max: 4_096), + "options": array( + items: object( + properties: [ + "id": string(max: 64), + "label": string(max: 120), + "description": string(max: 400), + "recommended": boolean(), + ], + required: ["id", "label"] + ), + max: 4 + ), + "multi_select": boolean(), + "allow_custom": boolean(), + "custom_placeholder": string(max: 120), ], - required: ["id", "label"] + required: ["id", "prompt"] ), - max: 4 - ), - "multi_select": boolean(), - "allow_custom": boolean(), - "custom_placeholder": string(max: 120), - ], required: ["prompt"], target: .user, safety: .locallyReadOnly + max: AgentUserInputLimits.maxQuestions + ) + ], required: ["questions"], target: .user, safety: .locallyReadOnly )) { invocation, context in - let arguments = try Arguments(invocation) - let options = try arguments.userInputOptions() - let request = AgentUserInputRequest( - prompt: try arguments.string("prompt"), - kind: options.isEmpty - ? .text - : .choice( - AgentUserInputChoice( - options: options, - allowsCustomAnswer: arguments.optionalBool("allow_custom") ?? true, - allowsMultipleAnswers: arguments.optionalBool("multi_select") ?? false, - customPlaceholder: arguments.optionalString("custom_placeholder") ?? "" - ) - ), - header: arguments.optionalString("header") ?? "" + let questions = try Arguments(invocation).userQuestions() + let response = await context.userInteraction.request( + AgentUserInputRequest(questions: questions) ) - switch await context.userInteraction.request(request) { - case .text(let answer): - return result(invocation, .object(["answer": .string(answer)])) + guard case .answered(let answers) = response else { throw AgentToolError.cancelled } + return result(invocation, .object([ + "answers": .array(reported(questions, answers)) + ])) + } + } + + /// Every question, answered or not, in the order it was asked. + /// + /// Reporting the skipped ones rather than omitting them is what makes a + /// null legible: a model reading its own list back finds each entry where + /// it put it, and an absent key would read as a bug rather than as the user + /// declining that one. + /// + /// Not private, because this shape *is* the contract with the model and a + /// test should be able to hold it still. + static func reported( + _ questions: [AgentUserQuestion], _ answers: [AgentUserInputAnswer] + ) -> [AgentJSONValue] { + let byID = Dictionary(answers.map { ($0.questionID, $0.value) }) { first, _ in first } + return questions.map { question in + switch byID[question.id] { case .choice(let selectedIDs, let custom): let labels = selectedIDs.compactMap { id in - options.first { $0.id == id }?.label + question.options.first { $0.id == id }?.label } - return result(invocation, .object([ + return row( + question.id, // The canonical human answer, whichever route produced it, - // so a model that reads only one field still reads the - // right one. - "answer": .string(custom ?? labels.joined(separator: ", ")), - "selected_option_ids": .array(selectedIDs.map(AgentJSONValue.string)), - "custom_text": custom.map(AgentJSONValue.string) ?? .null, - ])) + // so a model that reads one field reads the right one. + answer: custom ?? labels.joined(separator: ", "), + selected: selectedIDs, custom: custom + ) + case .text(let typed): + return row(question.id, answer: typed, selected: [], custom: typed) case .secret: - throw AgentToolError.invalidArguments("Secret input must use request_user_secret.") - case .cancelled: - throw AgentToolError.cancelled + // Unreachable — `request_user_input` never builds a secret + // question. Reported as skipped rather than echoed, so that if + // one ever does arrive here the plaintext still does not reach + // the model. + return row(question.id, answer: nil, selected: [], custom: nil) + case nil: + return row(question.id, answer: nil, selected: [], custom: nil) } } } + private static func row( + _ questionID: String, answer: String?, selected: [String], custom: String? + ) -> AgentJSONValue { + .object([ + "question_id": .string(questionID), + "answer": answer.map(AgentJSONValue.string) ?? .null, + "selected_option_ids": .array(selected.map(AgentJSONValue.string)), + "custom_text": custom.map(AgentJSONValue.string) ?? .null, + ]) + } + private static func requestUserSecret() -> AnyAgentTool { AnyAgentTool(descriptor: descriptor( "request_user_secret", "Collect a secret in SecureField and return a one-shot bound handle.", @@ -803,12 +841,27 @@ nonisolated enum AgentBuiltInTools { let hostID = try arguments.uuid("host_id") let toolName = try arguments.string("consumer_tool") let purpose = try arguments.string("purpose") + // One question, always. A secret is not a decision among options and + // has no business sharing a form with one. let request = AgentUserInputRequest( - prompt: try arguments.string("prompt"), kind: .secret, + questions: [ + AgentUserQuestion( + id: "secret", prompt: try arguments.string("prompt"), kind: .secret + ) + ], purpose: purpose, hostID: hostID ) switch await context.userInteraction.request(request) { - case .secret(let secret): + case .answered(let answers): + guard case .secret(let secret)? = answers.first?.value else { + // A secret must come back through `SecureField` and the + // broker. Anything else means it took the ordinary-answer + // path, where it would have been echoed on screen and kept + // in plain state. + throw AgentToolError.invalidArguments( + "The secret prompt returned ordinary text." + ) + } let handle = await context.secretBroker.issue( secret, binding: SecretBinding( @@ -818,11 +871,6 @@ nonisolated enum AgentBuiltInTools { return result(invocation, .object([ "secret_handle": .string(handle.id.uuidString), "single_use": .bool(true), ])) - case .text, .choice: - // A secret must come back through `SecureField` and the broker. - // Anything else means it took the ordinary-answer path, where it - // would have been echoed on screen and kept in plain state. - throw AgentToolError.invalidArguments("The secret prompt returned ordinary text.") case .cancelled: throw AgentToolError.cancelled } } @@ -1022,13 +1070,59 @@ nonisolated struct Arguments: Sendable { object[key]?.arrayValue?.compactMap(\.stringValue) ?? [] } - /// The `options` array of `request_user_input`. + /// The `questions` array of `request_user_input`. /// - /// Malformed rows are rejected rather than dropped. A menu silently missing - /// the option the user wanted is a worse failure than an error the model - /// can see and retry. - func userInputOptions() throws -> [AgentUserInputOption] { - guard let value = object["options"], value != .null else { return [] } + /// Everything malformed is rejected rather than dropped. A form silently + /// missing the question — or the option — the user needed is a worse + /// failure than an error the model can see and retry. + func userQuestions() throws -> [AgentUserQuestion] { + guard let rows = object["questions"]?.arrayValue else { + throw AgentToolError.invalidArguments("questions must be an array.") + } + guard !rows.isEmpty else { + throw AgentToolError.invalidArguments("questions must contain at least one question.") + } + guard rows.count <= AgentUserInputLimits.maxQuestions else { + throw AgentToolError.invalidArguments( + "At most \(AgentUserInputLimits.maxQuestions) questions may be asked at once." + ) + } + + var seen: Set = [] + return try rows.map { row in + guard let fields = row.objectValue, + let id = fields["id"]?.stringValue, !id.isEmpty, + let prompt = fields["prompt"]?.stringValue, !prompt.isEmpty + else { + throw AgentToolError.invalidArguments("Each question requires id and prompt.") + } + guard seen.insert(id).inserted else { + throw AgentToolError.invalidArguments("Question ids must be unique: \(id).") + } + + let options = try Self.options(in: fields) + return AgentUserQuestion( + id: id, + header: fields["header"]?.stringValue ?? "", + prompt: prompt, + kind: options.isEmpty + ? .text + : .choice( + AgentUserInputChoice( + options: options, + allowsCustomAnswer: fields["allow_custom"]?.boolValue ?? true, + allowsMultipleAnswers: fields["multi_select"]?.boolValue ?? false, + customPlaceholder: fields["custom_placeholder"]?.stringValue ?? "" + ) + ) + ) + } + } + + private static func options(in question: [String: AgentJSONValue]) throws + -> [AgentUserInputOption] + { + guard let value = question["options"], value != .null else { return [] } guard let rows = value.arrayValue else { throw AgentToolError.invalidArguments("options must be an array.") } diff --git a/Termind/Application/Agent/AgentSecurity.swift b/Termind/Application/Agent/AgentSecurity.swift index 0fb2011b..a6ad73df 100644 --- a/Termind/Application/Agent/AgentSecurity.swift +++ b/Termind/Application/Agent/AgentSecurity.swift @@ -201,30 +201,83 @@ nonisolated struct AgentUserInputChoice: Hashable, Sendable { var customPlaceholder: String = "" } -nonisolated struct AgentUserInputRequest: Identifiable, Hashable, Sendable { +nonisolated enum AgentUserInputLimits { + /// Codex's ceiling, and for the same reason: past three, a form stops being + /// a decision and starts being a questionnaire. + static let maxQuestions = 3 +} + +/// One thing the agent needs decided. +nonisolated struct AgentUserQuestion: Identifiable, Hashable, Sendable { nonisolated enum Kind: Hashable, Sendable { case text case choice(AgentUserInputChoice) case secret } - var id: UUID = UUID() - var prompt: String - var kind: Kind + /// Named by the model, unique within its request. It wrote the questions + /// and it reads the answers back by these names, so they are its own + /// vocabulary rather than an index we assign. + var id: String /// Short label naming the decision, shown above the prompt. var header: String = "" + var prompt: String + var kind: Kind + + /// The choices this question offers, or none for a free-form one. Saves + /// every reader of a question from re-matching on `kind` to find out. + var options: [AgentUserInputOption] { + guard case .choice(let choice) = kind else { return [] } + return choice.options + } +} + +/// A set of questions put to the user at once. +/// +/// Several rather than one because a group of decisions is usually a group — +/// asking about storage, then about migrations only once storage is answered, +/// hides the second question at the moment it would have informed the first. +/// The tool's calls are `.sequential`, so asking twice in a turn already meant +/// answering twice in a row; this makes it one form instead. +nonisolated struct AgentUserInputRequest: Identifiable, Hashable, Sendable { + var id: UUID = UUID() + var questions: [AgentUserQuestion] var purpose: String = "" var hostID: UUID? } +/// What has been filled into one question so far. +nonisolated struct AgentUserInputDraft: Hashable, Sendable { + /// The "something else" field, and the whole answer for a free-form + /// question. + var text = "" + var selection: Set = [] + + var trimmedText: String { text.trimmingCharacters(in: .whitespacesAndNewlines) } +} + +nonisolated struct AgentUserInputAnswer: Hashable, Sendable { + nonisolated enum Value: Hashable, Sendable { + case text(String) + /// Selected option ids in the order the question listed them, plus + /// whatever the user typed instead of — or, when several answers are + /// allowed, alongside — them. Kept structured rather than flattened to + /// a string so the tool can hand the model ids it chose itself. + case choice(selectedIDs: [String], custom: String?) + case secret(String) + } + + var questionID: String + var value: Value +} + nonisolated enum AgentUserInputResponse: Hashable, Sendable { - case text(String) - /// Selected option ids in the order the request listed them, plus whatever - /// the user typed instead of — or, when several answers are allowed, - /// alongside — them. Kept structured rather than flattened to a string so - /// the tool can hand the model ids it chose itself. - case choice(selectedIDs: [String], custom: String?) - case secret(String) + /// Only the questions the user actually answered. + /// + /// An absent question was skipped, and skipping is itself an answer — the + /// user declining to decide that one. So there is no `.skipped` case: the + /// absence carries it, and the tool reports it to the model as a null. + case answered([AgentUserInputAnswer]) case cancelled } diff --git a/Termind/Design/DesignTokens.swift b/Termind/Design/DesignTokens.swift index 7c57ee3f..8dab12da 100644 --- a/Termind/Design/DesignTokens.swift +++ b/Termind/Design/DesignTokens.swift @@ -176,6 +176,13 @@ nonisolated enum Metrics { /// draft instead; these two bound the result. static let composerMinHeight: CGFloat = 40 static let composerMaxHeight: CGFloat = 132 + + /// How tall the agent's question form may grow before it scrolls. + /// + /// Three questions with four described options each runs past 500pt, + /// and this card sits between the transcript and the composer — past + /// this it would push the conversation it is about off the screen. + static let userInputMaxHeight: CGFloat = 320 } enum TransferCenter { diff --git a/Termind/Features/AI/ConversationModel.swift b/Termind/Features/AI/ConversationModel.swift index c0f756f1..be844d43 100644 --- a/Termind/Features/AI/ConversationModel.swift +++ b/Termind/Features/AI/ConversationModel.swift @@ -218,10 +218,12 @@ final class ConversationModel: Identifiable { var pendingApproval: AgentApprovalRequest? { pendingApprovals.first } var pendingUserInput: AgentUserInputRequest? { pendingUserInputs.first } - /// The free-form answer, for a text question and for the "something else" - /// field beside a set of options. - var userInputDraft = "" - var userInputSelection: Set = [] + /// What has been filled in so far, keyed by question. One request can now + /// carry several questions, so the drafts are per question rather than one + /// pair of fields shared by whichever one happened to be on screen. + private(set) var userInputDrafts: [String: AgentUserInputDraft] = [:] + /// Kept out of `userInputDrafts` on purpose: a secret does not belong in + /// the same dictionary as text that is echoed back to the model. var secretInputDraft = "" /// The model-facing history, which is not what `messages` holds. /// @@ -477,64 +479,94 @@ final class ConversationModel: Identifiable { refreshRunState() } - /// Nothing is preselected, so this is what stops Continue from returning an - /// answer the user never gave. - var canSubmitUserInput: Bool { - guard let request = pendingUserInput else { return false } - switch request.kind { + func userInputDraft(for questionID: String) -> AgentUserInputDraft { + userInputDrafts[questionID] ?? AgentUserInputDraft() + } + + /// Whether this one question has been answered. Skipping is allowed, so + /// this drives a count rather than a validation error. + func isUserInputAnswered(_ question: AgentUserQuestion) -> Bool { + switch question.kind { case .secret: return !secretInputDraft.isEmpty - case .text: return !trimmedUserInputDraft.isEmpty - case .choice: return !userInputSelection.isEmpty || !trimmedUserInputDraft.isEmpty + default: + let draft = userInputDraft(for: question.id) + return !draft.selection.isEmpty || !draft.trimmedText.isEmpty } } - func toggleUserInputOption(_ id: String) { - guard case .choice(let choice)? = pendingUserInput?.kind else { return } + var answeredUserInputCount: Int { + pendingUserInput?.questions.filter(isUserInputAnswered).count ?? 0 + } + + /// Nothing is preselected, so this is what stops Continue from returning an + /// answer the user never gave. + /// + /// One answer is enough — a question may be left alone, and the tool + /// reports that as a null the model has to decide for itself. Zero is not + /// enough: answering nothing is Cancel, and dressing it as success would + /// hand the model a result that says only that it was not refused. + var canSubmitUserInput: Bool { answeredUserInputCount > 0 } + + func toggleUserInputOption(_ optionID: String, in questionID: String) { + guard case .choice(let choice)? = question(questionID)?.kind else { return } + var draft = userInputDraft(for: questionID) if choice.allowsMultipleAnswers { - if userInputSelection.contains(id) { - userInputSelection.remove(id) + if draft.selection.contains(optionID) { + draft.selection.remove(optionID) } else { - userInputSelection.insert(id) + draft.selection.insert(optionID) } } else { - userInputSelection = userInputSelection.contains(id) ? [] : [id] + draft.selection = draft.selection.contains(optionID) ? [] : [optionID] // One answer means one answer: a half-typed "something else" left // sitting under a chosen radio would be a second, contradicting one. - userInputDraft = "" + draft.text = "" } + userInputDrafts[questionID] = draft } /// The other half of that rule — typing clears the radio it would otherwise /// silently disagree with. Left alone in a multi-answer question, where the /// two are additive rather than exclusive. - func setUserInputCustomAnswer(_ text: String) { - userInputDraft = text - guard case .choice(let choice)? = pendingUserInput?.kind, + func setUserInputCustomAnswer(_ text: String, for questionID: String) { + var draft = userInputDraft(for: questionID) + draft.text = text + if case .choice(let choice)? = question(questionID)?.kind, !choice.allowsMultipleAnswers, !text.isEmpty - else { return } - userInputSelection.removeAll() + { + draft.selection.removeAll() + } + userInputDrafts[questionID] = draft } func submitUserInput() { guard let request = pendingUserInput, canSubmitUserInput else { return } - let response: AgentUserInputResponse - switch request.kind { - case .secret: - response = .secret(secretInputDraft) - case .text: - response = .text(userInputDraft) - case .choice(let choice): - let custom = trimmedUserInputDraft - response = .choice( - // Ordered by the request, not by whatever order the user - // clicked in — the model wrote the list and reads the answer - // against it. - selectedIDs: choice.options.map(\.id).filter(userInputSelection.contains), - custom: custom.isEmpty ? nil : custom - ) + let answers = request.questions.compactMap { question -> AgentUserInputAnswer? in + guard isUserInputAnswered(question) else { return nil } + let draft = userInputDraft(for: question.id) + switch question.kind { + case .secret: + return AgentUserInputAnswer( + questionID: question.id, value: .secret(secretInputDraft) + ) + case .text: + return AgentUserInputAnswer(questionID: question.id, value: .text(draft.text)) + case .choice(let choice): + let custom = draft.trimmedText + return AgentUserInputAnswer( + questionID: question.id, + // Ordered by the question, not by whatever order the user + // clicked in — the model wrote the list and reads the + // answer against it. + value: .choice( + selectedIDs: choice.options.map(\.id).filter(draft.selection.contains), + custom: custom.isEmpty ? nil : custom + ) + ) + } } advanceUserInput() - inputWaiters.removeValue(forKey: request.id)?.resume(returning: response) + inputWaiters.removeValue(forKey: request.id)?.resume(returning: .answered(answers)) } func cancelUserInput() { @@ -543,8 +575,8 @@ final class ConversationModel: Identifiable { inputWaiters.removeValue(forKey: request.id)?.resume(returning: .cancelled) } - private var trimmedUserInputDraft: String { - userInputDraft.trimmingCharacters(in: .whitespacesAndNewlines) + private func question(_ id: String) -> AgentUserQuestion? { + pendingUserInput?.questions.first { $0.id == id } } /// Retires the answered question and hands a clean sheet to the next one. @@ -556,8 +588,7 @@ final class ConversationModel: Identifiable { private func clearUserInputDrafts() { secretInputDraft = "" - userInputDraft = "" - userInputSelection = [] + userInputDrafts = [:] } /// What the transcript shows, from what is actually outstanding. diff --git a/Termind/Features/AI/ConversationTranscriptView.swift b/Termind/Features/AI/ConversationTranscriptView.swift index 7afb98b6..13f7a6bb 100644 --- a/Termind/Features/AI/ConversationTranscriptView.swift +++ b/Termind/Features/AI/ConversationTranscriptView.swift @@ -258,71 +258,121 @@ private struct QueueDepthLabel: View { } } -/// The agent asking the user to decide something. +/// The agent asking the user to decide one or more things. +/// +/// A form rather than a run of cards. A turn's questions are usually one group +/// of decisions, and revealing the second only once the first is answered hides +/// it at the moment it would have informed the first. /// /// Options are a list rather than a `Picker` because each one carries a /// sentence explaining what it costs, and a pop-up menu has nowhere to put /// that. Nothing is preselected — including the recommendation, which is a /// label and not a default, so that an answer always takes a deliberate click. +/// A question may also be left alone: the tool reports that as a null and the +/// model decides it itself, so the count in the footer is a count and not a +/// validation error, and an unanswered question is marked in no way at all. private struct AgentUserInputCard: View { let request: AgentUserInputRequest let conversation: ConversationModel + private var isForm: Bool { request.questions.count > 1 } + var body: some View { - @Bindable var conversation = conversation - VStack(alignment: .leading, spacing: Metrics.Space.s) { - HStack { - if !request.header.isEmpty { - Text(request.header) - .font(.caption.weight(.semibold)) - .foregroundStyle(.secondary) + VStack(alignment: .leading, spacing: Metrics.Space.m) { + if conversation.pendingUserInputs.count > 1 { + HStack { + Spacer() + QueueDepthLabel(remaining: conversation.pendingUserInputs.count) } - Spacer() - QueueDepthLabel(remaining: conversation.pendingUserInputs.count) - } - Text(request.prompt) - .font(.callout.weight(.medium)) - .fixedSize(horizontal: false, vertical: true) - - switch request.kind { - case .secret: - SecureField("Secret", text: $conversation.secretInputDraft) - .textFieldStyle(.roundedBorder) - case .choice(let choice): - choiceList(choice) - case .text: - TextField("Answer", text: $conversation.userInputDraft) - .textFieldStyle(.roundedBorder) } - HStack { - Button("Cancel") { conversation.cancelUserInput() } - Spacer() - Button("Continue") { conversation.submitUserInput() } - .buttonStyle(.borderedProminent) - .disabled(!conversation.canSubmitUserInput) + // Three questions with four described options each runs past 500pt, + // and this card sits between the transcript and the composer. But a + // bare `ScrollView` is greedy: capped at the same height it would + // hand a one-question card 320pt of box for 120pt of question. So + // the natural stack is preferred and the scrolling one is the + // fallback, which is what `ViewThatFits` is for. + ViewThatFits(in: .vertical) { + questionStack + ScrollView { questionStack } + .scrollBounceBehavior(.basedOnSize) } + .frame(maxHeight: Metrics.Chat.userInputMaxHeight) + + footer } .padding(Metrics.Space.l) } - private func choiceList(_ choice: AgentUserInputChoice) -> some View { - VStack(alignment: .leading, spacing: Metrics.Space.xs) { - ForEach(choice.options) { option in - optionRow(option, choice: choice) + private var questionStack: some View { + VStack(alignment: .leading, spacing: Metrics.Space.l) { + ForEach(request.questions) { question in + questionBlock(question) } - if choice.allowsCustomAnswer { - customRow(choice) + } + .frame(maxWidth: .infinity, alignment: .leading) + } + + private var footer: some View { + HStack(spacing: Metrics.Space.m) { + Button("Cancel") { conversation.cancelUserInput() } + Spacer(minLength: 0) + if isForm { + Text( + "\(conversation.answeredUserInputCount) of \(request.questions.count) answered" + ) + .font(.caption.monospacedDigit()) + .foregroundStyle(.secondary) + } + Button("Continue") { conversation.submitUserInput() } + .buttonStyle(.borderedProminent) + .disabled(!conversation.canSubmitUserInput) + } + } + + @ViewBuilder + private func questionBlock(_ question: AgentUserQuestion) -> some View { + @Bindable var conversation = conversation + VStack(alignment: .leading, spacing: Metrics.Space.s) { + if !question.header.isEmpty { + Text(question.header) + .font(.caption.weight(.semibold)) + .foregroundStyle(.secondary) + } + Text(question.prompt) + .font(.callout.weight(.medium)) + .fixedSize(horizontal: false, vertical: true) + + switch question.kind { + case .secret: + SecureField("Secret", text: $conversation.secretInputDraft) + .textFieldStyle(.roundedBorder) + case .choice(let choice): + choiceList(choice, in: question.id) + case .text: + TextField("Answer", text: customBinding(for: question.id)) + .textFieldStyle(.roundedBorder) } } } - private func optionRow(_ option: AgentUserInputOption, choice: AgentUserInputChoice) - -> some View - { - let isSelected = conversation.userInputSelection.contains(option.id) + private func choiceList(_ choice: AgentUserInputChoice, in questionID: String) -> some View { + VStack(alignment: .leading, spacing: Metrics.Space.xs) { + ForEach(choice.options) { option in + optionRow(option, choice: choice, in: questionID) + } + if choice.allowsCustomAnswer { + customRow(choice, in: questionID) + } + } + } + + private func optionRow( + _ option: AgentUserInputOption, choice: AgentUserInputChoice, in questionID: String + ) -> some View { + let isSelected = conversation.userInputDraft(for: questionID).selection.contains(option.id) return Button { - conversation.toggleUserInputOption(option.id) + conversation.toggleUserInputOption(option.id, in: questionID) } label: { HStack(alignment: .firstTextBaseline, spacing: Metrics.Space.s) { Image(systemName: marker(isSelected: isSelected, choice: choice)) @@ -358,31 +408,29 @@ private struct AgentUserInputCard: View { /// The escape hatch, sitting in the list as one more choice rather than /// below it as an afterthought — an answer the model did not think of is /// not a lesser kind of answer. - private func customRow(_ choice: AgentUserInputChoice) -> some View { - HStack(alignment: .firstTextBaseline, spacing: Metrics.Space.s) { - Image( - systemName: marker( - isSelected: !conversation.userInputDraft.isEmpty, choice: choice - ) - ) - .foregroundStyle( - conversation.userInputDraft.isEmpty - ? AnyShapeStyle(.secondary) : AnyShapeStyle(.tint) - ) - .frame(width: Metrics.Sidebar.rowIcon) + private func customRow(_ choice: AgentUserInputChoice, in questionID: String) -> some View { + let typed = conversation.userInputDraft(for: questionID).text + return HStack(alignment: .firstTextBaseline, spacing: Metrics.Space.s) { + Image(systemName: marker(isSelected: !typed.isEmpty, choice: choice)) + .foregroundStyle(typed.isEmpty ? AnyShapeStyle(.secondary) : AnyShapeStyle(.tint)) + .frame(width: Metrics.Sidebar.rowIcon) TextField( choice.customPlaceholder.isEmpty ? String(localized: "Something else…") : choice.customPlaceholder, - text: Binding( - get: { conversation.userInputDraft }, - set: { conversation.setUserInputCustomAnswer($0) } - ) + text: customBinding(for: questionID) ) .textFieldStyle(.roundedBorder) } } + private func customBinding(for questionID: String) -> Binding { + Binding( + get: { conversation.userInputDraft(for: questionID).text }, + set: { conversation.setUserInputCustomAnswer($0, for: questionID) } + ) + } + private func marker(isSelected: Bool, choice: AgentUserInputChoice) -> String { if choice.allowsMultipleAnswers { isSelected ? "checkmark.square.fill" : "square" diff --git a/TermindTests/AgentApprovalQueueTests.swift b/TermindTests/AgentApprovalQueueTests.swift index 33882e6f..b9c325b6 100644 --- a/TermindTests/AgentApprovalQueueTests.swift +++ b/TermindTests/AgentApprovalQueueTests.swift @@ -176,7 +176,11 @@ struct AgentApprovalQueueTests { conversation.handle(.reviewStarted(request)) #expect(conversation.runState == .reviewing) - conversation.present(AgentUserInputRequest(prompt: "which host?", kind: .text)) + conversation.present( + AgentUserInputRequest( + questions: [AgentUserQuestion(id: "host", prompt: "which host?", kind: .text)] + ) + ) #expect(conversation.runState == .waitingForUser) // The review finishing must not announce that nothing is outstanding. diff --git a/TermindTests/AgentUserInputTests.swift b/TermindTests/AgentUserInputTests.swift index b8e07e91..bf247978 100644 --- a/TermindTests/AgentUserInputTests.swift +++ b/TermindTests/AgentUserInputTests.swift @@ -7,79 +7,121 @@ import Testing @Suite struct AgentUserInputTests { - // MARK: - Parsing the tool's options + // MARK: - Parsing the tool's questions @Test - func optionsCarryTheirDescriptionAndRecommendation() throws { - let options = try arguments([ - "prompt": .string("Which database?"), - "options": .array([ + func questionsCarryTheirOptionsDescriptionsAndRecommendations() throws { + let questions = try arguments([ + "questions": .array([ .object([ - "id": .string("postgres"), "label": .string("Postgres"), - "description": .string("JSONB, and it matches the existing schema."), - "recommended": .bool(true), + "id": .string("storage"), + "header": .string("Storage"), + "prompt": .string("Which database?"), + "options": .array([ + .object([ + "id": .string("postgres"), "label": .string("Postgres"), + "description": .string("JSONB, and it matches the existing schema."), + "recommended": .bool(true), + ]), + .object(["id": .string("sqlite"), "label": .string("SQLite")]), + ]), ]), - .object(["id": .string("sqlite"), "label": .string("SQLite")]), - ]), - ]).userInputOptions() + .object(["id": .string("base_url"), "prompt": .string("Which URL?")]), + ]) + ]).userQuestions() - #expect(options.count == 2) - #expect(options[0].id == "postgres") - #expect(options[0].detail == "JSONB, and it matches the existing schema.") - #expect(options[0].isRecommended) + #expect(questions.count == 2) + #expect(questions[0].header == "Storage") + #expect(questions[0].options.count == 2) + #expect(questions[0].options[0].detail == "JSONB, and it matches the existing schema.") + #expect(questions[0].options[0].isRecommended) // Absent is absent, not false-y noise. - #expect(options[1].detail.isEmpty) - #expect(!options[1].isRecommended) - } + #expect(questions[0].options[1].detail.isEmpty) + #expect(!questions[0].options[1].isRecommended) - @Test - func noOptionsMeansAFreeFormQuestion() throws { - #expect(try arguments(["prompt": .string("Which URL?")]).userInputOptions().isEmpty) - #expect( - try arguments(["prompt": .string("Which URL?"), "options": .null]) - .userInputOptions().isEmpty - ) - } - - /// A malformed row is rejected rather than dropped: a menu quietly missing - /// the option the user wanted is worse than an error the model can retry. - @Test - func malformedOptionsAreRejectedRatherThanDropped() { - #expect(throws: AgentToolError.self) { - try arguments([ - "prompt": .string("?"), - "options": .array([.object(["label": .string("No id here")])]), - ]).userInputOptions() + // No options is a free-form question, not an empty menu. + guard case .text = questions[1].kind else { + Issue.record("expected a free-form question, got \(questions[1].kind)") + return } - #expect(throws: AgentToolError.self) { - try arguments([ - "prompt": .string("?"), + } + + @Test + func choiceFlagsDefaultToOfferingAWayOut() throws { + let questions = try arguments([ + "questions": .array([ + .object([ + "id": .string("q"), "prompt": .string("?"), + "options": .array([.object(["id": .string("a"), "label": .string("A")])]), + ]) + ]) + ]).userQuestions() + + guard case .choice(let choice) = questions[0].kind else { + Issue.record("expected a choice question") + return + } + #expect(choice.allowsCustomAnswer, "a way out has to be the default") + #expect(!choice.allowsMultipleAnswers) + } + + /// Everything malformed is rejected rather than dropped: a form silently + /// missing the question the user needed is worse than an error the model + /// can retry. + @Test + func malformedQuestionsAreRejectedRatherThanDropped() { + let bad: [[String: AgentJSONValue]] = [ + [:], + ["questions": .array([])], + ["questions": .object([:])], + ["questions": .array([.object(["prompt": .string("no id")])])], + ["questions": .array([.object(["id": .string("q")])])], + ["questions": .array([ + .object(["id": .string("q"), "prompt": .string("first")]), + .object(["id": .string("q"), "prompt": .string("duplicate id")]), + ])], + ["questions": .array([ + .object([ + "id": .string("q"), "prompt": .string("?"), + "options": .array([ + .object(["id": .string("a"), "label": .string("A")]), + .object(["id": .string("a"), "label": .string("Also A")]), + ]), + ]) + ])], + ["questions": .array([.object([ + "id": .string("q"), "prompt": .string("?"), "options": .array([.string("a bare string")]), - ]).userInputOptions() + ])])], + ] + for object in bad { + #expect(throws: AgentToolError.self) { try arguments(object).userQuestions() } } } @Test - func duplicateOptionIdsAreRejected() { + func askingMoreThanThreeQuestionsAtOnceIsRefused() { + let four = (1...4).map { index in + AgentJSONValue.object([ + "id": .string("q\(index)"), "prompt": .string("Question \(index)?"), + ]) + } #expect(throws: AgentToolError.self) { - try arguments([ - "prompt": .string("?"), - "options": .array([ - .object(["id": .string("a"), "label": .string("First")]), - .object(["id": .string("a"), "label": .string("Second")]), - ]), - ]).userInputOptions() + try arguments(["questions": .array(four)]).userQuestions() + } + #expect(throws: Never.self) { + try arguments(["questions": .array(Array(four.prefix(3)))]).userQuestions() } } - // MARK: - One answer, or several + // MARK: - One question @Test func nothingIsPreselectedEvenWhenAnOptionIsRecommended() { let conversation = ConversationModel() conversation.present(request(multiple: false)) - #expect(conversation.userInputSelection.isEmpty) + #expect(conversation.userInputDraft(for: "storage").selection.isEmpty) // A recommendation is a label, not a default — Continue stays off until // the user actually answers. #expect(!conversation.canSubmitUserInput) @@ -90,12 +132,12 @@ struct AgentUserInputTests { let conversation = ConversationModel() conversation.present(request(multiple: false)) - conversation.setUserInputCustomAnswer("MySQL") + conversation.setUserInputCustomAnswer("MySQL", for: "storage") #expect(conversation.canSubmitUserInput) - conversation.toggleUserInputOption("postgres") - #expect(conversation.userInputSelection == ["postgres"]) - #expect(conversation.userInputDraft.isEmpty) + conversation.toggleUserInputOption("postgres", in: "storage") + #expect(conversation.userInputDraft(for: "storage").selection == ["postgres"]) + #expect(conversation.userInputDraft(for: "storage").text.isEmpty) } @Test @@ -103,11 +145,11 @@ struct AgentUserInputTests { let conversation = ConversationModel() conversation.present(request(multiple: false)) - conversation.toggleUserInputOption("postgres") - conversation.setUserInputCustomAnswer("MySQL") + conversation.toggleUserInputOption("postgres", in: "storage") + conversation.setUserInputCustomAnswer("MySQL", for: "storage") - #expect(conversation.userInputSelection.isEmpty) - #expect(conversation.userInputDraft == "MySQL") + #expect(conversation.userInputDraft(for: "storage").selection.isEmpty) + #expect(conversation.userInputDraft(for: "storage").text == "MySQL") } @Test @@ -115,13 +157,13 @@ struct AgentUserInputTests { let conversation = ConversationModel() conversation.present(request(multiple: false)) - conversation.toggleUserInputOption("postgres") - conversation.toggleUserInputOption("sqlite") - #expect(conversation.userInputSelection == ["sqlite"]) + conversation.toggleUserInputOption("postgres", in: "storage") + conversation.toggleUserInputOption("sqlite", in: "storage") + #expect(conversation.userInputDraft(for: "storage").selection == ["sqlite"]) // Clicking the chosen one again takes the answer back. - conversation.toggleUserInputOption("sqlite") - #expect(conversation.userInputSelection.isEmpty) + conversation.toggleUserInputOption("sqlite", in: "storage") + #expect(conversation.userInputDraft(for: "storage").selection.isEmpty) } /// When several answers are allowed the two routes are additive, not @@ -131,30 +173,111 @@ struct AgentUserInputTests { let conversation = ConversationModel() conversation.present(request(multiple: true)) - conversation.toggleUserInputOption("postgres") - conversation.toggleUserInputOption("sqlite") - conversation.setUserInputCustomAnswer("and DuckDB") + conversation.toggleUserInputOption("postgres", in: "storage") + conversation.toggleUserInputOption("sqlite", in: "storage") + conversation.setUserInputCustomAnswer("and DuckDB", for: "storage") - #expect(conversation.userInputSelection == ["postgres", "sqlite"]) - #expect(conversation.userInputDraft == "and DuckDB") + #expect(conversation.userInputDraft(for: "storage").selection == ["postgres", "sqlite"]) + #expect(conversation.userInputDraft(for: "storage").text == "and DuckDB") + } + + // MARK: - Several questions at once + + @Test + func eachQuestionKeepsItsOwnAnswer() { + let conversation = ConversationModel() + conversation.present(threeQuestions()) + + conversation.toggleUserInputOption("postgres", in: "storage") + conversation.toggleUserInputOption("flyway", in: "migrations") + conversation.setUserInputCustomAnswer("staging-2", for: "host") + + #expect(conversation.userInputDraft(for: "storage").selection == ["postgres"]) + #expect(conversation.userInputDraft(for: "migrations").selection == ["flyway"]) + #expect(conversation.userInputDraft(for: "storage").text.isEmpty) + #expect(conversation.userInputDraft(for: "host").text == "staging-2") + #expect(conversation.answeredUserInputCount == 3) + } + + /// Single- and multi-answer questions sit in the same form and keep their + /// own rule; the mutual exclusion of one must not reach into the other. + @Test + func exclusionRulesApplyPerQuestionNotPerForm() { + let conversation = ConversationModel() + conversation.present(threeQuestions()) + + conversation.toggleUserInputOption("flyway", in: "migrations") + conversation.toggleUserInputOption("liquibase", in: "migrations") + #expect( + conversation.userInputDraft(for: "migrations").selection == ["flyway", "liquibase"], + "migrations allows several" + ) + + conversation.toggleUserInputOption("postgres", in: "storage") + conversation.toggleUserInputOption("sqlite", in: "storage") + #expect( + conversation.userInputDraft(for: "storage").selection == ["sqlite"], + "storage allows one" + ) + #expect( + conversation.userInputDraft(for: "migrations").selection == ["flyway", "liquibase"], + "and choosing in one question must not disturb another" + ) + } + + @Test + func oneAnswerIsEnoughToContinueButNoneIsNot() { + let conversation = ConversationModel() + conversation.present(threeQuestions()) + + #expect(!conversation.canSubmitUserInput, "answering nothing is Cancel, not Continue") + conversation.toggleUserInputOption("postgres", in: "storage") + #expect(conversation.canSubmitUserInput) + #expect(conversation.answeredUserInputCount == 1) } // MARK: - What the tool gets back @Test - func answerReportsIdsInTheOrderTheRequestListedThem() async { + func skippedQuestionsAreAbsentFromTheResponse() async { let conversation = ConversationModel() - let pending = request(multiple: true) - let answering = Task { await conversation.request(pending) } + let answering = Task { await conversation.request(threeQuestions()) } + await settle(conversation) + + conversation.toggleUserInputOption("flyway", in: "migrations") + conversation.submitUserInput() + + #expect( + await answering.value + == .answered([ + AgentUserInputAnswer( + questionID: "migrations", value: .choice(selectedIDs: ["flyway"], custom: nil) + ) + ]) + ) + } + + @Test + func answerReportsIdsInTheOrderTheQuestionListedThem() async { + let conversation = ConversationModel() + let answering = Task { await conversation.request(request(multiple: true)) } await settle(conversation) // Clicked back to front; the answer still reads front to back, because // the model wrote the list and reads the reply against it. - conversation.toggleUserInputOption("duckdb") - conversation.toggleUserInputOption("postgres") + conversation.toggleUserInputOption("duckdb", in: "storage") + conversation.toggleUserInputOption("postgres", in: "storage") conversation.submitUserInput() - #expect(await answering.value == .choice(selectedIDs: ["postgres", "duckdb"], custom: nil)) + #expect( + await answering.value + == .answered([ + AgentUserInputAnswer( + questionID: "storage", + value: .choice(selectedIDs: ["postgres", "duckdb"], custom: nil) + ) + ]) + ) } @Test @@ -163,28 +286,36 @@ struct AgentUserInputTests { let answering = Task { await conversation.request(request(multiple: false)) } await settle(conversation) - conversation.setUserInputCustomAnswer(" MySQL ") + conversation.setUserInputCustomAnswer(" MySQL ", for: "storage") conversation.submitUserInput() // Trimmed, because trailing whitespace is not part of anyone's answer. - #expect(await answering.value == .choice(selectedIDs: [], custom: "MySQL")) + #expect( + await answering.value + == .answered([ + AgentUserInputAnswer( + questionID: "storage", value: .choice(selectedIDs: [], custom: "MySQL") + ) + ]) + ) } @Test func cancellingReportsCancelledRatherThanAnEmptyAnswer() async { let conversation = ConversationModel() - let answering = Task { await conversation.request(request(multiple: false)) } + let answering = Task { await conversation.request(threeQuestions()) } await settle(conversation) + conversation.toggleUserInputOption("postgres", in: "storage") conversation.cancelUserInput() #expect(await answering.value == .cancelled) #expect(conversation.pendingUserInput == nil) - #expect(conversation.userInputSelection.isEmpty) + #expect(conversation.userInputDraft(for: "storage").selection.isEmpty) } @Test - func submittingNothingIsRefusedRatherThanSentAsAnEmptyChoice() async { + func submittingNothingIsRefusedRatherThanSentAsAnEmptyAnswerList() async { let conversation = ConversationModel() let answering = Task { await conversation.request(request(multiple: false)) } await settle(conversation) @@ -192,27 +323,135 @@ struct AgentUserInputTests { conversation.submitUserInput() #expect(conversation.pendingUserInput != nil, "the request must still be waiting") - conversation.toggleUserInputOption("sqlite") + conversation.toggleUserInputOption("sqlite", in: "storage") conversation.submitUserInput() - #expect(await answering.value == .choice(selectedIDs: ["sqlite"], custom: nil)) + #expect( + await answering.value + == .answered([ + AgentUserInputAnswer( + questionID: "storage", value: .choice(selectedIDs: ["sqlite"], custom: nil) + ) + ]) + ) } @Test func aFreeFormQuestionStillAnswersAsPlainText() async { let conversation = ConversationModel() - let answering = Task { - await conversation.request(AgentUserInputRequest(prompt: "Base URL?", kind: .text)) - } + let pending = AgentUserInputRequest( + questions: [AgentUserQuestion(id: "base_url", prompt: "Base URL?", kind: .text)] + ) + let answering = Task { await conversation.request(pending) } await settle(conversation) - conversation.userInputDraft = "https://api.example.com" + conversation.setUserInputCustomAnswer("https://api.example.com", for: "base_url") conversation.submitUserInput() - #expect(await answering.value == .text("https://api.example.com")) + #expect( + await answering.value + == .answered([ + AgentUserInputAnswer( + questionID: "base_url", value: .text("https://api.example.com") + ) + ]) + ) + } + + @Test + func aSecretAnswersThroughItsOwnDraftAndNotTheSharedOne() async { + let conversation = ConversationModel() + let pending = AgentUserInputRequest( + questions: [AgentUserQuestion(id: "secret", prompt: "Sudo password?", kind: .secret)] + ) + let answering = Task { await conversation.request(pending) } + await settle(conversation) + + #expect(!conversation.canSubmitUserInput) + conversation.secretInputDraft = "hunter2" + #expect(conversation.canSubmitUserInput) + conversation.submitUserInput() + + #expect( + await answering.value + == .answered([ + AgentUserInputAnswer(questionID: "secret", value: .secret("hunter2")) + ]) + ) + // Never left behind in the plain drafts, which are what the transcript + // and the next question read from. + #expect(conversation.userInputDraft(for: "secret").text.isEmpty) + #expect(conversation.secretInputDraft.isEmpty) + } + + // MARK: - What the model reads + + /// The skipped question still appears, carrying a null. Omitting it would + /// read as a bug; a null says plainly that this one is now the model's call. + @Test + func everyQuestionIsReportedWhetherAnsweredOrNot() { + let questions = threeQuestions().questions + let rows = AgentBuiltInTools.reported( + questions, + [ + AgentUserInputAnswer( + questionID: "migrations", + value: .choice(selectedIDs: ["flyway", "liquibase"], custom: "or none") + ), + AgentUserInputAnswer(questionID: "host", value: .text("staging-2")), + ] + ) + + #expect(rows.count == 3) + // Order follows the questions, not the answers. + #expect(field(rows[0], "question_id") == .string("storage")) + #expect(field(rows[0], "answer") == .null) + #expect(field(rows[0], "selected_option_ids") == .array([])) + #expect(field(rows[0], "custom_text") == .null) + + // A typed answer wins the canonical `answer`, and the ids still travel. + #expect(field(rows[1], "answer") == .string("or none")) + #expect( + field(rows[1], "selected_option_ids") + == .array([.string("flyway"), .string("liquibase")]) + ) + #expect(field(rows[2], "answer") == .string("staging-2")) + } + + /// With no typed answer the canonical `answer` is the labels, not the ids — + /// a model that reads one field should read something a human wrote. + @Test + func selectedLabelsBecomeTheCanonicalAnswer() { + let rows = AgentBuiltInTools.reported( + [storage(multiple: true)], + [ + AgentUserInputAnswer( + questionID: "storage", + value: .choice(selectedIDs: ["postgres", "duckdb"], custom: nil) + ) + ] + ) + #expect(field(rows[0], "answer") == .string("Postgres, DuckDB")) + #expect(field(rows[0], "custom_text") == .null) + } + + /// Unreachable through `request_user_input`, which never builds a secret + /// question — but if one ever arrives, the plaintext must not be echoed. + @Test + func aSecretIsNeverReportedBackToTheModel() { + let rows = AgentBuiltInTools.reported( + [AgentUserQuestion(id: "secret", prompt: "Password?", kind: .secret)], + [AgentUserInputAnswer(questionID: "secret", value: .secret("hunter2"))] + ) + #expect(field(rows[0], "answer") == .null) + #expect(field(rows[0], "custom_text") == .null) } // MARK: - Helpers + private func field(_ row: AgentJSONValue, _ key: String) -> AgentJSONValue? { + row.objectValue?[key] + } + private func arguments(_ object: [String: AgentJSONValue]) throws -> Arguments { try Arguments( AgentToolInvocation( @@ -225,8 +464,31 @@ struct AgentUserInputTests { } private func request(multiple: Bool) -> AgentUserInputRequest { - AgentUserInputRequest( - prompt: "Which database?", + AgentUserInputRequest(questions: [storage(multiple: multiple)]) + } + + private func threeQuestions() -> AgentUserInputRequest { + AgentUserInputRequest(questions: [ + storage(multiple: false), + AgentUserQuestion( + id: "migrations", header: "Migrations", prompt: "Which tools may we use?", + kind: .choice( + AgentUserInputChoice( + options: [ + AgentUserInputOption(id: "flyway", label: "Flyway"), + AgentUserInputOption(id: "liquibase", label: "Liquibase"), + ], + allowsMultipleAnswers: true + ) + ) + ), + AgentUserQuestion(id: "host", prompt: "Which host is staging?", kind: .text), + ]) + } + + private func storage(multiple: Bool) -> AgentUserQuestion { + AgentUserQuestion( + id: "storage", header: "Storage", prompt: "Which database?", kind: .choice( AgentUserInputChoice( options: [ @@ -239,8 +501,7 @@ struct AgentUserInputTests { ], allowsMultipleAnswers: multiple ) - ), - header: "Storage" + ) ) }