diff --git a/.changeset/ask-user-question-forms.md b/.changeset/ask-user-question-forms.md index 9ba1f9e..3eab8ae 100644 --- a/.changeset/ask-user-question-forms.md +++ b/.changeset/ask-user-question-forms.md @@ -2,4 +2,4 @@ "@jmfederico/pi-web": patch --- -Add an `ask_user` session tool that lets agents post structured question sets as one browser form. Agents end their run while the form waits; users can submit full or partial answers, unanswered questions are reported explicitly, pending forms survive browser and web/API reconnects, and closed forms remain readable in the transcript. Disable the tool with `askUser: false` or `PI_WEB_ASK_USER=false`. +Add an `ask_user` session tool that lets agents post structured question sets as one chat-native browser form. The form uses the transcript's single scroll area, keeps its header visible, and always gives every question a Custom free-text answer. Agents end their run while the form waits; users can submit full or partial answers, unanswered questions are reported explicitly, pending forms survive browser and web/API reconnects, and closed forms remain readable in the transcript. Disable the tool with `askUser: false` or `PI_WEB_ASK_USER=false`. diff --git a/docs/config.html b/docs/config.html index fd57961..44d2105 100644 --- a/docs/config.html +++ b/docs/config.html @@ -810,9 +810,9 @@

The tool accepts one set of 1–20 questions. Each question has a unique id, its question text, optional supporting detail, up to 12 options with stable values and - user-facing labels, and optional allowOther and multiple flags. A question must - offer at least one option or allow free text. No question is required: the user may leave any of them - unanswered. + user-facing labels, and an optional multiple flag. The browser always adds a + Custom free-text answer, including when the model supplies no options. No question is + required: the user may leave any of them unanswered.

