diff --git a/android/README.md b/android/README.md index cdbe27e5..62f69551 100644 --- a/android/README.md +++ b/android/README.md @@ -322,7 +322,12 @@ reveals the original input/result, including fields omitted from the preview. Mixed text/media results stay JSON instead of dropping non-text blocks. Question details show recorded selections, custom answers and notes with -Markdown questions/options. `request_user_input` also restores answers from +Markdown questions/options. Codex choice questions with `isOther: true` add +**None of the above** and focus optional notes when selected; empty notes are +valid. Translations never change the submitted `None of the above` wire value. +Pi/MCP forms without `isOther` keep their existing choices. Recorded other +answers and notes are also shown in question details. +`request_user_input` also restores answers from historical results; live permission answers take precedence. Answered cards avoid duplicate results, but retain errors and the full input/result/answers under **Source**. Pending questions are answered in the conversation. diff --git a/android/app/src/androidTest/kotlin/app/hapi/companion/feature/chat/QuestionDetailsTest.kt b/android/app/src/androidTest/kotlin/app/hapi/companion/feature/chat/QuestionDetailsTest.kt index 26e8e5d4..75a806d0 100644 --- a/android/app/src/androidTest/kotlin/app/hapi/companion/feature/chat/QuestionDetailsTest.kt +++ b/android/app/src/androidTest/kotlin/app/hapi/companion/feature/chat/QuestionDetailsTest.kt @@ -10,14 +10,17 @@ import androidx.compose.ui.test.SemanticsMatcher import androidx.compose.ui.test.assert import androidx.compose.ui.test.assertHasNoClickAction import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsFocused import androidx.compose.ui.test.assertIsNotSelected import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.junit4.StateRestorationTester import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo +import androidx.compose.ui.test.performTextInput import app.hapi.companion.feature.chat.blocks.PendingPermissionFooter import app.hapi.companion.feature.chat.blocks.ToolCallBody import app.hapi.companion.ui.theme.HapiTheme @@ -35,6 +38,62 @@ class QuestionDetailsTest { private fun pending() = ChatToolCall(id = "question", name = "request_user_input", state = "running", input = input, createdAt = 0, description = null, permission = ToolPermission(id = "reply", status = "pending", presence = emptySet())) + private fun otherTool(multiple: Boolean = false) = pending().copy(input = HapiJson.parseToJsonElement( + """{"questions":[{"id":"target","question":"Choose","isOther":true,"multiple":$multiple,"options":[{"label":"Alpha"},{"label":"Beta"}]}]}""", + )) + + @Test fun otherChoiceFocusesOptionalNotesAndSubmitsWithoutAnyNote() { + val submitted = mutableListOf>() + compose.setContent { + HapiTheme { PendingPermissionFooter(tool = otherTool(), requestId = "reply", flavor = "codex", override = null, + onAction = { id, action -> submitted += id to action }) } + } + compose.onNodeWithTag("request-input-target-option-0").performClick() + compose.onNodeWithTag("request-input-target-option-2").performClick().assertIsSelected() + compose.onNodeWithTag("request-input-target-option-0").assertIsNotSelected() + compose.onNodeWithTag("request-input-target-note").assertIsFocused() + compose.runOnIdle { assertEquals(emptyList>(), submitted) } + compose.onNodeWithText("Submit").performClick() + compose.runOnIdle { + assertEquals(listOf("reply" to PermissionAction.NestedAnswers(mapOf("target" to listOf("None of the above")))), submitted) + } + } + + @Test fun otherChoiceIsExclusiveAndRestoresNotesWithoutChangingWireValues() { + val restoration = StateRestorationTester(compose) + var submitted: PermissionAction? = null + restoration.setContent { + HapiTheme { PendingPermissionFooter(tool = otherTool(multiple = true), requestId = "reply", flavor = "codex", override = null, + onAction = { _, action -> submitted = action }) } + } + compose.onNodeWithTag("request-input-target-option-0").performClick() + compose.onNodeWithTag("request-input-target-option-1").performClick() + compose.onNodeWithTag("request-input-target-option-2").performClick() + compose.onNodeWithTag("request-input-target-note").performTextInput(" 自定义\n说明 ") + compose.onNodeWithTag("request-input-target-option-1").performClick().assertIsSelected() + compose.onNodeWithTag("request-input-target-option-2").assertIsNotSelected().performClick() + compose.onNodeWithTag("request-input-target-option-0").assertIsNotSelected() + compose.onNodeWithTag("request-input-target-option-1").assertIsNotSelected() + restoration.emulateSavedInstanceStateRestore() + compose.onNodeWithTag("request-input-target-option-2").assertIsSelected() + compose.onNodeWithText("Submit").performClick() + compose.runOnIdle { + assertEquals(PermissionAction.NestedAnswers(mapOf("target" to listOf("None of the above", "user_note: 自定义\n说明"))), submitted) + } + } + + @Test fun recordedOtherAnswerIsSelectedReadOnlyAndKeepsItsNote() { + val tool = otherTool().copy(state = "completed", result = HapiJson.parseToJsonElement( + """{"answers":{"target":{"answers":["None of the above","user_note: recorded note"]}}}""", + )) + compose.setContent { + HapiTheme { Column(Modifier.verticalScroll(rememberScrollState())) { ToolCallBody(tool, basePath = null) } } + } + compose.waitUntil(10_000) { compose.onAllNodesWithText("None of the above").fetchSemanticsNodes().isNotEmpty() } + compose.onNodeWithTag("question-0-option-2").assertIsSelected().assertHasNoClickAction() + compose.onNodeWithText("recorded note").performScrollTo().assertIsDisplayed() + } + @Test fun detailsFollowLiveAnswersWithoutActionsAndKeepSourceAndErrorsAccessible() { val tool = mutableStateOf(pending()) compose.setContent { @@ -74,6 +133,7 @@ class QuestionDetailsTest { } } compose.onNodeWithText("staging").performClick() + compose.onNodeWithTag("request-input-target-option-2").assertDoesNotExist() compose.onNodeWithText("Submit").performClick() compose.runOnIdle { assertEquals("reply" to PermissionAction.NestedAnswers(mapOf("target" to listOf("staging", "user_note: production"))), resolved) diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/PermissionActionViews.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/PermissionActionViews.kt index db3d7437..9795f229 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/PermissionActionViews.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/PermissionActionViews.kt @@ -28,6 +28,9 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusRequester +import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.tooling.preview.Preview @@ -41,6 +44,7 @@ import app.hapi.companion.feature.chat.permissions.isRequestUserInputAnswered import app.hapi.companion.feature.chat.permissions.parseAskUserQuestions import app.hapi.companion.feature.chat.permissions.parseRequestUserInputQuestions import app.hapi.companion.feature.chat.permissions.requestUserInputAnswerValues +import app.hapi.companion.feature.chat.permissions.selectRequestUserInputOption import app.hapi.companion.ui.theme.HapiTheme import app.hapi.companion.ui.theme.hapi import app.hapi.protocol.chat.ChatToolCall @@ -427,6 +431,7 @@ private fun RequestUserInputFooter( verticalArrangement = Arrangement.spacedBy(10.dp), ) { questions.forEach { question -> + val noteFocusRequester = remember(tool.id, question.id) { FocusRequester() } Column(verticalArrangement = Arrangement.spacedBy(6.dp)) { if (question.question.isNotEmpty()) { Text( @@ -438,7 +443,8 @@ private fun RequestUserInputFooter( style = MaterialTheme.typography.bodyMedium, ) } - question.options.forEach { option -> + question.answerOptions.forEachIndexed { optionIndex, option -> + val isOther = question.isOtherOption(optionIndex) val checked = option.label in (selected.value[question.id] ?: emptySet()) Surface( shape = RoundedCornerShape(10.dp), @@ -451,15 +457,12 @@ private fun RequestUserInputFooter( .fillMaxWidth() .selectable(selected = checked, enabled = !resolving, onClick = { val current = selected.value[question.id] ?: emptySet() - val next = when { - question.multiple -> - if (option.label in current) current - option.label - else current + option.label - else -> setOf(option.label) - } + val next = selectRequestUserInputOption(question, current, optionIndex) selected.value = selected.value + (question.id to next) validationError = null - }), + if (isOther) noteFocusRequester.requestFocus() + }) + .testTag("request-input-${question.id}-option-$optionIndex"), ) { Row( modifier = Modifier.padding(horizontal = 10.dp, vertical = 6.dp), @@ -471,8 +474,12 @@ private fun RequestUserInputFooter( RadioButton(selected = checked, onClick = null, enabled = !resolving) } Column(modifier = Modifier.padding(start = 6.dp)) { - Text(text = option.label, style = MaterialTheme.typography.bodyMedium) - option.description?.let { description -> + Text( + text = if (isOther) stringResource(R.string.chat_perm_none_of_the_above) else option.label, + style = MaterialTheme.typography.bodyMedium, + ) + val description = if (isOther) stringResource(R.string.chat_perm_other_note_optional) else option.description + description?.let { Text( text = description, style = MaterialTheme.typography.labelSmall, @@ -491,7 +498,9 @@ private fun RequestUserInputFooter( }, enabled = !resolving, placeholder = { Text(question.placeholder ?: stringResource(R.string.chat_perm_add_note)) }, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth() + .focusRequester(noteFocusRequester) + .testTag("request-input-${question.id}-note"), ) } } diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionDetailsView.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionDetailsView.kt index cd59c683..9da58541 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionDetailsView.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionDetailsView.kt @@ -39,7 +39,8 @@ internal fun QuestionDetailsView(questions: List) { } question.options.forEachIndexed { optionIndex, option -> QuestionAnswerCard( - text = option.label, description = option.description, markdown = true, + text = if (option.isOther) stringResource(R.string.chat_perm_none_of_the_above) else option.label, + description = option.description, markdown = true, isSelected = option.selected, multiple = question.multiple, showControl = question.hasAnswers, modifier = Modifier.testTag("question-$index-option-$optionIndex"), ) diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentation.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentation.kt index 23ab8aea..fa98599b 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentation.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentation.kt @@ -30,7 +30,9 @@ internal data class QuestionDetail( val hasAnswers: Boolean get() = options.any { it.selected } || otherAnswers.isNotEmpty() || note != null } -internal data class QuestionOptionDetail(val label: String, val description: String?, val selected: Boolean) +internal data class QuestionOptionDetail( + val label: String, val description: String?, val selected: Boolean, val isOther: Boolean = false, +) internal fun isQuestionDetailsTool(name: String): Boolean { val name = toolPresentationName(name) @@ -93,12 +95,13 @@ private fun questionAnswerMap(value: JsonElement?): Map> { private fun requestQuestionDetails( questions: List, answers: Map>, ): List = questions.map { question -> + val options = question.answerOptions val selected = mutableSetOf() val other = mutableListOf() var note: String? = null for (value in answers[question.id].orEmpty()) { val trimmed = value.trim() - val option = question.options.firstOrNull { it.label == trimmed } + val option = options.firstOrNull { it.label == trimmed } when { // An actual option named "user_note: ..." is not metadata. option != null -> selected += option.label @@ -112,7 +115,9 @@ private fun requestQuestionDetails( } QuestionDetail( header = null, question = question.question, multiple = question.multiple, - options = question.options.map { QuestionOptionDetail(it.label, it.description, it.label in selected) }, + options = options.mapIndexed { index, option -> + QuestionOptionDetail(option.label, option.description, option.label in selected, question.isOtherOption(index)) + }, otherAnswers = other, note = note, ) } diff --git a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/permissions/PermissionInputs.kt b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/permissions/PermissionInputs.kt index 5141d4c5..4bbff39f 100644 --- a/android/app/src/main/kotlin/app/hapi/companion/feature/chat/permissions/PermissionInputs.kt +++ b/android/app/src/main/kotlin/app/hapi/companion/feature/chat/permissions/PermissionInputs.kt @@ -101,6 +101,9 @@ fun parseAskUserQuestions(input: JsonElement?, cursorDialect: Boolean): List + get() = if (isOther && options.isNotEmpty()) { + options + AskOption(id = null, label = REQUEST_USER_INPUT_OTHER, description = null) + } else options + + fun isOtherOption(index: Int): Boolean = isOther && options.isNotEmpty() && index == options.size +} + +fun selectRequestUserInputOption(question: RequestUserInputQuestion, selected: Set, index: Int): Set { + val option = question.answerOptions.getOrNull(index) ?: return selected + if (question.isOtherOption(index) || !question.multiple) return setOf(option.label) + val current = if (question.isOther) selected - REQUEST_USER_INPUT_OTHER else selected + return if (option.label in current) current - option.label else current + option.label +} /** `parseRequestUserInputInput` (the URL-confirmation flow is web-only). */ fun parseRequestUserInputQuestions(input: JsonElement?): List { @@ -144,6 +162,7 @@ fun parseRequestUserInputQuestions(input: JsonElement?): List其他… %1$s（可选） 添加备注… + 以上都不是 + 可在备注中补充说明（可选）。 打开此链接？ diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index 830ed38b..286f1547 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -359,6 +359,8 @@ Other… %1$s (optional) Add a note… + None of the above + Optionally, add details in notes. Open link? diff --git a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentationTest.kt b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentationTest.kt index 7466c717..696bf0fe 100644 --- a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentationTest.kt +++ b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/blocks/QuestionToolPresentationTest.kt @@ -12,6 +12,35 @@ import kotlin.test.assertNull import kotlin.test.assertTrue class QuestionToolPresentationTest { + @Test fun otherAndNotesShowFromRecordedAnswersWithoutChangingRawInput() { + for (flag in listOf(""", "isOther":true""", "")) { + val input = """{"questions":[{"id":"choice"$flag,"options":[{"label":"A"}]}]}""" + for (note in listOf("", """, "user_note: 自定义\n说明"""")) { + val source = """{"answers":{"choice":{"answers":["None of the above"$note]}}}""" + for (result in listOf(json(source), JsonPrimitive(source))) { + val call = tool("request_user_input", input, result = result) + val details = questionToolDetails(call) + assertTrue(details.hasAnswers) + assertFalse(details.showResult) + if (flag.isEmpty()) assertEquals(listOf("None of the above"), details.questions[0].otherAnswers) + else { + assertEquals(listOf(false, true), details.questions[0].options.map { it.selected }) + assertTrue(details.questions[0].options[1].isOther) + assertEquals(emptyList(), details.questions[0].otherAnswers) + } + assertEquals(if (note.isEmpty()) null else "自定义\n说明", details.questions[0].note) + assertEquals(json(input), call.input) + } + } + } + val input = """{"questions":[{"id":"choice","isOther":true,"options":[{"label":"A"}]}]}""" + val call = tool("request_user_input", input, """{"choice":{"answers":["None of the above","user_note: live"]}}""", + json("""{"answers":{"choice":{"answers":["A","user_note: stale"]}}}""")) + assertEquals(listOf(false, true), questionToolDetails(call).questions[0].options.map { it.selected }) + assertEquals("live", questionToolDetails(call).questions[0].note) + assertFalse(questionToolDetails(tool("request_user_input", input, result = json("""{"status":"resolved"}"""))).hasAnswers) + } + private val askInput = """{"questions":[{"header":"Storage","question":"Choose **storage**","multiSelect":true,"options":[{"label":"SQLite","description":"Local file"},{"label":"Postgres"}]},{"question":"Anything else?","options":[]}]}""" private val requestInput = """{"questions":[{"id":"target","question":"Deploy where?","multiple":true,"options":[{"label":"staging","description":"Safe"},{"label":"production"}]},{"id":"comment","question":"Comment","prefill":"Not an answer"}]}""" private fun json(source: String) = HapiJson.parseToJsonElement(source) diff --git a/android/app/src/test/kotlin/app/hapi/companion/feature/chat/permissions/RequestUserInputTest.kt b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/permissions/RequestUserInputTest.kt new file mode 100644 index 00000000..53a0d202 --- /dev/null +++ b/android/app/src/test/kotlin/app/hapi/companion/feature/chat/permissions/RequestUserInputTest.kt @@ -0,0 +1,48 @@ +package app.hapi.companion.feature.chat.permissions + +import app.hapi.protocol.wire.HapiJson +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class RequestUserInputTest { + private fun question(fields: String): RequestUserInputQuestion = parseRequestUserInputQuestions( + HapiJson.parseToJsonElement("""{"questions":[{"id":"choice",$fields}]}"""), + ).single() + + @Test fun otherRequiresExplicitBooleanAndNonemptyOptions() { + for (flag in listOf("true", "false", "null", "\"true\"")) { + val question = question(""""isOther":$flag,"options":[{"label":"A"}]""") + assertEquals(flag == "true", question.isOther) + assertEquals(listOf("A"), question.options.map { it.label }) + assertEquals(if (flag == "true") listOf("A", "None of the above") else listOf("A"), question.answerOptions.map { it.label }) + assertFalse(question.isOtherOption(0)) + assertEquals(flag == "true", question.isOtherOption(1)) + } + val missing = question(""""options":[{"label":"A"}]""") + assertFalse(missing.isOther) + assertEquals(missing.options, missing.answerOptions) + val text = question(""""isOther":true,"options":[]""") + assertTrue(text.answerOptions.isEmpty()) + assertFalse(text.isOtherOption(0)) + assertFalse(isRequestUserInputAnswered(text, emptyList(), "")) + } + + @Test fun otherIsExclusiveAndNotesAreOptional() { + val question = question(""""isOther":true,"multiple":true,"options":[{"label":"A"},{"label":"B"}]""") + val other = selectRequestUserInputOption(question, setOf("A", "B"), 2) + assertEquals(setOf("None of the above"), other) + assertTrue(isRequestUserInputAnswered(question, other.toList(), "")) + assertFalse(isRequestUserInputAnswered(question, emptyList(), "custom answer")) + var selected = selectRequestUserInputOption(question, other, 0) + assertEquals(setOf("A"), selected) + selected = selectRequestUserInputOption(question, selected, 1) + assertEquals(setOf("A", "B"), selected) + assertEquals(setOf("B"), selectRequestUserInputOption(question, selected, 0)) + for (note in listOf("", " \n ", " 自定义\n说明 ")) { + assertEquals(listOf("None of the above") + (if (note.isBlank()) emptyList() else listOf("user_note: ${note.trim()}")), + requestUserInputAnswerValues(other.toList(), note)) + } + } +} diff --git a/cli/src/codex/shared/permissions.test.ts b/cli/src/codex/shared/permissions.test.ts index aba1d5a3..69e93e33 100644 --- a/cli/src/codex/shared/permissions.test.ts +++ b/cli/src/codex/shared/permissions.test.ts @@ -18,6 +18,22 @@ function fixture() { return { permissions, request, respond, handlers, send, state: () => state }; } describe('shared request arbitration', () => { + it.each([{ notes: [] }, { notes: ['user_note: custom answer'] }])('preserves isOther and canonical other answers (%j) without claiming a winner', async ({ notes }) => { + const f = fixture(); + const request = { ...f.request, params: { ...f.request.params, isBlocking: true, + questions: f.request.params.questions.map(question => ({ ...question, isOther: true })) } }; + f.permissions.receive(request); + const [id, pending] = Object.entries(f.state().requests ?? {})[0]!; + expect(pending.arguments).toEqual(request.params); + const answers = { choice: { answers: ['None of the above', ...notes] } }; + await f.handlers.get('permission')!({ id, approved: true, answers }); + await vi.waitFor(() => expect(f.respond).toHaveBeenCalledWith(1, { answers })); + expect(f.state().completedRequests).toBeUndefined(); + f.permissions.resolved('thread', request.id); + expect(f.state().completedRequests?.[id]).toMatchObject({ status: 'resolved' }); + expect(f.state().completedRequests?.[id]).not.toHaveProperty('answers'); + }); + it('withdrawal on disconnect is canceled, not a claim that a native answer won', () => { const f = fixture(); f.permissions.receive(f.request); f.permissions.close(); expect(Object.values(f.state().completedRequests ?? {})[0]?.status).toBe('canceled'); diff --git a/cli/src/codex/utils/appServerPermissionAdapter.test.ts b/cli/src/codex/utils/appServerPermissionAdapter.test.ts index 125b88b7..1044b5f2 100644 --- a/cli/src/codex/utils/appServerPermissionAdapter.test.ts +++ b/cli/src/codex/utils/appServerPermissionAdapter.test.ts @@ -25,13 +25,14 @@ describe('registerAppServerPermissionHandlers', () => { expect(id).toBe('tool-123'); expect(input).toEqual({ itemId: 'tool-123', - questions: [{ id: 'approve_nav', question: 'Approve app tool call?' }] + isBlocking: true, + questions: [{ id: 'approve_nav', question: 'Approve app tool call?', isOther: true }] }); return { decision: 'accept', answers: { approve_nav: { - answers: ['Allow'] + answers: ['None of the above', 'user_note: custom answer'] } } }; @@ -48,12 +49,13 @@ describe('registerAppServerPermissionHandlers', () => { await expect(handler?.({ itemId: 'tool-123', - questions: [{ id: 'approve_nav', question: 'Approve app tool call?' }] + isBlocking: true, + questions: [{ id: 'approve_nav', question: 'Approve app tool call?', isOther: true }] })).resolves.toEqual({ decision: 'accept', answers: { approve_nav: { - answers: ['Allow'] + answers: ['None of the above', 'user_note: custom answer'] } } }); diff --git a/ios/Hapi/Features/Chat/Blocks/QuestionAnswerDraft.swift b/ios/Hapi/Features/Chat/Blocks/QuestionAnswerDraft.swift index 2535371f..c76c349b 100644 --- a/ios/Hapi/Features/Chat/Blocks/QuestionAnswerDraft.swift +++ b/ios/Hapi/Features/Chat/Blocks/QuestionAnswerDraft.swift @@ -33,10 +33,15 @@ struct QuestionAnswerForm: Equatable { var options: [AskOption] { switch self { case .ask(let question, _, _): question.options - case .request(let question): question.options + case .request(let question): question.answerOptions } } + func isOtherOption(_ index: Int) -> Bool { + if case .request(let question) = self { return question.isOtherOption(at: index) } + return false + } + var multiple: Bool { switch self { case .ask(let question, _, _): question.multiSelect @@ -158,8 +163,12 @@ struct QuestionAnswerDraft: Equatable { guard index == currentPage(in: form), form.fields.indices.contains(index), form.fields[index].options.indices.contains(option) else { return } let field = form.fields[index] - if field.multiple { - var selected = selections[index] ?? [] + if field.isOtherOption(option) { + selections[index] = [option] + expanded[index] = true + // Unlike an ordinary single selection, leave time to add optional notes. + } else if field.multiple { + var selected = Set((selections[index] ?? []).filter { !field.isOtherOption($0) }) if selected.contains(option) { selected.remove(option) } else { selected.insert(option) } selections[index] = selected @@ -239,7 +248,9 @@ enum QuestionCardState: Equatable { } func questionAnswerSummary(_ question: QuestionDetail) -> String? { - let choices = question.options.filter(\.selected).map { QuestionOptionTitle($0.label).text } + let choices = question.options.filter(\.selected).map { + $0.isOther ? String(localized: "None of the above") : QuestionOptionTitle($0.label).text + } let values = choices + question.otherAnswers + (question.note.map { [$0.isEmpty ? String(localized: "(empty)") : $0] } ?? []) guard !values.isEmpty else { return nil } return chatTruncate(values.joined(separator: " · ").split(whereSeparator: \.isWhitespace).joined(separator: " "), 240) diff --git a/ios/Hapi/Features/Chat/Blocks/QuestionDetailsView.swift b/ios/Hapi/Features/Chat/Blocks/QuestionDetailsView.swift index ea35c622..f9f3a693 100644 --- a/ios/Hapi/Features/Chat/Blocks/QuestionDetailsView.swift +++ b/ios/Hapi/Features/Chat/Blocks/QuestionDetailsView.swift @@ -23,7 +23,8 @@ struct QuestionDetailsView: View { } ForEach(Array(question.options.enumerated()), id: \.offset) { optionIndex, option in QuestionAnswerCard( - text: option.label, description: option.description, markdown: true, + text: option.isOther ? String(localized: "None of the above") : option.label, + description: option.description, markdown: true, selected: option.selected, multiple: question.multiple, showControl: question.hasAnswers ) .accessibilityIdentifier("question-\(index)-option-\(optionIndex)") diff --git a/ios/Hapi/Features/Chat/Blocks/QuestionToolCard.swift b/ios/Hapi/Features/Chat/Blocks/QuestionToolCard.swift index a67bd8c8..d8fbc5a9 100644 --- a/ios/Hapi/Features/Chat/Blocks/QuestionToolCard.swift +++ b/ios/Hapi/Features/Chat/Blocks/QuestionToolCard.swift @@ -184,11 +184,13 @@ struct QuestionAnswerFormView: View { QuestionSelectionRow( option: field.options[optionIndex], selected: draft.selections[index]?.contains(optionIndex) == true, - multiple: field.multiple + multiple: field.multiple, + isOther: field.isOtherOption(optionIndex) ) { guard enabled, !submitting else { return } textFocused = false draft.select(optionIndex, at: index, in: form) + if field.isOtherOption(optionIndex) { textFocused = true } } .disabled(!enabled || submitting) .accessibilityIdentifier("question-\(index)-choice-\(optionIndex)") @@ -251,15 +253,17 @@ struct QuestionAnswerFormView: View { .accessibilityIdentifier("question-previous") } if !form.fields[index].options.isEmpty { + let expanded = draft.showsText(at: index, in: form) Button { draft.toggleText(at: index, in: form) textFocused = draft.showsText(at: index, in: form) } label: { Label { - Text(verbatim: textTitle(index: index)) + Text(verbatim: expanded && !form.fields[index].allowsCustomAnswer + ? String(localized: "Hide note") : textTitle(index: index)) .fixedSize(horizontal: false, vertical: true) } icon: { - Image(systemName: draft.showsText(at: index, in: form) ? "chevron.up" : "plus") + Image(systemName: expanded ? "chevron.up" : "plus") } .font(typography.captionFont) .frame(minHeight: 44) @@ -304,6 +308,7 @@ private struct QuestionSelectionRow: View { let option: AskOption let selected: Bool let multiple: Bool + let isOther: Bool let action: () -> Void @Environment(\.hapiTheme) private var theme @Environment(\.hapiTypography) private var typography @@ -318,8 +323,8 @@ private struct QuestionSelectionRow: View { .padding(.top, 2) .accessibilityHidden(true) VStack(alignment: .leading, spacing: 4) { - QuestionOptionLabel(label: option.label) - if let description = option.description { + QuestionOptionLabel(label: isOther ? String(localized: "None of the above") : option.label) + if let description = isOther ? String(localized: "Optionally, add details in notes.") : option.description { // Inline markdown keeps the caption metrics instead of // promoting supporting text to full-size body paragraphs. Text(.init(description)) diff --git a/ios/Hapi/Features/Chat/Blocks/QuestionToolPresentation.swift b/ios/Hapi/Features/Chat/Blocks/QuestionToolPresentation.swift index 749a3b2f..5b56639e 100644 --- a/ios/Hapi/Features/Chat/Blocks/QuestionToolPresentation.swift +++ b/ios/Hapi/Features/Chat/Blocks/QuestionToolPresentation.swift @@ -24,6 +24,7 @@ struct QuestionOptionDetail: Equatable, Sendable { let label: String let description: String? let selected: Bool + var isOther = false } func isQuestionDetailsTool(_ name: String) -> Bool { @@ -101,13 +102,14 @@ private func requestQuestionDetails( _ questions: [RequestUserInputQuestion], answers: [String: [String]] ) -> [QuestionDetail] { questions.map { question in + let options = question.answerOptions var selected: Set = [] var other: [String] = [] var note: String? for value in answers[question.id] ?? [] { let trimmed = trimQuestionAnswer(value) // An actual option named "user_note: ..." is not metadata. - if let option = question.options.first(where: { $0.label == trimmed }) { + if let option = options.first(where: { $0.label == trimmed }) { selected.insert(option.label) } else if value.hasPrefix("user_note: ") { let text = String(value.dropFirst("user_note: ".count)) @@ -124,8 +126,9 @@ private func requestQuestionDetails( } return QuestionDetail( header: question.header, question: question.question, multiple: question.multiple, - options: question.options.map { - QuestionOptionDetail(label: $0.label, description: $0.description, selected: selected.contains($0.label)) + options: options.enumerated().map { index, option in + QuestionOptionDetail(label: option.label, description: option.description, + selected: selected.contains(option.label), isOther: question.isOtherOption(at: index)) }, otherAnswers: other, note: note ) diff --git a/ios/Hapi/Resources/Localizable.xcstrings b/ios/Hapi/Resources/Localizable.xcstrings index 2feb700a..a28a9daf 100644 --- a/ios/Hapi/Resources/Localizable.xcstrings +++ b/ios/Hapi/Resources/Localizable.xcstrings @@ -2435,6 +2435,17 @@ } } }, + "Hide note" : { + "extractionState" : "manual", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "收起备注" + } + } + } + }, "Hub" : { "extractionState" : "manual", "localizations" : { @@ -3216,6 +3227,17 @@ } } }, + "None of the above" : { + "extractionState" : "manual", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "以上都不是" + } + } + } + }, "Not delivered" : { "extractionState" : "manual", "localizations" : { @@ -3392,6 +3414,17 @@ } } }, + "Optionally, add details in notes." : { + "extractionState" : "manual", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "可在备注中补充说明（可选）。" + } + } + } + }, "Options" : { "extractionState" : "manual", "localizations" : { diff --git a/ios/HapiTests/QuestionAnswerDraftTests.swift b/ios/HapiTests/QuestionAnswerDraftTests.swift index 8c73f35f..c40afa93 100644 --- a/ios/HapiTests/QuestionAnswerDraftTests.swift +++ b/ios/HapiTests/QuestionAnswerDraftTests.swift @@ -12,6 +12,54 @@ final class QuestionAnswerDraftTests: XCTestCase { private let choices = #"{"questions":[{"id":"first","header":"First","question":"Choose **one**","options":[{"label":"A (Recommended)"},{"label":"B"}]},{"id":"last","header":"Last","question":"Choose again","options":[{"label":"C"},{"label":"D"}]}]}"# + func testOtherOpensNotesWithoutAdvancingAndAllowsEmptyNotes() throws { + let input = choices.replacingOccurrences(of: "\"options\"", with: "\"isOther\":true,\"options\"") + let form = try QuestionAnswerForm(tool: tool(input)) + for note in ["", " \n ", " 自定义\n说明 "] { + var draft = QuestionAnswerDraft() + draft.select(0, at: 0, in: form) + XCTAssertEqual(draft.page, 1, "Ordinary choices still advance") + draft.previous(in: form) + draft.select(2, at: 0, in: form) + XCTAssertEqual(draft.page, 0, "Other must leave time to add a note") + XCTAssertEqual(draft.selections[0], [2]) + XCTAssertTrue(draft.showsText(at: 0, in: form)) + XCTAssertTrue(draft.isAnswered(at: 0, in: form), "Notes are optional") + draft.setText(note, at: 0, in: form) + let refreshed = try QuestionAnswerForm(tool: tool(input)) + XCTAssertEqual(draft.text(at: 0, in: refreshed), note) + draft.next(in: form) + XCTAssertEqual(draft.text(at: 1, in: form), "") + draft.select(0, at: 1, in: form) + let trimmed = note.trimmingCharacters(in: .whitespacesAndNewlines) + XCTAssertEqual(draft.submission(in: form), .nestedAnswers([ + "first": ["None of the above"] + (trimmed.isEmpty ? [] : ["user_note: \(trimmed)"]), "last": ["C"], + ])) + draft.previous(in: form) + XCTAssertEqual(draft.selections[0], [2]) + draft.select(1, at: 0, in: form) + XCTAssertEqual(draft.selections[0], [1]) + XCTAssertEqual(draft.text(at: 0, in: form), note, "Switching choices preserves notes") + } + } + + func testOtherIsExclusiveWithMultipleChoicesAndReopensCollapsedNotes() throws { + let form = try QuestionAnswerForm(tool: tool(#"{"questions":[{"id":"choice","isOther":true,"multiple":true,"options":[{"label":"A"},{"label":"B"}]}]}"#)) + var draft = QuestionAnswerDraft() + draft.select(0, at: 0, in: form) + draft.select(1, at: 0, in: form) + draft.select(2, at: 0, in: form) + XCTAssertEqual(draft.selections[0], [2]) + draft.setText("Keep", at: 0, in: form) + draft.toggleText(at: 0, in: form) + draft.select(2, at: 0, in: form) + XCTAssertTrue(draft.showsText(at: 0, in: form)) + draft.select(1, at: 0, in: form) + draft.select(0, at: 0, in: form) + XCTAssertEqual(draft.selections[0], [0, 1]) + XCTAssertEqual(draft.submission(in: form), .nestedAnswers(["choice": ["A", "B", "user_note: Keep"]])) + } + func testSingleSelectAdvancesOnlyOnExplicitSelectionAndLastRequiresSubmission() throws { let call = try tool(choices) let form = QuestionAnswerForm(tool: call) diff --git a/ios/HapiTests/QuestionCardPresentationTests.swift b/ios/HapiTests/QuestionCardPresentationTests.swift index 739ce6e1..da418c79 100644 --- a/ios/HapiTests/QuestionCardPresentationTests.swift +++ b/ios/HapiTests/QuestionCardPresentationTests.swift @@ -94,6 +94,40 @@ final class QuestionCardPresentationTests: XCTestCase { permission: ToolPermission(id: "reply", status: .pending)) } + func testOtherNotesStayOnTheCurrentQuestionAndRenderRecordedAnswers() async throws { + let source = #"{"questions":[{"id":"choice","header":"选择","question":"请选择一项","isOther":true,"options":[{"label":"Alpha"},{"label":"Beta"}]},{"id":"last","question":"Next question","options":[{"label":"Finish"}]}]}"# + for (name, theme, size, width) in [("light", HapiTheme.light, DynamicTypeSize.large, CGFloat(390)), + ("large-type", .dark, .accessibility3, 320)] { + let driver = Driver(tool: try makeTool(source)) + let (window, _) = try host(Specimen(driver: driver, theme: theme, typeSize: size), width: width) + defer { window.isHidden = true } + try await settle { driver.height > 100 } + let initialHeight = driver.height + let form = QuestionAnswerForm(tool: driver.tool) + driver.draft.select(2, at: 0, in: form) + XCTAssertEqual(driver.draft.page, 0) + XCTAssertTrue(driver.draft.isAnswered(at: 0, in: form)) + try await settle { driver.height > initialHeight } + driver.draft.setText("自定义\n说明", at: 0, in: form) + driver.tool.description = "Unrelated SSE update" + try await Task.sleep(for: .milliseconds(150)) + XCTAssertEqual(driver.draft.selections[0], [2]) + XCTAssertEqual(driver.draft.text(at: 0, in: form), "自定义\n说明") + XCTAssertFalse(questionToolDetails(driver.tool).hasAnswers) + try capture(window, name: "other-notes-\(name)") + driver.tool.state = .completed + driver.tool.permission = ToolPermission(id: "reply", status: .resolved) + XCTAssertFalse(questionToolDetails(driver.tool).hasAnswers, "Resolution does not prove which answer won") + driver.tool.result = .object(["answers": .object(["choice": .object([ + "answers": .array([.string("None of the above"), .string("user_note: 自定义\n说明")]), + ])])]) + try await settle { driver.height < initialHeight } + let details = questionToolDetails(driver.tool) + XCTAssertEqual(details.questions[0].options.map(\.selected), [false, false, true]) + XCTAssertEqual(details.questions[0].note, "自定义\n说明") + } + } + func testQuestionCardsFitThemesAndTypeSizesAndCollapseOnlyWithRecordedAnswers() async throws { for (name, theme, size, width) in [ ("light", HapiTheme.light, DynamicTypeSize.large, CGFloat(390)), diff --git a/ios/HapiTests/QuestionToolPresentationTests.swift b/ios/HapiTests/QuestionToolPresentationTests.swift index 17811522..82031bec 100644 --- a/ios/HapiTests/QuestionToolPresentationTests.swift +++ b/ios/HapiTests/QuestionToolPresentationTests.swift @@ -3,6 +3,38 @@ import XCTest import HapiProtocol final class QuestionToolPresentationTests: XCTestCase { + func testOtherAndNotesShowFromRecordedAnswersWithoutChangingRawInput() throws { + for flag in [#", "isOther":true"#, ""] { + let input = "{\"questions\":[{\"id\":\"choice\"\(flag),\"options\":[{\"label\":\"A\"}]}]}" + for note in ["", #", "user_note: 自定义\n说明""#] { + let source = "{\"answers\":{\"choice\":{\"answers\":[\"None of the above\"\(note)]}}}" + for result in [try json(source), JSONValue.string(source)] { + let call = try tool("request_user_input", input: input, result: result) + let details = questionToolDetails(call) + XCTAssertTrue(details.hasAnswers) + XCTAssertFalse(details.showResult) + if flag.isEmpty { + XCTAssertEqual(details.questions[0].otherAnswers, ["None of the above"]) + } else { + XCTAssertEqual(details.questions[0].options.map(\.selected), [false, true]) + XCTAssertTrue(details.questions[0].options[1].isOther) + XCTAssertEqual(details.questions[0].otherAnswers, []) + } + XCTAssertEqual(details.questions[0].note, note.isEmpty ? nil : "自定义\n说明") + XCTAssertEqual(call.input, try json(input)) + } + } + } + let input = #"{"questions":[{"id":"choice","isOther":true,"options":[{"label":"A"}]}]}"# + let call = try tool("request_user_input", input: input, + answers: #"{"choice":{"answers":["None of the above","user_note: live"]}}"#, + result: json(#"{"answers":{"choice":{"answers":["A","user_note: stale"]}}}"#)) + XCTAssertEqual(questionToolDetails(call).questions[0].options.map(\.selected), [false, true]) + XCTAssertEqual(questionToolDetails(call).questions[0].note, "live") + let resolved = try tool("request_user_input", input: input, result: json(#"{"status":"resolved"}"#)) + XCTAssertFalse(questionToolDetails(resolved).hasAnswers) + } + private let askInput = #"{"questions":[{"header":"Storage","question":"Choose **storage**","multiSelect":true,"options":[{"label":"SQLite","description":"Local file"},{"label":"Postgres"}]},{"question":"Anything else?","options":[]}]}"# private let requestInput = #"{"questions":[{"id":"target","question":"Deploy where?","multiple":true,"options":[{"label":"staging","description":"Safe"},{"label":"production"}]},{"id":"comment","question":"Comment","prefill":"Not an answer"}]}"# diff --git a/ios/Packages/HapiKit/Sources/HapiClient/Chat/PermissionInputs.swift b/ios/Packages/HapiKit/Sources/HapiClient/Chat/PermissionInputs.swift index a3229584..9374715f 100644 --- a/ios/Packages/HapiKit/Sources/HapiClient/Chat/PermissionInputs.swift +++ b/ios/Packages/HapiKit/Sources/HapiClient/Chat/PermissionInputs.swift @@ -122,6 +122,9 @@ public func parseAskUserQuestions(_ input: JSONValue?, cursorDialect: Bool) -> [ return questions } +/// Codex wire value. Presentation may localize it; answer serialization must not. +public let requestUserInputOtherAnswer = "None of the above" + public struct RequestUserInputQuestion: Equatable, Sendable { public let id: String public let header: String? @@ -132,6 +135,16 @@ public struct RequestUserInputQuestion: Equatable, Sendable { public let placeholder: String? public let prefill: String? public let inputType: String? + public let isOther: Bool + + public var answerOptions: [AskOption] { + guard isOther, !options.isEmpty else { return options } + return options + [AskOption(id: nil, label: requestUserInputOtherAnswer, description: nil)] + } + + public func isOtherOption(at index: Int) -> Bool { + isOther && !options.isEmpty && index == options.count + } public init( id: String, @@ -142,7 +155,8 @@ public struct RequestUserInputQuestion: Equatable, Sendable { placeholder: String?, prefill: String?, inputType: String? = nil, - header: String? = nil + header: String? = nil, + isOther: Bool = false ) { self.id = id self.header = header @@ -153,6 +167,7 @@ public struct RequestUserInputQuestion: Equatable, Sendable { self.placeholder = placeholder self.prefill = prefill self.inputType = inputType + self.isOther = isOther } } @@ -190,7 +205,8 @@ public func parseRequestUserInputQuestions(_ input: JSONValue?) -> [RequestUserI placeholder: object["placeholder"]?.stringValue, prefill: object["prefill"]?.stringValue, inputType: object["inputType"]?.stringValue == "editor" ? "editor" : nil, - header: trimmedString(object["header"]).flatMap { $0.isEmpty ? nil : $0 } + header: trimmedString(object["header"]).flatMap { $0.isEmpty ? nil : $0 }, + isOther: object["isOther"]?.boolValue == true )) } return questions diff --git a/ios/Packages/HapiKit/Tests/HapiClientTests/Chat/RequestUserInputTests.swift b/ios/Packages/HapiKit/Tests/HapiClientTests/Chat/RequestUserInputTests.swift new file mode 100644 index 00000000..cba227f0 --- /dev/null +++ b/ios/Packages/HapiKit/Tests/HapiClientTests/Chat/RequestUserInputTests.swift @@ -0,0 +1,40 @@ +import Foundation +import HapiClient +import HapiProtocol +import Testing + +struct RequestUserInputTests { + private func question(_ fields: String) throws -> RequestUserInputQuestion { + let input = try JSONDecoder().decode(JSONValue.self, from: Data("{\"questions\":[{\"id\":\"choice\",\(fields)}]}".utf8)) + return try #require(parseRequestUserInputQuestions(input).first) + } + + @Test func otherRequiresExplicitBooleanAndNonemptyOptions() throws { + for flag in ["true", "false", "null", "\"true\""] { + let question = try question("\"isOther\":\(flag),\"options\":[{\"label\":\"A\"}]") + #expect(question.isOther == (flag == "true")) + #expect(question.options.map(\.label) == ["A"]) + #expect(question.answerOptions.map(\.label) == (flag == "true" ? ["A", "None of the above"] : ["A"])) + #expect(!question.isOtherOption(at: 0)) + #expect(question.isOtherOption(at: 1) == (flag == "true")) + } + let missing = try question(#""options":[{"label":"A"}]"#) + #expect(!missing.isOther) + #expect(missing.answerOptions == missing.options) + let text = try question(#""isOther":true,"options":[]"#) + #expect(text.answerOptions.isEmpty) + #expect(!text.isOtherOption(at: 0)) + #expect(!isRequestUserInputAnswered(text, selected: [], note: "")) + } + + @Test func optionalNoteKeepsCanonicalOtherValue() throws { + let question = try question(#""isOther":true,"options":[{"label":"A"}]"#) + #expect(isRequestUserInputAnswered(question, selected: [requestUserInputOtherAnswer], note: "")) + #expect(!isRequestUserInputAnswered(question, selected: [], note: "custom answer")) + for note in ["", " \n ", " 自定义\n说明 "] { + let trimmed = note.trimmingCharacters(in: .whitespacesAndNewlines) + #expect(requestUserInputAnswerValues(selected: [requestUserInputOtherAnswer], note: note) + == ["None of the above"] + (trimmed.isEmpty ? [] : ["user_note: \(trimmed)"])) + } + } +} diff --git a/ios/README.md b/ios/README.md index 4b1f9770..0b2afef6 100644 --- a/ios/README.md +++ b/ios/README.md @@ -351,6 +351,11 @@ question**. **Previous question** retains all choices and notes; the last step always requires **Submit answer**. Recommended labels are display-only badges, never default selections or rewritten wire values. Other-answer/note fields expand on demand; text-only questions and prefilled drafts show them immediately. +Codex choice questions with `isOther: true` also offer **None of the above**. +Selecting it stays on the current question and focuses optional notes; empty +notes are valid. Its wire value remains `None of the above` in every language, +and recorded answers/notes appear in summaries and details. Requests without +`isOther` (including Pi and MCP forms) keep their existing choices. All form state survives transcript-cell recycling for the retained request. Successful records collapse to answer summaries; missing recorded answers are shown as handled, not inferred from local drafts. Ordinary approvals retain their approval footer. diff --git a/shared/fixtures/chat/permission-request-user-input-other-pending.json b/shared/fixtures/chat/permission-request-user-input-other-pending.json new file mode 100644 index 00000000..10412805 --- /dev/null +++ b/shared/fixtures/chat/permission-request-user-input-other-pending.json @@ -0,0 +1,185 @@ +{ + "description": "Codex isOther survives the pipeline without modifying the original options or inventing an answer.", + "expected": { + "blocks": [ + { + "createdAt": 1755000000000, + "id": "shared-user", + "kind": "user-text", + "localId": null, + "text": "Choose an option." + }, + { + "createdAt": 1755000001000, + "id": "other-call", + "kind": "tool-call", + "localId": null, + "tool": { + "id": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "permission": { + "status": "pending" + }, + "state": "running" + } + } + ], + "hasReadyEvent": false, + "latestUsage": null, + "visibleBlocks": [ + { + "createdAt": 1755000000000, + "id": "shared-user", + "kind": "user-text", + "localId": null, + "text": "Choose an option." + }, + { + "createdAt": 1755000001000, + "id": "other-call", + "kind": "tool-call", + "localId": null, + "tool": { + "id": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "permission": { + "status": "pending" + }, + "state": "running" + } + } + ] + }, + "fixtureVersion": 1, + "input": { + "agentState": { + "requests": { + "other-request": { + "arguments": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "createdAt": 1755000001100, + "tool": "request_user_input", + "toolCallId": "other-call" + } + } + }, + "messages": [ + { + "content": { + "content": { + "text": "Choose an option.", + "type": "text" + }, + "role": "user" + }, + "createdAt": 1755000000000, + "id": "shared-user", + "localId": null, + "seq": 1 + }, + { + "content": { + "content": { + "data": { + "callId": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "type": "tool-call" + }, + "type": "codex" + }, + "role": "agent" + }, + "createdAt": 1755000001000, + "id": "other-question", + "localId": null, + "seq": 2 + } + ], + "options": { + "hasMoreMessages": false + } + }, + "name": "permission-request-user-input-other-pending" +} diff --git a/shared/fixtures/chat/permission-request-user-input-other-with-note.json b/shared/fixtures/chat/permission-request-user-input-other-with-note.json new file mode 100644 index 00000000..1f1da188 --- /dev/null +++ b/shared/fixtures/chat/permission-request-user-input-other-with-note.json @@ -0,0 +1,259 @@ +{ + "description": "Authoritatively recorded other answer, with an optional note; both result history and permission answers preserve canonical wire values. Not a locally inferred shared-session winner.", + "expected": { + "blocks": [ + { + "createdAt": 1755000000000, + "id": "shared-user", + "kind": "user-text", + "localId": null, + "text": "Choose an option." + }, + { + "createdAt": 1755000001000, + "id": "other-call", + "kind": "tool-call", + "localId": null, + "tool": { + "id": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "permission": { + "answers": { + "choice": { + "answers": [ + "None of the above", + "user_note: 自定义\n说明" + ] + } + }, + "status": "approved" + }, + "result": { + "answers": { + "choice": { + "answers": [ + "None of the above", + "user_note: 自定义\n说明" + ] + } + } + }, + "state": "completed" + } + } + ], + "hasReadyEvent": false, + "latestUsage": null, + "visibleBlocks": [ + { + "createdAt": 1755000000000, + "id": "shared-user", + "kind": "user-text", + "localId": null, + "text": "Choose an option." + }, + { + "createdAt": 1755000001000, + "id": "other-call", + "kind": "tool-call", + "localId": null, + "tool": { + "id": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "permission": { + "answers": { + "choice": { + "answers": [ + "None of the above", + "user_note: 自定义\n说明" + ] + } + }, + "status": "approved" + }, + "result": { + "answers": { + "choice": { + "answers": [ + "None of the above", + "user_note: 自定义\n说明" + ] + } + } + }, + "state": "completed" + } + } + ] + }, + "fixtureVersion": 1, + "input": { + "agentState": { + "completedRequests": { + "other-request": { + "answers": { + "choice": { + "answers": [ + "None of the above", + "user_note: 自定义\n说明" + ] + } + }, + "arguments": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "completedAt": 1755000005000, + "createdAt": 1755000001100, + "status": "approved", + "tool": "request_user_input", + "toolCallId": "other-call" + } + }, + "requests": {} + }, + "messages": [ + { + "content": { + "content": { + "text": "Choose an option.", + "type": "text" + }, + "role": "user" + }, + "createdAt": 1755000000000, + "id": "shared-user", + "localId": null, + "seq": 1 + }, + { + "content": { + "content": { + "data": { + "callId": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "type": "tool-call" + }, + "type": "codex" + }, + "role": "agent" + }, + "createdAt": 1755000001000, + "id": "other-question", + "localId": null, + "seq": 2 + }, + { + "content": { + "content": { + "data": { + "callId": "other-call", + "is_error": false, + "output": { + "answers": { + "choice": { + "answers": [ + "None of the above", + "user_note: 自定义\n说明" + ] + } + } + }, + "type": "tool-call-result" + }, + "type": "codex" + }, + "role": "agent" + }, + "createdAt": 1755000005000, + "id": "other-result", + "localId": null, + "seq": 3 + } + ], + "options": { + "hasMoreMessages": false + } + }, + "name": "permission-request-user-input-other-with-note" +} diff --git a/shared/fixtures/chat/permission-request-user-input-other-without-note.json b/shared/fixtures/chat/permission-request-user-input-other-without-note.json new file mode 100644 index 00000000..27192f53 --- /dev/null +++ b/shared/fixtures/chat/permission-request-user-input-other-without-note.json @@ -0,0 +1,253 @@ +{ + "description": "Authoritatively recorded other answer, with an optional note; both result history and permission answers preserve canonical wire values. Not a locally inferred shared-session winner.", + "expected": { + "blocks": [ + { + "createdAt": 1755000000000, + "id": "shared-user", + "kind": "user-text", + "localId": null, + "text": "Choose an option." + }, + { + "createdAt": 1755000001000, + "id": "other-call", + "kind": "tool-call", + "localId": null, + "tool": { + "id": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "permission": { + "answers": { + "choice": { + "answers": [ + "None of the above" + ] + } + }, + "status": "approved" + }, + "result": { + "answers": { + "choice": { + "answers": [ + "None of the above" + ] + } + } + }, + "state": "completed" + } + } + ], + "hasReadyEvent": false, + "latestUsage": null, + "visibleBlocks": [ + { + "createdAt": 1755000000000, + "id": "shared-user", + "kind": "user-text", + "localId": null, + "text": "Choose an option." + }, + { + "createdAt": 1755000001000, + "id": "other-call", + "kind": "tool-call", + "localId": null, + "tool": { + "id": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "permission": { + "answers": { + "choice": { + "answers": [ + "None of the above" + ] + } + }, + "status": "approved" + }, + "result": { + "answers": { + "choice": { + "answers": [ + "None of the above" + ] + } + } + }, + "state": "completed" + } + } + ] + }, + "fixtureVersion": 1, + "input": { + "agentState": { + "completedRequests": { + "other-request": { + "answers": { + "choice": { + "answers": [ + "None of the above" + ] + } + }, + "arguments": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "completedAt": 1755000005000, + "createdAt": 1755000001100, + "status": "approved", + "tool": "request_user_input", + "toolCallId": "other-call" + } + }, + "requests": {} + }, + "messages": [ + { + "content": { + "content": { + "text": "Choose an option.", + "type": "text" + }, + "role": "user" + }, + "createdAt": 1755000000000, + "id": "shared-user", + "localId": null, + "seq": 1 + }, + { + "content": { + "content": { + "data": { + "callId": "other-call", + "input": { + "isBlocking": true, + "questions": [ + { + "header": "Choice", + "id": "choice", + "isOther": true, + "options": [ + { + "description": "First choice", + "label": "A" + }, + { + "description": "Second choice", + "label": "B" + } + ], + "question": "Which option?" + } + ] + }, + "name": "request_user_input", + "type": "tool-call" + }, + "type": "codex" + }, + "role": "agent" + }, + "createdAt": 1755000001000, + "id": "other-question", + "localId": null, + "seq": 2 + }, + { + "content": { + "content": { + "data": { + "callId": "other-call", + "is_error": false, + "output": { + "answers": { + "choice": { + "answers": [ + "None of the above" + ] + } + } + }, + "type": "tool-call-result" + }, + "type": "codex" + }, + "role": "agent" + }, + "createdAt": 1755000005000, + "id": "other-result", + "localId": null, + "seq": 3 + } + ], + "options": { + "hasMoreMessages": false + } + }, + "name": "permission-request-user-input-other-without-note" +} diff --git a/web/README.md b/web/README.md index d34742cc..78c6fca2 100644 --- a/web/README.md +++ b/web/README.md @@ -7,6 +7,10 @@ React Mini App / PWA for monitoring and controlling hapi sessions. - Session list with status, pending approvals, todos, and summaries. - Chat view with streaming updates and message sending. - Permission approval and denial workflows. +- Codex `request_user_input` choices honor `isOther`: **None of the above** + focuses optional notes, supports empty notes, and preserves the canonical + wire value across languages. Recorded other answers and notes remain visible + in live and historical cards; Pi/MCP forms without the flag are unchanged. - Permission mode and model selection. - Machine list and remote session spawn. - File browser and git status/diff views. diff --git a/web/scripts/fixtures/cases/permissions.ts b/web/scripts/fixtures/cases/permissions.ts index 30218279..4d8349fe 100644 --- a/web/scripts/fixtures/cases/permissions.ts +++ b/web/scripts/fixtures/cases/permissions.ts @@ -45,6 +45,23 @@ const sharedQuestionResult = (status: 'resolved' | 'canceled') => wireMessage({ } }) +const otherQuestionInput = { + isBlocking: true, + questions: [{ id: 'choice', header: 'Choice', question: 'Which option?', isOther: true, options: [ + { label: 'A', description: 'First choice' }, { label: 'B', description: 'Second choice' } + ] }] +} +const otherQuestionMessage = wireMessage({ + id: 'other-question', seq: 2, createdAt: T0 + 1_000, content: { + role: 'agent', content: { type: 'codex', data: { + type: 'tool-call', name: 'request_user_input', callId: 'other-call', input: otherQuestionInput + } } + } +}) +const otherQuestionRequest = { + tool: 'request_user_input', toolCallId: 'other-call', arguments: otherQuestionInput, createdAt: T0 + 1_100 +} + /** * Permission requests are NOT messages: they live in session.agentState * (requests → completedRequests). A pending request whose tool_use message is @@ -54,6 +71,29 @@ const sharedQuestionResult = (status: 'resolved' | 'canceled') => wireMessage({ * the oldest loaded message (web/src/chat/reducer.ts). */ export const permissionCases: FixtureCase[] = [ + { + name: 'permission-request-user-input-other-pending', + description: 'Codex isOther survives the pipeline without modifying the original options or inventing an answer.', + messages: [sharedQuestionMessages[0]!, otherQuestionMessage], + agentState: { requests: { 'other-request': otherQuestionRequest } } + }, + ...[false, true].map((withNote): FixtureCase => { + const answers = { choice: { answers: ['None of the above', ...(withNote ? ['user_note: 自定义\n说明'] : [])] } } + return { + name: `permission-request-user-input-other-${withNote ? 'with-note' : 'without-note'}`, + description: 'Authoritatively recorded other answer, with an optional note; both result history and permission answers preserve canonical wire values. Not a locally inferred shared-session winner.', + messages: [sharedQuestionMessages[0]!, otherQuestionMessage, wireMessage({ + id: 'other-result', seq: 3, createdAt: T0 + 5_000, content: { + role: 'agent', content: { type: 'codex', data: { + type: 'tool-call-result', callId: 'other-call', output: { answers }, is_error: false + } } + } + })], + agentState: { requests: {}, completedRequests: { 'other-request': { + ...otherQuestionRequest, completedAt: T0 + 5_000, status: 'approved', answers + } } } + } + }), { name: 'permission-shared-pending', description: 'requestUserInput without a native transcript item: CLI persists one question, permission state attaches without duplicating it.', diff --git a/web/src/components/ToolCard/RequestUserInputFooter.test.tsx b/web/src/components/ToolCard/RequestUserInputFooter.test.tsx index 7dc9169a..a59fcc79 100644 --- a/web/src/components/ToolCard/RequestUserInputFooter.test.tsx +++ b/web/src/components/ToolCard/RequestUserInputFooter.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { describe, expect, it, vi } from 'vitest' import type { ApiClient } from '@/api/client' import type { ChatToolCall } from '@/chat/types' @@ -13,11 +13,22 @@ vi.mock('@/hooks/usePlatform', () => ({ })) vi.mock('@/lib/use-translation', () => ({ - useTranslation: () => ({ t: (key: string) => key }) + useTranslation: () => ({ t: (key: string) => key === 'tool.requestUserInput.otherLabel' ? '以上都不是' : key }) })) import { RequestUserInputFooter } from './RequestUserInputFooter' +const choice = { id: 'choice', question: 'Choose', isOther: true, options: [{ label: 'Alpha' }, { label: 'Beta' }] } + +function renderFooter(input: unknown = { questions: [choice] }) { + const approvePermission = vi.fn().mockResolvedValue(undefined) + const props = { + api: { approvePermission } as unknown as ApiClient, + sessionId: 'session-1', tool: makeTool(input), disabled: false, onDone: vi.fn() + } + return { ...render(), props, approvePermission } +} + function makeTool(input: unknown): ChatToolCall { return { id: 'request-1', @@ -38,6 +49,71 @@ function makeTool(input: unknown): ChatToolCall { } describe('RequestUserInputFooter', () => { + it.each(['', ' \n ', ' 自定义\n说明 '])('focuses notes without submitting and posts canonical values (%j)', async (note) => { + const { approvePermission } = renderFooter() + fireEvent.click(screen.getByRole('button', { name: /Alpha/ })) + fireEvent.click(screen.getByRole('button', { name: /以上都不是/ })) + expect(screen.getByRole('textbox')).toHaveFocus() + expect(screen.getByRole('button', { name: /Alpha/ })).toHaveAttribute('aria-pressed', 'false') + expect(screen.getByRole('button', { name: /以上都不是/ })).toHaveAttribute('aria-pressed', 'true') + expect(approvePermission).not.toHaveBeenCalled() + fireEvent.change(screen.getByRole('textbox'), { target: { value: note } }) + fireEvent.click(screen.getByRole('button', { name: 'tool.submit' })) + await waitFor(() => expect(approvePermission).toHaveBeenCalledWith('session-1', 'permission-1', { + answers: { choice: { answers: ['None of the above', ...(note.trim() ? [`user_note: ${note.trim()}`] : [])] } } + })) + }) + + it('retains notes across choices, questions, and SSE rerenders without auto-advancing', async () => { + const { props, rerender, approvePermission } = renderFooter({ questions: [choice, { ...choice, id: 'second', question: 'Second' }] }) + fireEvent.click(screen.getByRole('button', { name: /以上都不是/ })) + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'keep this note' } }) + fireEvent.click(screen.getByRole('button', { name: /Alpha/ })) + expect(screen.getByRole('button', { name: /以上都不是/ })).toHaveAttribute('aria-pressed', 'false') + expect(screen.getByRole('textbox')).toHaveValue('keep this note') + fireEvent.click(screen.getByRole('button', { name: /以上都不是/ })) + expect(screen.getByText('Choose')).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: 'tool.next' })) + expect(screen.getByRole('textbox')).toHaveValue('') + fireEvent.click(screen.getByRole('button', { name: /Beta/ })) + fireEvent.click(screen.getByRole('button', { name: 'tool.prev' })) + rerender() + expect(screen.getByRole('textbox')).toHaveValue('keep this note') + expect(screen.getByRole('button', { name: /以上都不是/ })).toHaveAttribute('aria-pressed', 'true') + fireEvent.click(screen.getByRole('button', { name: 'tool.next' })) + fireEvent.click(screen.getByRole('button', { name: 'tool.submit' })) + await waitFor(() => expect(approvePermission).toHaveBeenCalledWith('session-1', 'permission-1', { + answers: { choice: { answers: ['None of the above', 'user_note: keep this note'] }, second: { answers: ['Beta'] } } + })) + }) + + it('does not interpret a note alone as None of the above', () => { + const { approvePermission } = renderFooter() + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'custom' } }) + fireEvent.click(screen.getByRole('button', { name: 'tool.submit' })) + expect(approvePermission).not.toHaveBeenCalled() + expect(screen.getByText('tool.selectOption')).toBeInTheDocument() + }) + + it.each([false, undefined])('does not add other to Pi or MCP choices (%s)', (isOther) => { + renderFooter({ questions: [{ ...choice, isOther }] }) + expect(screen.queryByRole('button', { name: /以上都不是/ })).not.toBeInTheDocument() + }) + + it('retains the answer on failure and removes controls when resolved elsewhere', async () => { + const { props, rerender, approvePermission } = renderFooter() + approvePermission.mockRejectedValueOnce(new Error('try again')) + fireEvent.click(screen.getByRole('button', { name: /以上都不是/ })) + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'draft' } }) + fireEvent.click(screen.getByRole('button', { name: 'tool.submit' })) + await screen.findByText('try again') + expect(screen.getByRole('textbox')).toHaveValue('draft') + expect(screen.getByRole('button', { name: /以上都不是/ })).toHaveAttribute('aria-pressed', 'true') + rerender() + expect(screen.queryByRole('button', { name: 'tool.submit' })).not.toBeInTheDocument() + expect(approvePermission).toHaveBeenCalledTimes(1) + }) + it('uses a Pi extension pure-text placeholder and prefill when initializing the request', () => { render( @@ -82,6 +86,7 @@ export function RequestUserInputFooter(props: { const [loading, setLoading] = useState(false) const [error, setError] = useState(null) + const noteRef = useRef(null) useEffect(() => { setStep(0) @@ -166,19 +171,17 @@ export function RequestUserInputFooter(props: { setStep((s) => Math.max(s - 1, 0)) } - const selectOption = (question: RequestUserInputQuestion, optionLabel: string) => { + const selectOption = (question: RequestUserInputQuestion, option: RequestUserInputOption) => { haptic.selection() + setError(null) setStateByQuestion((prev) => ({ ...prev, [question.id]: { ...prev[question.id], - selected: question.multiple - ? prev[question.id]?.selected.includes(optionLabel) - ? prev[question.id].selected.filter((value) => value !== optionLabel) - : [...(prev[question.id]?.selected ?? []), optionLabel] - : [optionLabel] + selected: selectRequestUserInputOption(question, prev[question.id]?.selected ?? [], option) } })) + if (option.isOther) noteRef.current?.focus() } const updateUserNote = (questionId: string, value: string) => { @@ -236,16 +239,16 @@ export function RequestUserInputFooter(props: { // Question with options <>
- {currentQuestion.options.map((opt, optIdx) => { + {requestUserInputOptions(currentQuestion).map((opt, optIdx) => { const isSelected = currentState?.selected.includes(opt.label) ?? false return ( selectOption(currentQuestion, opt.label)} + title={opt.isOther ? t('tool.requestUserInput.otherLabel') : opt.label} + description={opt.isOther ? t('tool.requestUserInput.otherDescription') : opt.description} + onClick={() => selectOption(currentQuestion, opt)} /> ) })} @@ -257,6 +260,8 @@ export function RequestUserInputFooter(props: { {t('tool.requestUserInput.noteLabel')}