diff --git a/Termind/Application/Agent/AgentBuiltInTools.swift b/Termind/Application/Agent/AgentBuiltInTools.swift index 789153fa..b764171b 100644 --- a/Termind/Application/Agent/AgentBuiltInTools.swift +++ b/Termind/Application/Agent/AgentBuiltInTools.swift @@ -689,22 +689,75 @@ nonisolated enum AgentBuiltInTools { private static func requestUserInput() -> AnyAgentTool { AnyAgentTool(descriptor: descriptor( - "request_user_input", "Pause and ask the user for text or one structured choice.", + "request_user_input", + """ + Ask the user to decide something you cannot decide yourself, and wait for the answer. \ + 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. + """, properties: [ "prompt": string(max: 4_096), - "choices": array(items: string(max: 256), max: 20), + "header": string(max: 24), + "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: ["prompt"], target: .user, safety: .locallyReadOnly )) { invocation, context in let arguments = try Arguments(invocation) - let choices = arguments.optionalStrings("choices") + let options = try arguments.userInputOptions() let request = AgentUserInputRequest( prompt: try arguments.string("prompt"), - kind: choices.isEmpty ? .text : .choice(choices) + 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") ?? "" ) switch await context.userInteraction.request(request) { - case .text(let answer): return result(invocation, .object(["answer": .string(answer)])) - case .secret: throw AgentToolError.invalidArguments("Secret input must use request_user_secret.") - case .cancelled: throw AgentToolError.cancelled + case .text(let answer): + return result(invocation, .object(["answer": .string(answer)])) + case .choice(let selectedIDs, let custom): + let labels = selectedIDs.compactMap { id in + options.first { $0.id == id }?.label + } + return result(invocation, .object([ + // 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, + ])) + case .secret: + throw AgentToolError.invalidArguments("Secret input must use request_user_secret.") + case .cancelled: + throw AgentToolError.cancelled } } } @@ -737,7 +790,11 @@ nonisolated enum AgentBuiltInTools { return result(invocation, .object([ "secret_handle": .string(handle.id.uuidString), "single_use": .bool(true), ])) - case .text: throw AgentToolError.invalidArguments("The secret prompt returned ordinary text.") + 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 } } @@ -932,6 +989,35 @@ nonisolated struct Arguments: Sendable { object[key]?.arrayValue?.compactMap(\.stringValue) ?? [] } + /// The `options` 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 [] } + guard let rows = value.arrayValue else { + throw AgentToolError.invalidArguments("options must be an array.") + } + var seen: Set = [] + return try rows.map { row in + guard let fields = row.objectValue, + let id = fields["id"]?.stringValue, !id.isEmpty, + let label = fields["label"]?.stringValue, !label.isEmpty + else { + throw AgentToolError.invalidArguments("Each option requires id and label.") + } + guard seen.insert(id).inserted else { + throw AgentToolError.invalidArguments("Option ids must be unique: \(id).") + } + return AgentUserInputOption( + id: id, label: label, + detail: fields["description"]?.stringValue ?? "", + isRecommended: fields["recommended"]?.boolValue ?? false + ) + } + } + func requestHeaders() throws -> [RestrictedHTTPHeader] { guard let value = object["headers"], value != .null else { return [] } guard let rows = value.arrayValue else { diff --git a/Termind/Application/Agent/AgentSecurity.swift b/Termind/Application/Agent/AgentSecurity.swift index 8ea6b2a2..0fb2011b 100644 --- a/Termind/Application/Agent/AgentSecurity.swift +++ b/Termind/Application/Agent/AgentSecurity.swift @@ -173,17 +173,57 @@ actor SecretBroker { var count: Int { entries.count } } +/// One choice offered by `request_user_input`. +/// +/// A bare string used to be the whole option, which left the model nowhere to +/// put the tradeoff: it either crammed it into the label or lost it, and the +/// reader was left choosing between four words with no way to tell them apart. +nonisolated struct AgentUserInputOption: Identifiable, Hashable, Sendable { + var id: String + var label: String + /// One sentence on what this choice means or costs. Optional, and the + /// reason this type exists. + var detail: String = "" + /// Advisory only. It marks the model's recommendation; it never preselects, + /// because a preselected answer is one the user can submit without reading. + var isRecommended: Bool = false +} + +nonisolated struct AgentUserInputChoice: Hashable, Sendable { + var options: [AgentUserInputOption] + /// Whether the user may answer with something the model did not think of. + /// + /// Defaults to true, and that default is the whole point: an agent that + /// offers four wrong answers and no fifth field leaves Cancel as the only + /// honest response, and a cancelled run tells it nothing about why. + var allowsCustomAnswer: Bool = true + var allowsMultipleAnswers: Bool = false + var customPlaceholder: String = "" +} + nonisolated struct AgentUserInputRequest: Identifiable, Hashable, Sendable { - nonisolated enum Kind: Hashable, Sendable { case text, choice([String]), secret } + nonisolated enum Kind: Hashable, Sendable { + case text + case choice(AgentUserInputChoice) + case secret + } + var id: UUID = UUID() var prompt: String var kind: Kind + /// Short label naming the decision, shown above the prompt. + var header: String = "" var purpose: String = "" var hostID: UUID? } 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) case cancelled } diff --git a/Termind/Features/AI/ConversationModel.swift b/Termind/Features/AI/ConversationModel.swift index 6b6d9d24..f266896c 100644 --- a/Termind/Features/AI/ConversationModel.swift +++ b/Termind/Features/AI/ConversationModel.swift @@ -197,7 +197,10 @@ final class ConversationModel: Identifiable { var toolCards: [AgentToolCard] = [] var pendingApproval: AgentApprovalRequest? var pendingUserInput: AgentUserInputRequest? + /// The free-form answer, for a text question and for the "something else" + /// field beside a set of options. var userInputDraft = "" + var userInputSelection: Set = [] var secretInputDraft = "" /// The model-facing history, which is not what `messages` holds. /// @@ -433,31 +436,88 @@ final class ConversationModel: Identifiable { func request(_ request: AgentUserInputRequest) async -> AgentUserInputResponse { pendingUserInput = request userInputDraft = "" + userInputSelection = [] secretInputDraft = "" runState = .waitingForUser return await withCheckedContinuation { inputWaiters[request.id] = $0 } } + /// 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 { + case .secret: return !secretInputDraft.isEmpty + case .text: return !trimmedUserInputDraft.isEmpty + case .choice: return !userInputSelection.isEmpty || !trimmedUserInputDraft.isEmpty + } + } + + func toggleUserInputOption(_ id: String) { + guard case .choice(let choice)? = pendingUserInput?.kind else { return } + if choice.allowsMultipleAnswers { + if userInputSelection.contains(id) { + userInputSelection.remove(id) + } else { + userInputSelection.insert(id) + } + } else { + userInputSelection = userInputSelection.contains(id) ? [] : [id] + // One answer means one answer: a half-typed "something else" left + // sitting under a chosen radio would be a second, contradicting one. + userInputDraft = "" + } + } + + /// 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, + !choice.allowsMultipleAnswers, !text.isEmpty + else { return } + userInputSelection.removeAll() + } + func submitUserInput() { - guard let request = pendingUserInput else { return } + guard let request = pendingUserInput, canSubmitUserInput else { return } let response: AgentUserInputResponse switch request.kind { - case .secret: response = .secret(secretInputDraft) - case .text, .choice: response = .text(userInputDraft) + 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 + ) } - secretInputDraft = "" - userInputDraft = "" - pendingUserInput = nil + clearUserInput() runState = .running inputWaiters.removeValue(forKey: request.id)?.resume(returning: response) } func cancelUserInput() { guard let request = pendingUserInput else { return } + clearUserInput() + inputWaiters.removeValue(forKey: request.id)?.resume(returning: .cancelled) + } + + private var trimmedUserInputDraft: String { + userInputDraft.trimmingCharacters(in: .whitespacesAndNewlines) + } + + private func clearUserInput() { secretInputDraft = "" userInputDraft = "" + userInputSelection = [] pendingUserInput = nil - inputWaiters.removeValue(forKey: request.id)?.resume(returning: .cancelled) } func handle(_ event: AgentEvent) { diff --git a/Termind/Features/AI/ConversationTranscriptView.swift b/Termind/Features/AI/ConversationTranscriptView.swift index 6d490bea..21c21765 100644 --- a/Termind/Features/AI/ConversationTranscriptView.swift +++ b/Termind/Features/AI/ConversationTranscriptView.swift @@ -237,6 +237,12 @@ private struct AgentApprovalCard: View { } } +/// The agent asking the user to decide something. +/// +/// 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. private struct AgentUserInputCard: View { let request: AgentUserInputRequest let conversation: ConversationModel @@ -244,27 +250,119 @@ private struct AgentUserInputCard: View { var body: some View { @Bindable var conversation = conversation VStack(alignment: .leading, spacing: Metrics.Space.s) { - Text(request.prompt).font(.callout.weight(.medium)) + if !request.header.isEmpty { + Text(request.header) + .font(.caption.weight(.semibold)) + .foregroundStyle(.secondary) + } + 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 choices): - Picker("Answer", selection: $conversation.userInputDraft) { - Text("Choose…").tag("") - ForEach(choices, id: \.self) { Text($0).tag($0) } - } + 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) } } .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) + } + if choice.allowsCustomAnswer { + customRow(choice) + } + } + } + + private func optionRow(_ option: AgentUserInputOption, choice: AgentUserInputChoice) + -> some View + { + let isSelected = conversation.userInputSelection.contains(option.id) + return Button { + conversation.toggleUserInputOption(option.id) + } label: { + HStack(alignment: .firstTextBaseline, spacing: Metrics.Space.s) { + Image(systemName: marker(isSelected: isSelected, choice: choice)) + .foregroundStyle(isSelected ? AnyShapeStyle(.tint) : AnyShapeStyle(.secondary)) + .frame(width: Metrics.Sidebar.rowIcon) + + VStack(alignment: .leading, spacing: Metrics.Space.xxs) { + HStack(spacing: Metrics.Space.s) { + Text(option.label).font(.callout) + if option.isRecommended { + Text("Recommended") + .font(.caption2.weight(.medium)) + .padding(.horizontal, Metrics.Space.s) + .padding(.vertical, 1) + .background(.tint.opacity(0.15), in: .capsule) + .foregroundStyle(.tint) + } + } + if !option.detail.isEmpty { + Text(option.detail) + .font(.caption) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + } + } + Spacer(minLength: 0) + } + .contentShape(.rect) + } + .buttonStyle(.plain) + } + + /// 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) + + TextField( + choice.customPlaceholder.isEmpty + ? String(localized: "Something else…") : choice.customPlaceholder, + text: Binding( + get: { conversation.userInputDraft }, + set: { conversation.setUserInputCustomAnswer($0) } + ) + ) + .textFieldStyle(.roundedBorder) + } + } + + private func marker(isSelected: Bool, choice: AgentUserInputChoice) -> String { + if choice.allowsMultipleAnswers { + isSelected ? "checkmark.square.fill" : "square" + } else { + isSelected ? "largecircle.fill.circle" : "circle" + } + } } diff --git a/TermindTests/AgentUserInputTests.swift b/TermindTests/AgentUserInputTests.swift new file mode 100644 index 00000000..6b9f1eb2 --- /dev/null +++ b/TermindTests/AgentUserInputTests.swift @@ -0,0 +1,256 @@ +import Foundation +import Testing + +@testable import Termind + +@MainActor +@Suite +struct AgentUserInputTests { + + // MARK: - Parsing the tool's options + + @Test + func optionsCarryTheirDescriptionAndRecommendation() throws { + let options = try arguments([ + "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")]), + ]), + ]).userInputOptions() + + #expect(options.count == 2) + #expect(options[0].id == "postgres") + #expect(options[0].detail == "JSONB, and it matches the existing schema.") + #expect(options[0].isRecommended) + // Absent is absent, not false-y noise. + #expect(options[1].detail.isEmpty) + #expect(!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() + } + #expect(throws: AgentToolError.self) { + try arguments([ + "prompt": .string("?"), + "options": .array([.string("a bare string")]), + ]).userInputOptions() + } + } + + @Test + func duplicateOptionIdsAreRejected() { + #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() + } + } + + // MARK: - One answer, or several + + @Test + func nothingIsPreselectedEvenWhenAnOptionIsRecommended() { + let conversation = ConversationModel() + conversation.pendingUserInput = request(multiple: false) + + #expect(conversation.userInputSelection.isEmpty) + // A recommendation is a label, not a default — Continue stays off until + // the user actually answers. + #expect(!conversation.canSubmitUserInput) + } + + @Test + func choosingAnOptionInASingleAnswerQuestionClearsATypedOne() { + let conversation = ConversationModel() + conversation.pendingUserInput = request(multiple: false) + + conversation.setUserInputCustomAnswer("MySQL") + #expect(conversation.canSubmitUserInput) + + conversation.toggleUserInputOption("postgres") + #expect(conversation.userInputSelection == ["postgres"]) + #expect(conversation.userInputDraft.isEmpty) + } + + @Test + func typingInASingleAnswerQuestionClearsTheChosenOption() { + let conversation = ConversationModel() + conversation.pendingUserInput = request(multiple: false) + + conversation.toggleUserInputOption("postgres") + conversation.setUserInputCustomAnswer("MySQL") + + #expect(conversation.userInputSelection.isEmpty) + #expect(conversation.userInputDraft == "MySQL") + } + + @Test + func singleAnswerSelectionReplacesRatherThanAccumulates() { + let conversation = ConversationModel() + conversation.pendingUserInput = request(multiple: false) + + conversation.toggleUserInputOption("postgres") + conversation.toggleUserInputOption("sqlite") + #expect(conversation.userInputSelection == ["sqlite"]) + + // Clicking the chosen one again takes the answer back. + conversation.toggleUserInputOption("sqlite") + #expect(conversation.userInputSelection.isEmpty) + } + + /// When several answers are allowed the two routes are additive, not + /// exclusive — "these two, plus this other thing" is a real answer. + @Test + func severalAnswersMayIncludeATypedOne() { + let conversation = ConversationModel() + conversation.pendingUserInput = request(multiple: true) + + conversation.toggleUserInputOption("postgres") + conversation.toggleUserInputOption("sqlite") + conversation.setUserInputCustomAnswer("and DuckDB") + + #expect(conversation.userInputSelection == ["postgres", "sqlite"]) + #expect(conversation.userInputDraft == "and DuckDB") + } + + // MARK: - What the tool gets back + + @Test + func answerReportsIdsInTheOrderTheRequestListedThem() async { + let conversation = ConversationModel() + let pending = request(multiple: true) + let answering = Task { await conversation.request(pending) } + 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.submitUserInput() + + #expect(await answering.value == .choice(selectedIDs: ["postgres", "duckdb"], custom: nil)) + } + + @Test + func aTypedAnswerComesBackAsCustomTextWithNoSelection() async { + let conversation = ConversationModel() + let answering = Task { await conversation.request(request(multiple: false)) } + await settle(conversation) + + conversation.setUserInputCustomAnswer(" MySQL ") + conversation.submitUserInput() + + // Trimmed, because trailing whitespace is not part of anyone's answer. + #expect(await answering.value == .choice(selectedIDs: [], custom: "MySQL")) + } + + @Test + func cancellingReportsCancelledRatherThanAnEmptyAnswer() async { + let conversation = ConversationModel() + let answering = Task { await conversation.request(request(multiple: false)) } + await settle(conversation) + + conversation.cancelUserInput() + + #expect(await answering.value == .cancelled) + #expect(conversation.pendingUserInput == nil) + #expect(conversation.userInputSelection.isEmpty) + } + + @Test + func submittingNothingIsRefusedRatherThanSentAsAnEmptyChoice() async { + let conversation = ConversationModel() + let answering = Task { await conversation.request(request(multiple: false)) } + await settle(conversation) + + conversation.submitUserInput() + #expect(conversation.pendingUserInput != nil, "the request must still be waiting") + + conversation.toggleUserInputOption("sqlite") + conversation.submitUserInput() + #expect(await answering.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)) + } + await settle(conversation) + + conversation.userInputDraft = "https://api.example.com" + conversation.submitUserInput() + + #expect(await answering.value == .text("https://api.example.com")) + } + + // MARK: - Helpers + + private func arguments(_ object: [String: AgentJSONValue]) throws -> Arguments { + try Arguments( + AgentToolInvocation( + runID: UUID(), + call: AgentToolCall( + id: "call-1", name: "request_user_input", arguments: .object(object) + ) + ) + ) + } + + private func request(multiple: Bool) -> AgentUserInputRequest { + AgentUserInputRequest( + prompt: "Which database?", + kind: .choice( + AgentUserInputChoice( + options: [ + AgentUserInputOption( + id: "postgres", label: "Postgres", + detail: "Matches the existing schema.", isRecommended: true + ), + AgentUserInputOption(id: "sqlite", label: "SQLite"), + AgentUserInputOption(id: "duckdb", label: "DuckDB"), + ], + allowsMultipleAnswers: multiple + ) + ), + header: "Storage" + ) + } + + /// `request(_:)` publishes the pending question and then suspends on its + /// continuation. Waiting for that to be installed is what keeps these tests + /// from racing the answer in ahead of the question. + private func settle(_ conversation: ConversationModel) async { + for _ in 0..<100 where conversation.pendingUserInput == nil { + await Task.yield() + } + #expect(conversation.pendingUserInput != nil, "the request never became pending") + } +}