Calling ask_user posts the whole set as one browser form and ends the current agent run diff --git a/docs/config.md b/docs/config.md index 14fdf02..76ebfcb 100644 --- a/docs/config.md +++ b/docs/config.md @@ -275,7 +275,7 @@ In **Settings → Session daemon**, these keys are saved on the selected machine `askUser` controls whether agents receive the core `ask_user` tool. It defaults to `true`; set it to `false`, or set `PI_WEB_ASK_USER=false`, to remove the tool. The environment override accepts `0|1|true|false` and takes precedence over the config file. -The tool accepts one set of 1–20 questions. Each question has a unique `id`, its `question` text, optional supporting `detail`, up to 12 options with stable values and user-facing labels, and optional `allowOther` and `multiple` flags. A question must offer at least one option or allow free text. No question is required: the user may leave any of them unanswered. +The tool accepts one set of 1–20 questions. Each question has a unique `id`, its `question` text, optional supporting `detail`, up to 12 options with stable values and user-facing labels, and an optional `multiple` flag. The browser always adds a **Custom** free-text answer, including when the model supplies no options. No question is required: the user may leave any of them unanswered. Calling `ask_user` posts the whole set as one browser form and ends the current agent run instead of waiting for the user. The open form is owned by the session daemon, so it survives a browser disconnect, browser reload, or web/API restart while that daemon keeps running. When the user submits, the answers arrive as a follow-up that wakes the session; each question is reported with its selected option values or free text, or explicitly as unanswered. diff --git a/src/client/src/api/parsers.test.ts b/src/client/src/api/parsers.test.ts index 4fd6a21..41f06db 100644 --- a/src/client/src/api/parsers.test.ts +++ b/src/client/src/api/parsers.test.ts @@ -723,14 +723,14 @@ describe("API parsers", () => { })).toThrow("Invalid notification clear reason"); }); - it("parses an open ask with options, details, other, and multi-select", () => { + it("parses an open ask and normalizes every question to allow custom answers", () => { const parsed = parseSessionStatus({ ...statusWire(), pendingAsk: pendingAskWire() }); expect(parsed.pendingAsk).toEqual({ askId: "ask-1", askedAt: "2026-07-20T00:00:00.000Z", questions: [ - { id: "q1", question: "Which database?", detail: "Pick the primary store", options: [{ value: "pg", label: "Postgres", detail: "Relational" }, { value: "sqlite", label: "SQLite" }] }, + { id: "q1", question: "Which database?", detail: "Pick the primary store", options: [{ value: "pg", label: "Postgres", detail: "Relational" }, { value: "sqlite", label: "SQLite" }], allowOther: true }, { id: "q2", question: "Which extras?", options: [{ value: "metrics", label: "Metrics" }], allowOther: true, multiple: true }, ], }); @@ -740,13 +740,15 @@ describe("API parsers", () => { expect(parseSessionStatus(statusWire()).pendingAsk).toBeUndefined(); }); - it("rejects an ask that cannot be rendered or answered honestly", () => { + it("validates an ask before rendering it", () => { const ask = pendingAskWire(); const first = ask.questions[0]; expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [] } })).toThrow("Pending ask has no questions"); expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [first, first] } })).toThrow("Duplicate ask question id"); expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, askId: "" } })).toThrow("Expected non-empty string field: askId"); - expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [{ id: "q1", question: "Anything?", options: [] }] } })).toThrow("Ask question offers no way to answer"); + expect(parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [{ id: "q1", question: "Anything?", options: [] }] } }).pendingAsk?.questions[0]) + .toEqual({ id: "q1", question: "Anything?", options: [], allowOther: true }); + expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [{ id: "q1", question: "Anything?", options: [], allowOther: "yes" }] } })).toThrow("Expected optional boolean field: allowOther"); expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [{ id: "q1", question: "Which?", options: [{ value: "a", label: "A" }, { value: "a", label: "Also A" }] }] } })).toThrow("Duplicate ask option value"); expect(() => parseSessionStatus({ ...statusWire(), pendingAsk: { ...ask, questions: [{ id: "q1", question: "x".repeat(ASK_USER_TEXT_MAX_LENGTH + 1), options: [{ value: "a", label: "A" }] }] } })).toThrow("String field exceeds limit: question"); }); @@ -814,7 +816,7 @@ function pendingAskWire() { askId: "ask-1", askedAt: "2026-07-20T00:00:00.000Z", questions: [ - { id: "q1", question: "Which database?", detail: "Pick the primary store", options: [{ value: "pg", label: "Postgres", detail: "Relational" }, { value: "sqlite", label: "SQLite" }] }, + { id: "q1", question: "Which database?", detail: "Pick the primary store", options: [{ value: "pg", label: "Postgres", detail: "Relational" }, { value: "sqlite", label: "SQLite" }], allowOther: false }, { id: "q2", question: "Which extras?", options: [{ value: "metrics", label: "Metrics" }], allowOther: true, multiple: true }, ], }; diff --git a/src/client/src/api/parsers.ts b/src/client/src/api/parsers.ts index f457788..97e23f0 100644 --- a/src/client/src/api/parsers.ts +++ b/src/client/src/api/parsers.ts @@ -218,17 +218,16 @@ function parseAskUserQuestion(value: unknown): AskUserQuestion { const record = requireRecord(value); const options = boundedArrayOf(record["options"], parseAskUserQuestionOption, ASK_USER_OPTION_LIMIT, "options"); assertUniqueStrings(options.map((option) => option.value), "ask option value"); - const allowOther = parseOptionalBoolean(record["allowOther"], "allowOther"); + // Validate the legacy wire field when present, but normalize every question to + // the current invariant: the browser always offers a custom answer. + parseOptionalBoolean(record["allowOther"], "allowOther"); const multiple = parseOptionalBoolean(record["multiple"], "multiple"); - // A question offering neither options nor a free-text field cannot be answered - // at all, which would make reporting it as unanswered meaningless. - if (options.length === 0 && allowOther !== true) throw new Error("Ask question offers no way to answer"); return { id: requireBoundedNonEmptyString(record, "id", ASK_USER_ID_MAX_LENGTH), question: requireBoundedNonEmptyString(record, "question", ASK_USER_TEXT_MAX_LENGTH), ...optionalField("detail", optionalBoundedNonEmptyString(record, "detail", ASK_USER_TEXT_MAX_LENGTH)), options, - ...(allowOther === undefined ? {} : { allowOther }), + allowOther: true, ...(multiple === undefined ? {} : { multiple }), }; } diff --git a/src/client/src/askDrafts.test.ts b/src/client/src/askDrafts.test.ts index 10f65cf..75826a7 100644 --- a/src/client/src/askDrafts.test.ts +++ b/src/client/src/askDrafts.test.ts @@ -50,11 +50,10 @@ const singleSelect: AskUserQuestion = { options: [{ value: "pg", label: "Postgres" }, { value: "sqlite", label: "SQLite" }], }; -const multiSelectWithOther: AskUserQuestion = { +const multiSelect: AskUserQuestion = { id: "q2", question: "Which extras?", options: [{ value: "metrics", label: "Metrics" }, { value: "tracing", label: "Tracing" }], - allowOther: true, multiple: true, }; @@ -62,10 +61,9 @@ const freeTextOnly: AskUserQuestion = { id: "q3", question: "Anything else?", options: [], - allowOther: true, }; -const questions = [singleSelect, multiSelectWithOther, freeTextOnly]; +const questions = [singleSelect, multiSelect, freeTextOnly]; afterEach(() => { Object.defineProperty(globalThis, "localStorage", { value: undefined, configurable: true }); @@ -151,7 +149,7 @@ describe("ask answer state", () => { expect(toSubmission(questions, answers).answers.map((answer) => answer.id)).toEqual(["q1", "q3"]); }); - it("keeps several values and other text together for a multi-select question", () => { + it("keeps several values and custom text together for a multi-select question", () => { const answers: AskDraftAnswers = { q2: { values: ["metrics", "tracing"], otherText: "profiling" } }; expect(toSubmission(questions, answers)).toEqual({ @@ -178,15 +176,13 @@ describe("ask answer state", () => { }); }); - it("keeps other text for a single-select question that has no selected option", () => { - const singleWithOther: AskUserQuestion = { ...singleSelect, allowOther: true }; - - expect(toSubmission([singleWithOther], { q1: { values: [], otherText: "neither" } })).toEqual({ + it("keeps custom text for every single-select question when no option is selected", () => { + expect(toSubmission([singleSelect], { q1: { values: [], otherText: "neither" } })).toEqual({ answers: [{ id: "q1", values: [], otherText: "neither" }], }); }); - it("bounds other text at the shared limit", () => { + it("bounds custom text at the shared limit", () => { const answers: AskDraftAnswers = { q3: { values: [], otherText: "a".repeat(ASK_USER_OTHER_TEXT_MAX_LENGTH + 10) } }; expect(toSubmission(questions, answers).answers[0]?.otherText).toHaveLength(ASK_USER_OTHER_TEXT_MAX_LENGTH); diff --git a/src/client/src/askDrafts.ts b/src/client/src/askDrafts.ts index 4959659..064135f 100644 --- a/src/client/src/askDrafts.ts +++ b/src/client/src/askDrafts.ts @@ -120,7 +120,7 @@ function submittableAnswer(question: AskUserQuestion, answer: AskDraftAnswer | u if (answer === undefined) return undefined; const offered = new Set(question.options.map((option) => option.value)); const values = [...new Set(answer.values)].filter((value) => offered.has(value)); - const otherText = normalizedOtherText(question, answer.otherText); + const otherText = normalizedOtherText(answer.otherText); if (question.multiple !== true && values.length + (otherText === undefined ? 0 : 1) > 1) { const single = values[0]; if (single !== undefined) return { id: question.id, values: [single] }; @@ -130,8 +130,8 @@ function submittableAnswer(question: AskUserQuestion, answer: AskDraftAnswer | u return { id: question.id, values, ...(otherText === undefined ? {} : { otherText }) }; } -function normalizedOtherText(question: AskUserQuestion, otherText: string | undefined): string | undefined { - if (otherText === undefined || question.allowOther !== true) return undefined; +function normalizedOtherText(otherText: string | undefined): string | undefined { + if (otherText === undefined) return undefined; const trimmed = otherText.trim().slice(0, ASK_USER_OTHER_TEXT_MAX_LENGTH); return trimmed === "" ? undefined : trimmed; } diff --git a/src/client/src/chatMessages.test.ts b/src/client/src/chatMessages.test.ts index 7d2d68e..9c664a4 100644 --- a/src/client/src/chatMessages.test.ts +++ b/src/client/src/chatMessages.test.ts @@ -10,12 +10,12 @@ const askUserOutcome: AskUserOutcome = { closedAt: "2026-07-20T10:05:00.000Z", questions: [ { - question: { id: "db", question: "Which database?", options: [{ value: "pg", label: "Postgres" }] }, + question: { id: "db", question: "Which database?", options: [{ value: "pg", label: "Postgres" }], allowOther: true }, answered: true, values: ["pg"], }, { - question: { id: "cache", question: "Which cache?", options: [{ value: "redis", label: "Redis" }] }, + question: { id: "cache", question: "Which cache?", options: [{ value: "redis", label: "Redis" }], allowOther: true }, answered: false, values: [], }, diff --git a/src/client/src/chatTranscript.test.ts b/src/client/src/chatTranscript.test.ts index 04f4243..5b69977 100644 --- a/src/client/src/chatTranscript.test.ts +++ b/src/client/src/chatTranscript.test.ts @@ -12,12 +12,12 @@ const askUserOutcome: AskUserOutcome = { closedAt: "2026-07-20T10:05:00.000Z", questions: [ { - question: { id: "editor", question: "Which editor?", options: [{ value: "vim", label: "Vim" }] }, + question: { id: "editor", question: "Which editor?", options: [{ value: "vim", label: "Vim" }], allowOther: true }, answered: true, values: ["vim"], }, { - question: { id: "region", question: "Which region?", options: [{ value: "eu", label: "Europe" }] }, + question: { id: "region", question: "Which region?", options: [{ value: "eu", label: "Europe" }], allowOther: true }, answered: false, values: [], }, diff --git a/src/client/src/components/AskUserCard.test.ts b/src/client/src/components/AskUserCard.test.ts index eda1be2..8c349b0 100644 --- a/src/client/src/components/AskUserCard.test.ts +++ b/src/client/src/components/AskUserCard.test.ts @@ -33,7 +33,7 @@ describe("ask-user-card live form", () => { expect(code.name).toBe(vim.name); expect(web.type).toBe("checkbox"); expect(web.name).not.toBe(vim.name); - expect(root.querySelector("[aria-live='polite']")?.textContent).toContain("Answered 0 of 2"); + expect(root.querySelector("[aria-live='polite']")?.textContent).toContain("0 of 2 answered"); // Focus and interaction run through the rendered native control rather than // extracting Lit handlers, so this exercises the form's browser boundary. @@ -44,7 +44,7 @@ describe("ask-user-card live form", () => { expect(vim.checked).toBe(true); expect(code.checked).toBe(false); - expect(root.querySelector("[aria-live='polite']")?.textContent).toContain("Answered 1 of 2"); + expect(root.querySelector("[aria-live='polite']")?.textContent).toContain("1 of 2 answered"); }); it("accumulates several checkbox values for a multi-select question", async () => { @@ -58,7 +58,7 @@ describe("ask-user-card live form", () => { inputWithValue(root, "desktop").click(); await card.updateComplete; - expect(root.querySelector("[aria-live='polite']")?.textContent).toContain("Answered 1 of 1"); + expect(root.querySelector("[aria-live='polite']")?.textContent).toContain("1 of 1 answered"); buttonWithText(root, "Send answers").click(); await Promise.resolve(); @@ -67,10 +67,10 @@ describe("ask-user-card live form", () => { }); }); - it("reveals and focuses a labelled other field while preserving multi-select options", async () => { + it("always offers and focuses a labelled custom field while preserving multi-select options", async () => { const onSubmit = vi.fn(); const card = await mountOpenAsk(openAsk([ - question("stack", "Pick the stack", [option("lit", "Lit"), option("react", "React")], { multiple: true, allowOther: true }), + question("stack", "Pick the stack", [option("lit", "Lit"), option("react", "React")], { multiple: true }), ]), onSubmit); const root = renderRoot(card); @@ -79,9 +79,9 @@ describe("ask-user-card live form", () => { await card.updateComplete; await Promise.resolve(); - const textarea = requiredElement(root.querySelector("textarea"), "other textarea"); - const label = requiredElement(textarea.closest("label"), "other label"); - expect(label.textContent).toContain("Your answer for “Pick the stack”"); + const textarea = requiredElement(root.querySelector("textarea"), "custom textarea"); + const label = requiredElement(textarea.closest("label"), "custom label"); + expect(label.textContent).toContain("Custom answer"); expect(root.activeElement).toBe(textarea); textarea.value = "Svelte"; @@ -95,12 +95,33 @@ describe("ask-user-card live form", () => { }); }); + it("shows and submits the custom field directly when no options were supplied", async () => { + const onSubmit = vi.fn(); + const card = await mountOpenAsk(openAsk([ + question("notes", "Anything else?", []), + ]), onSubmit); + const root = renderRoot(card); + const textarea = requiredElement(root.querySelector("textarea"), "custom textarea"); + + expect(root.querySelector("input")).toBeNull(); + expect(requiredElement(textarea.closest("label"), "custom label").textContent).toContain("Custom answer"); + textarea.value = "Keep the first version small."; + textarea.dispatchEvent(new Event("input", { bubbles: true, composed: true })); + await card.updateComplete; + buttonWithText(root, "Send answers").click(); + await Promise.resolve(); + + expect(onSubmit).toHaveBeenCalledWith("ask-1", { + answers: [{ id: "notes", values: [], otherText: "Keep the first version small." }], + }); + }); + it("names unanswered questions before allowing a partial submit", async () => { const onSubmit = vi.fn(); const card = await mountOpenAsk(openAsk([ question("editor", "Choose an editor", [option("vim", "Vim")]), question("deploy", "Choose a deployment target", [option("cloud", "Cloud")]), - question("notes", "Add implementation notes", [], { allowOther: true }), + question("notes", "Add implementation notes", []), ]), onSubmit); const root = renderRoot(card); @@ -139,7 +160,7 @@ describe("ask-user-card record mode", () => { closedAt: "2026-07-20T10:05:00.000Z", questions: [ unansweredRecord(question("speed", "Preferred pace", [option("fast", "Fast"), option("careful", "Careful")])), - unansweredRecord(question("rationale", "Why?", [], { allowOther: true })), + unansweredRecord(question("rationale", "Why?", [])), unansweredRecord(question("region", "Deployment region", [option("eu", "Europe")])), ], answeredCount: 0, @@ -154,7 +175,7 @@ describe("ask-user-card record mode", () => { const root = renderRoot(card); expect(root.querySelector("input, textarea, button, select")).toBeNull(); - expect(root.textContent).toContain("Questions superseded"); + expect(root.textContent).toContain("Superseded"); expect(root.textContent).toContain("Fast"); expect(root.textContent).toContain("It keeps the feedback loop short."); expect(root.textContent).toContain("Draft answer · not sent"); @@ -200,7 +221,7 @@ function question( id: string, text: string, options: AskUserQuestion["options"], - settings: { detail?: string; multiple?: boolean; allowOther?: boolean } = {}, + settings: { detail?: string; multiple?: boolean } = {}, ): AskUserQuestion { return { id, @@ -208,7 +229,6 @@ function question( options, ...(settings.detail === undefined ? {} : { detail: settings.detail }), ...(settings.multiple === undefined ? {} : { multiple: settings.multiple }), - ...(settings.allowOther === undefined ? {} : { allowOther: settings.allowOther }), }; } diff --git a/src/client/src/components/AskUserCard.ts b/src/client/src/components/AskUserCard.ts index f9d6b62..b77f190 100644 --- a/src/client/src/components/AskUserCard.ts +++ b/src/client/src/components/AskUserCard.ts @@ -70,20 +70,16 @@ export class AskUserCard extends LitElement { return html`

-
-

Pi needs your input

-

Questions from the model

-
- ${ask.questions.length} ${ask.questions.length === 1 ? "question" : "questions"} +

Questions

+ + ${count} of ${ask.questions.length} answered +
{ this.handleSubmit(event, ask); }}> -
+
${ask.questions.map((question, index) => this.renderQuestion(ask, question, index))}