feat(agent-core): feed AskUserQuestion answers back as question text and option labels (#1414)

* feat(agent-core): feed AskUserQuestion answers back as question text and option labels

The flattened answers record the model receives was keyed by synthesized
ids (q_0 / opt_0_1), forcing a cross-message positional lookup against the
original tool call to understand what the user picked — both unreadable in
transcripts and a real model-misreads-the-choice badcase.

- toAgentCoreResponse now takes the original broker request and translates
  wire ids back to question text (keys) and option labels (values);
  unknown ids are kept verbatim, missing request falls back to raw ids
- wire protocol unchanged: clients still answer with option ids; the
  resolve route reads the pending request before settling it
- question texts must be unique per call and option labels unique per
  question, enforced in the tool execution path (AJV cannot express the
  zod refine) and mirrored on the exported schemas
- web transcript card resolves both the new label form and legacy id
  transcripts; TUI and ACP paths already produced the text form

* fix(agent-core): align multi-select answer join across clients and harden question schema

- Join multi-select labels with ', ' in the server translator, matching
  what the TUI reverse-RPC path already emits, so the model sees one
  format regardless of which client answered
- Trim segments in the web transcript resolver before label matching:
  TUI-answered multi-select transcripts (', '-joined) previously lost
  their highlight to a spurious leading-space Other row
- Move the question-text/legacy-q_<i> answer lookup out of the SFC into
  askUserToolParse as answerFor(), per that module's testability intent
- Require non-empty question text and option labels (.min(1)) so empty
  strings are rejected by AJV at the tool boundary instead of failing
  deeper in the protocol layer

* fix(agent-core): resolve option ids only within the answered question

The translator's option-id lookup was a single flat map across all
questions, so a stale or malformed response pairing one question with
another question's option id (q_1 + opt_0_0) was silently translated
into a label that was never offered for that question. Scope the lookup
to the answered question's own options; cross-question and unknown ids
now both pass through verbatim, staying diagnosable.
This commit is contained in:
Kai 2026-07-06 16:37:54 +08:00 committed by GitHub
parent f7b557732b
commit c12f30951f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 568 additions and 122 deletions

View file

@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---
AskUserQuestion now feeds answers back to the model as question text and option labels (e.g. `{"answers":{"Which database?":"Postgres"}}`) instead of synthesized ids like `q_0`/`opt_0_1`, so the model no longer has to map positional ids back to the original options — the wire protocol is unchanged, clients still answer with option ids. Question texts must now be unique per call and option labels unique per question; the web transcript card resolves both the new label form and legacy id transcripts.

View file

@ -1,11 +1,12 @@
<!-- apps/kimi-web/src/components/chat/tool-calls/AskUserTool.vue
Result card for the AskUserQuestion tool. On a successful answer the
output is a single JSON line ({ answers, note? }); answers are keyed by
synthesized question id (`q_<index>`) and the values are synthesized option
ids (`opt_<q>_<o>`, comma-joined for multi-select) or free-text (Other). We
zip answers back to the input questions by index and echo the full option
list, marking the picked option(s) selected and the rest faint so the
transcript shows both what was chosen and what was passed over.
question text and the values are option labels (comma-joined for
multi-select) or free-text (Other). Legacy transcripts instead carry
synthesized ids (`q_<index>` keys, `opt_<q>_<o>` values) both forms are
resolved. We zip answers back to the input questions and echo the full
option list, marking the picked option(s) selected and the rest faint
so the transcript shows both what was chosen and what was passed over.
Background launches and error cases return plain-text output instead of
the answer JSON; those fall back to a raw output view so the task id /
@ -16,6 +17,7 @@ import { useI18n } from 'vue-i18n';
import type { FilePreviewRequest, ToolCall, ToolMedia } from '../../../types';
import { toolGlyph, toolLabel } from '../../../lib/toolMeta';
import {
answerFor,
parseAskInput,
parseAskOutput,
resolveAnswer,
@ -54,7 +56,7 @@ const isDismissed = computed(
() => recognized.value && Object.keys(output.value.answers).length === 0 && output.value.note.length > 0,
);
const resolved = computed(() =>
questions.value.map((_, i) => resolveAnswer(output.value.answers[`q_${i}`])),
questions.value.map((q, i) => resolveAnswer(answerFor(output.value.answers, q.question, i), q.options)),
);
const answeredCount = computed(() => Object.keys(output.value.answers).length);

View file

@ -1,14 +1,16 @@
// Pure parsers for the AskUserQuestion tool card. Kept separate from the SFC so
// the index-zip / id-decode logic is unit-testable without a DOM.
// the answer-resolution logic is unit-testable without a DOM.
//
// Wire shape (from agent-core SCHEMAS §6.4):
// Wire shape:
// tool.arg : JSON { questions: [{ question, header, options[{label,description}], multi_select }] }
// Input questions carry NO id — order === broker order.
// tool.output[0]: on a successful answer, JSON { answers: Record<qid, string|true>, note? }
// qid = `q_<index>`; value = `opt_<q>_<o>` (single),
// `opt_<q>_<o>,opt_<q>_<o>` (multi, comma-joined), free-text
// (Other), or `opt_…,<text>` (multi+Other). skipped → omitted.
// Dismissed → { answers: {}, note }.
// tool.output[0]: on a successful answer, JSON { answers: Record<question text, string|true>, note? }
// value = the chosen option's label (single), labels joined
// with ', ' (multi), free-text (Other), or labels+text
// (multi+Other). skipped → omitted. Dismissed → { answers: {}, note }.
// LEGACY (transcripts from before the label form): keys are
// `q_<index>` and values are synthesized `opt_<q>_<o>` ids —
// still decoded so old sessions keep rendering.
// : on a background launch, plain text (`task_id: …\nstatus: …`);
// on an error (e.g. unsupported interactive questions), plain
// text. Those are NOT the answer payload and must be shown raw.
@ -106,21 +108,59 @@ export function parseAskOutput(output: string[] | undefined): AskOutput {
};
}
/** Look up one question's flattened answer: current transcripts key by the
* question text; legacy transcripts key by `q_<index>`. */
export function answerFor(
answers: Record<string, string | true>,
questionText: string,
index: number,
): string | true | undefined {
return answers[questionText] ?? answers[`q_${index}`];
}
const OPT_ID = /^opt_\d+_(\d+)$/;
/** Decode one question's flattened answer into picked option indices plus any
* free-text "Other" segment. Option ids carry their own index, so this is
* exact rather than a label match; non-`opt_` segments are treated as the
* Other text (joined back with `,` in case the free text itself contained one). */
export function resolveAnswer(value: string | true | undefined): Resolved {
* free-text "Other" segment.
*
* Current form: the value is option label text matched exactly against
* `options` (whole-string first, so a single-select label containing a comma
* still resolves; then per comma-segment for multi-select). Segments are
* trimmed before matching because joiners vary by client (', ' from the
* server translator and TUI, ',' in older transcripts). Legacy form: the
* value is `opt_<q>_<o>` ids whose trailing index is decoded directly.
* Segments that are neither a label nor an id are the Other free text
* (rejoined with ', ' in case the text itself contained a comma). */
export function resolveAnswer(
value: string | true | undefined,
options: readonly AskOption[] = [],
): Resolved {
if (value === undefined) return { selected: new Set(), otherText: '', indeterminate: false };
if (value === true) return { selected: new Set(), otherText: '', indeterminate: true };
const indexByLabel = new Map<string, number>();
// First occurrence wins on (out-of-contract) duplicate labels.
options.forEach((o, i) => {
if (o.label.length > 0 && !indexByLabel.has(o.label)) indexByLabel.set(o.label, i);
});
const whole = indexByLabel.get(value);
if (whole !== undefined) {
return { selected: new Set([whole]), otherText: '', indeterminate: false };
}
const selected = new Set<number>();
const others: string[] = [];
for (const seg of value.split(',')) {
for (const rawSeg of value.split(',')) {
const seg = rawSeg.trim();
const byLabel = indexByLabel.get(seg);
if (byLabel !== undefined) {
selected.add(byLabel);
continue;
}
const m = OPT_ID.exec(seg);
if (m) selected.add(Number(m[1]));
else if (seg.length > 0) others.push(seg);
}
return { selected, otherText: others.join(','), indeterminate: false };
return { selected, otherText: others.join(', '), indeterminate: false };
}

View file

@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest';
import {
answerFor,
parseAskInput,
parseAskOutput,
resolveAnswer,
@ -54,7 +55,15 @@ describe('parseAskInput', () => {
});
describe('parseAskOutput', () => {
it('recognizes an answer payload and reads answers', () => {
it('recognizes an answer payload and reads answers (question-text keys, label values)', () => {
const out = parseAskOutput([
JSON.stringify({ answers: { 'Which auth provider?': 'Auth0' }, note: '' }),
]);
expect(out.recognized).toBe(true);
expect(out.answers).toEqual({ 'Which auth provider?': 'Auth0' });
});
it('recognizes a legacy answer payload (q_<i> keys, opt ids)', () => {
const out = parseAskOutput([JSON.stringify({ answers: { q_0: 'opt_0_1' }, note: '' })]);
expect(out.recognized).toBe(true);
expect(out.answers).toEqual({ q_0: 'opt_0_1' });
@ -97,46 +106,115 @@ describe('parseAskOutput', () => {
});
describe('resolveAnswer', () => {
it('decodes a single-select option id to its index', () => {
const r = resolveAnswer('opt_0_1');
const single = [
{ label: 'Clerk', description: '' },
{ label: 'Auth0', description: '' },
];
const multi = [
{ label: 'Vercel', description: '' },
{ label: 'Fly.io', description: '' },
{ label: 'AWS', description: '' },
];
it('matches a single-select label to its index', () => {
const r = resolveAnswer('Auth0', single);
expect([...r.selected]).toEqual([1]);
expect(r.otherText).toBe('');
expect(r.indeterminate).toBe(false);
});
it('decodes a comma-joined multi-select into several indices', () => {
const r = resolveAnswer('opt_1_0,opt_1_2');
it('matches comma-joined multi-select labels into several indices', () => {
const r = resolveAnswer('Vercel,AWS', multi);
expect(r.selected).toEqual(new Set([0, 2]));
});
it('treats a free-text value as an Other answer', () => {
const r = resolveAnswer('Use OIDC instead of static keys');
expect(r.selected.size).toBe(0);
expect(r.otherText).toBe('Use OIDC instead of static keys');
it("matches comma-space-joined multi-select labels (server / TUI ', ' form)", () => {
const r = resolveAnswer('Vercel, AWS', multi);
expect(r.selected).toEqual(new Set([0, 2]));
expect(r.otherText).toBe('');
});
it('splits a multi+Other value into options plus the free-text segment', () => {
const r = resolveAnswer('opt_0_0,opt_0_2,Custom thing');
it('splits a multi+Other value into labels plus the free-text segment', () => {
const r = resolveAnswer('Vercel, AWS, Custom thing', multi);
expect(r.selected).toEqual(new Set([0, 2]));
expect(r.otherText).toBe('Custom thing');
});
it('joins non-id segments back so Other text containing a comma survives', () => {
const r = resolveAnswer('opt_0_1,alpha,beta');
it('resolves a whole-value label containing a comma (single-select)', () => {
const withComma = [
{ label: 'Fast, but risky', description: '' },
{ label: 'Slow and safe', description: '' },
];
const r = resolveAnswer('Fast, but risky', withComma);
expect([...r.selected]).toEqual([0]);
expect(r.otherText).toBe('');
});
it('treats a free-text value as an Other answer', () => {
const r = resolveAnswer('Use OIDC instead of static keys', single);
expect(r.selected.size).toBe(0);
expect(r.otherText).toBe('Use OIDC instead of static keys');
});
it('decodes a legacy single-select option id to its index', () => {
const r = resolveAnswer('opt_0_1', single);
expect([...r.selected]).toEqual([1]);
expect(r.otherText).toBe('');
expect(r.indeterminate).toBe(false);
});
it('decodes legacy comma-joined multi-select ids into several indices', () => {
const r = resolveAnswer('opt_1_0,opt_1_2', multi);
expect(r.selected).toEqual(new Set([0, 2]));
});
it('splits a legacy multi+Other value into options plus the free-text segment', () => {
const r = resolveAnswer('opt_0_0,opt_0_2,Custom thing', multi);
expect(r.selected).toEqual(new Set([0, 2]));
expect(r.otherText).toBe('Custom thing');
});
it('joins non-matching segments back so Other text containing a comma survives', () => {
const r = resolveAnswer('Auth0,alpha,beta', single);
expect([...r.selected]).toEqual([1]);
expect(r.otherText).toBe('alpha, beta');
});
it('decodes legacy ids without any options context', () => {
const r = resolveAnswer('opt_0_1');
expect([...r.selected]).toEqual([1]);
expect(r.otherText).toBe('alpha,beta');
});
it('marks the literal true as indeterminate', () => {
const r = resolveAnswer(true);
const r = resolveAnswer(true, single);
expect(r.indeterminate).toBe(true);
expect(r.selected.size).toBe(0);
});
it('returns an empty result for skipped / unanswered questions', () => {
const r = resolveAnswer(undefined);
const r = resolveAnswer(undefined, single);
expect(r.selected.size).toBe(0);
expect(r.otherText).toBe('');
expect(r.indeterminate).toBe(false);
});
});
describe('answerFor', () => {
it('prefers the question-text key (current form)', () => {
const answers = { 'Which auth provider?': 'Auth0' } as const;
expect(answerFor(answers, 'Which auth provider?', 0)).toBe('Auth0');
});
it('falls back to the legacy q_<index> key', () => {
const answers = { q_1: 'opt_1_2' } as const;
expect(answerFor(answers, 'Where to deploy?', 1)).toBe('opt_1_2');
});
it('returns undefined when neither key is present (skipped question)', () => {
expect(answerFor({}, 'Which auth provider?', 0)).toBeUndefined();
});
it('passes through the literal true (indeterminate answer)', () => {
expect(answerFor({ 'Which auth provider?': true }, 'Which auth provider?', 0)).toBe(true);
});
});

View file

@ -38,6 +38,11 @@ export interface QuestionItem {
}
export type QuestionAnswerMethod = 'enter' | 'space' | 'number_key';
/**
* Flattened answers keyed by question text; values are the chosen option
* label(s) (comma-joined for multi-select) or free-form "Other" text.
* `true` marks a question as answered without echoing a concrete value.
*/
export type QuestionAnswers = Record<string, string | true>;
export interface QuestionResponse {

View file

@ -40,6 +40,9 @@
* **Synthesizing stable ids** (SDK has no per-item / per-option `id`):
* - `QuestionItem.id` `q_<index>` (e.g. `q_0`, `q_1`, ...)
* - `QuestionOption.id` `opt_<parent_idx>_<option_idx>` (e.g. `opt_0_0`)
* Ids are a wire-only concern: clients answer with them, and
* `toAgentCoreResponse` translates them back to question text / option
* labels so the flattened record the model sees is self-explanatory.
*
* **Anti-corruption**: this is the ONLY place protocolSDK shape translation
* happens for question.
@ -172,33 +175,61 @@ export function toBrokerRequest(
* Protocol REST response body in-process SDK `QuestionResponse` (with
* `answers` flattened to `Record<string, string | true>`).
*
* Normalization rules from SCHEMAS §6.4:
* - single option_id
* - multi option_ids.join(',')
* The wire keeps synthesized ids (`q_<idx>` / `opt_<q>_<o>`) so clients can
* answer unambiguously, but the flattened record is what the ask-user tool
* feeds back to the model so ids are translated back to text here using
* the original broker `request`:
* - key the question's text (falls back to the raw qid
* when the request is unavailable or the qid is
* unknown stale client, defensive)
* - single option label
* - multi labels.join(', ')
* - other text
* - multi_with_other [...option_ids, other_text].join(',')
* - multi_with_other [...labels, other_text].join(', ')
* - skipped OMIT entry
*
* Multi-select joins use `', '` to match what the TUI reverse-RPC path
* already emits, so the model sees one format regardless of which client
* answered.
*
* Unknown qids and option ids including ids that belong to a DIFFERENT
* question than the one being answered are kept verbatim rather than
* resolved or dropped: translating a cross-question id would hand the model
* a plausible-looking label that was never offered for that question, while
* the raw id stays diagnosable.
*/
export function toAgentCoreResponse(
resp: ProtocolQuestionResponse,
request?: ProtocolQuestionRequest,
): InProcessQuestionResponse {
const itemsById = new Map<string, ProtocolQuestionItem>();
for (const item of request?.questions ?? []) {
itemsById.set(item.id, item);
}
const flattened: InProcessQuestionAnswers = {};
for (const [qid, ans] of Object.entries(resp.answers)) {
const item = itemsById.get(qid);
const key = item?.question ?? qid;
// Resolve option ids only within the answered question's own options
// (at most 4, so a linear scan is fine).
const optionText = (id: string): string =>
item?.options.find((o) => o.id === id)?.label ?? id;
switch (ans.kind) {
case 'single':
flattened[qid] = ans.option_id;
flattened[key] = optionText(ans.option_id);
break;
case 'multi':
flattened[qid] = ans.option_ids.join(',');
flattened[key] = ans.option_ids.map(optionText).join(', ');
break;
case 'other':
flattened[qid] = ans.text;
flattened[key] = ans.text;
break;
case 'multi_with_other':
flattened[qid] = [...ans.option_ids, ans.other_text].join(',');
flattened[key] = [...ans.option_ids.map(optionText), ans.other_text].join(', ');
break;
case 'skipped':
// Omitted from the record — matches SCHEMAS §6.4 ("if skipped continue").
// Omitted from the record — skipped questions carry no answer.
break;
default: {
// Defensive: never-reached if Zod schema is the SOT, but TS narrowing

View file

@ -15,6 +15,7 @@ Overusing this tool interrupts the user's flow. Only use it when the user's inpu
- Use multi_select to allow multiple answers to be selected for a question
- Keep option labels concise (1-5 words), use descriptions for trade-offs and details
- Each question should have 2-4 meaningful, distinct options
- Question texts must be unique across the call, and option labels must be unique within each question
- You can ask 1-4 questions at a time; group related questions to minimize interruptions
- If you recommend a specific option, list it first and append "(Recommended)" to its label
- The result is JSON with an `answers` object whose keys identify each answered question; if `answers` is empty and a `note` says the user dismissed it, they declined to answer — proceed with your best judgment and do not re-ask the same question
- The result is JSON with an `answers` object keyed by question text; each value is the chosen option's label (comma-separated labels for multi_select, or the user's own words if they picked "Other"); if `answers` is empty and a `note` says the user dismissed it, they declined to answer — proceed with your best judgment and do not re-ask the same question

View file

@ -34,12 +34,13 @@ import DESCRIPTION from './ask-user.md?raw';
const QuestionOptionSchema = z.object({
label: z
.string()
.min(1)
.describe("Concise display text (1-5 words). If recommended, append '(Recommended)'."),
description: z.string().default('').describe('Brief explanation of trade-offs or implications.'),
});
const QuestionItemSchema = z.object({
question: z.string().describe("A specific, actionable question. End with '?'."),
question: z.string().min(1).describe("A specific, actionable question. End with '?'."),
header: z
.string()
.default('')
@ -67,6 +68,36 @@ export interface AskUserQuestionInput {
}>;
}
const QUESTION_UNIQUENESS_MESSAGE =
'Question texts must be unique across questions, and option labels must be unique within each question.';
/**
* Answers are keyed by question text with option labels as values, so both
* must be unambiguous: question texts unique across the call, option labels
* unique within their question. Runtime tool-arg validation is AJV against
* the JSON Schema (where zod refinements are unrepresentable), so the
* execution path re-runs this check itself.
*/
function questionUniquenessError(
questions: AskUserQuestionInput['questions'],
): string | null {
const texts = new Set<string>();
for (const q of questions) {
if (texts.has(q.question)) {
return `Invalid questions: duplicate question text ${JSON.stringify(q.question)}. ${QUESTION_UNIQUENESS_MESSAGE} Rephrase the duplicates and call the tool again.`;
}
texts.add(q.question);
const labels = new Set<string>();
for (const option of q.options) {
if (labels.has(option.label)) {
return `Invalid questions: duplicate option label ${JSON.stringify(option.label)} in question ${JSON.stringify(q.question)}. ${QUESTION_UNIQUENESS_MESSAGE} Rephrase the duplicates and call the tool again.`;
}
labels.add(option.label);
}
}
return null;
}
const AskUserQuestionInputBaseSchema = z.object({
questions: z
.array(QuestionItemSchema)
@ -82,10 +113,15 @@ const AskUserQuestionInputSchemaWithBackground = AskUserQuestionInputBaseSchema.
.describe(
'Set true to ask in the background and return immediately with a background task_id; you are notified automatically when the user answers — do not poll with TaskOutput while the question is pending.',
),
}).refine((data) => questionUniquenessError(data.questions) === null, {
message: QUESTION_UNIQUENESS_MESSAGE,
});
export const AskUserQuestionInputSchema: z.ZodType<AskUserQuestionInput> =
AskUserQuestionInputBaseSchema;
AskUserQuestionInputBaseSchema.refine(
(data) => questionUniquenessError(data.questions) === null,
{ message: QUESTION_UNIQUENESS_MESSAGE },
);
const QUESTION_DISMISSED_MESSAGE = 'User dismissed the question without answering.';
@ -123,6 +159,13 @@ export class AskUserQuestionTool implements BuiltinTool<AskUserQuestionInput> {
turnId,
}: ExecutableToolContext,
): Promise<ExecutableToolResult> {
// AJV (the runtime arg validator) cannot express the uniqueness refine,
// so enforce it here before any UI interaction or task registration.
const uniquenessError = questionUniquenessError(args.questions);
if (uniquenessError !== null) {
return { isError: true, output: uniquenessError };
}
if (args.background === true) {
return this.executeInBackground(args, { toolCallId, turnId, signal });
}

View file

@ -1,8 +1,11 @@
/**
* Question adapter unit tests (W8.2 / Chain 6).
*
* Covers SCHEMAS §6.4 5-kind Record<string, string | true> normalization
* verbatim.
* Covers the 5-kind Record<string, string | true> normalization: wire
* answers arrive keyed by synthesized ids (`q_<idx>` / `opt_<q>_<o>`), and
* `toAgentCoreResponse` translates them back to question text / option labels
* using the original broker request, so the model sees self-explanatory text
* instead of positional ids.
*/
import { describe, expect, it } from 'vitest';
@ -96,92 +99,184 @@ describe('question-adapter · toBrokerRequest (in-process → protocol)', () =>
});
});
describe('question-adapter · toAgentCoreResponse · SCHEMAS §6.4 verbatim', () => {
it("'single' → answers[qid] = option_id", () => {
const inProc = toAgentCoreResponse({
answers: { q_0: { kind: 'single', option_id: 'opt_0_1' } },
});
expect(inProc.answers).toEqual({ q_0: 'opt_0_1' });
describe('question-adapter · toAgentCoreResponse · id → text translation', () => {
/** Broker request whose synthesized ids the answers below refer to. */
const request = toBrokerRequest(
{
questions: [
{
question: 'Which animal?',
options: [{ label: 'Cat' }, { label: 'Dog' }],
},
{
question: 'Which colors?',
options: [{ label: 'Red' }, { label: 'Green' }, { label: 'Blue' }],
multiSelect: true,
},
],
},
{
questionId: '01J_QUESTION',
sessionId: 'sess_x',
createdAt: '2026-06-04T10:30:00.000Z',
},
);
it("'single' → answers[question text] = option label", () => {
const inProc = toAgentCoreResponse(
{ answers: { q_0: { kind: 'single', option_id: 'opt_0_1' } } },
request,
);
expect(inProc.answers).toEqual({ 'Which animal?': 'Dog' });
});
it("'multi' → answers[qid] = option_ids.join(',') (lossy)", () => {
const inProc = toAgentCoreResponse({
answers: {
q_0: { kind: 'multi', option_ids: ['opt_0_0', 'opt_0_2'] },
},
});
expect(inProc.answers).toEqual({ q_0: 'opt_0_0,opt_0_2' });
});
it("'other' → answers[qid] = text", () => {
const inProc = toAgentCoreResponse({
answers: { q_0: { kind: 'other', text: 'Hippopotamus' } },
});
expect(inProc.answers).toEqual({ q_0: 'Hippopotamus' });
});
it("'multi_with_other' → [...option_ids, other_text].join(',')", () => {
const inProc = toAgentCoreResponse({
answers: {
q_0: {
kind: 'multi_with_other',
option_ids: ['opt_0_0', 'opt_0_1'],
other_text: 'Custom',
it("'multi' → answers[question text] = labels.join(', ')", () => {
const inProc = toAgentCoreResponse(
{
answers: {
q_1: { kind: 'multi', option_ids: ['opt_1_0', 'opt_1_2'] },
},
},
});
expect(inProc.answers).toEqual({ q_0: 'opt_0_0,opt_0_1,Custom' });
request,
);
expect(inProc.answers).toEqual({ 'Which colors?': 'Red, Blue' });
});
it("'other' → answers[question text] = free text verbatim", () => {
const inProc = toAgentCoreResponse(
{ answers: { q_0: { kind: 'other', text: 'Hippopotamus' } } },
request,
);
expect(inProc.answers).toEqual({ 'Which animal?': 'Hippopotamus' });
});
it("'multi_with_other' → [...labels, other_text].join(', ')", () => {
const inProc = toAgentCoreResponse(
{
answers: {
q_1: {
kind: 'multi_with_other',
option_ids: ['opt_1_0', 'opt_1_1'],
other_text: 'Custom',
},
},
},
request,
);
expect(inProc.answers).toEqual({ 'Which colors?': 'Red, Green, Custom' });
});
it("'skipped' → entry OMITTED entirely from the record", () => {
const inProc = toAgentCoreResponse({
answers: {
q_0: { kind: 'single', option_id: 'opt_0_0' },
q_1: { kind: 'skipped' },
q_2: { kind: 'other', text: 'Custom' },
const inProc = toAgentCoreResponse(
{
answers: {
q_0: { kind: 'single', option_id: 'opt_0_0' },
q_1: { kind: 'skipped' },
},
},
});
expect(inProc.answers).toEqual({
q_0: 'opt_0_0',
q_2: 'Custom',
});
request,
);
expect(inProc.answers).toEqual({ 'Which animal?': 'Cat' });
expect(Object.keys(inProc.answers)).not.toContain('Which colors?');
expect(Object.keys(inProc.answers)).not.toContain('q_1');
});
it('handles a mixed 4-item response with one skipped (e2e prompt acceptance)', () => {
const inProc = toAgentCoreResponse({
answers: {
q_0: { kind: 'single', option_id: 'opt_0_0' },
q_1: { kind: 'multi', option_ids: ['opt_1_0', 'opt_1_1'] },
q_2: { kind: 'other', text: 'Hippopotamus' },
q_3: { kind: 'skipped' },
it('keeps unknown qids / option ids verbatim instead of dropping the answer (stale client)', () => {
const inProc = toAgentCoreResponse(
{
answers: {
q_0: { kind: 'single', option_id: 'opt_0_9' },
q_9: { kind: 'single', option_id: 'opt_9_0' },
},
},
method: 'click',
});
request,
);
expect(inProc.answers).toEqual({
q_0: 'opt_0_0',
q_1: 'opt_1_0,opt_1_1',
q_2: 'Hippopotamus',
'Which animal?': 'opt_0_9',
q_9: 'opt_9_0',
});
});
it("keeps a cross-question option id verbatim instead of resolving another question's label", () => {
// opt_0_0 is 'Cat' — an option of question 0, never offered for question 1.
// Translating it would hand the model a plausible-looking answer that was
// never on the list; the raw id stays diagnosable.
const inProc = toAgentCoreResponse(
{
answers: {
q_1: { kind: 'single', option_id: 'opt_0_0' },
},
},
request,
);
expect(inProc.answers).toEqual({ 'Which colors?': 'opt_0_0' });
});
it('resolves in-question ids and keeps cross-question ids verbatim within one multi answer', () => {
const inProc = toAgentCoreResponse(
{
answers: {
q_1: { kind: 'multi', option_ids: ['opt_1_0', 'opt_0_1'] },
},
},
request,
);
expect(inProc.answers).toEqual({ 'Which colors?': 'Red, opt_0_1' });
});
it('falls back to raw ids when no request is available (defensive path)', () => {
const inProc = toAgentCoreResponse(
{
answers: {
q_0: { kind: 'single', option_id: 'opt_0_1' },
q_1: { kind: 'multi', option_ids: ['opt_1_0', 'opt_1_2'] },
},
},
undefined,
);
expect(inProc.answers).toEqual({
q_0: 'opt_0_1',
q_1: 'opt_1_0, opt_1_2',
});
});
it('handles a mixed response with one skipped (e2e prompt acceptance)', () => {
const inProc = toAgentCoreResponse(
{
answers: {
q_0: { kind: 'other', text: 'Hippopotamus' },
q_1: { kind: 'skipped' },
},
method: 'click',
},
request,
);
expect(inProc.answers).toEqual({ 'Which animal?': 'Hippopotamus' });
// method 'click' is NOT in agent-core's in-process method union — dropped.
expect((inProc as { method?: string }).method).toBeUndefined();
});
it("keeps agent-core method values like 'enter' / 'space' / 'number_key'", () => {
const inProc = toAgentCoreResponse({
answers: { q_0: { kind: 'skipped' } },
method: 'enter',
});
const inProc = toAgentCoreResponse(
{
answers: { q_0: { kind: 'skipped' } },
method: 'enter',
},
request,
);
expect((inProc as { method?: string }).method).toBe('enter');
});
it('produces an empty answers record when ALL questions are skipped (partial-answer marker, NOT dismiss)', () => {
const inProc = toAgentCoreResponse({
answers: {
q_0: { kind: 'skipped' },
q_1: { kind: 'skipped' },
const inProc = toAgentCoreResponse(
{
answers: {
q_0: { kind: 'skipped' },
q_1: { kind: 'skipped' },
},
},
});
request,
);
expect(inProc.answers).toEqual({});
// Distinct from dismissedResult() which returns null.
expect(inProc).not.toBeNull();

View file

@ -86,6 +86,106 @@ describe('AskUserQuestionTool', () => {
).toBe(false);
});
it('rejects empty question text and empty option labels at the schema layer', () => {
expect(
AskUserQuestionInputSchema.safeParse(input({ question: '' })).success,
).toBe(false);
expect(
AskUserQuestionInputSchema.safeParse(
input({
options: [
{ label: '', description: 'Empty label' },
{ label: 'B', description: '' },
],
}),
).success,
).toBe(false);
});
it('rejects duplicate question texts across questions (schema + execution)', async () => {
const duplicated: AskUserQuestionInput = {
questions: [input().questions[0]!, input().questions[0]!],
};
expect(AskUserQuestionInputSchema.safeParse(duplicated).success).toBe(false);
const { tool, requestQuestion } = makeTool();
const result = await executeTool(tool, {
turnId: '0',
toolCallId: 'call_dup_question',
args: duplicated,
signal,
});
expect(result.isError).toBe(true);
expect(result.output).toContain('unique');
expect(requestQuestion).not.toHaveBeenCalled();
});
it('rejects duplicate option labels within one question (schema + execution)', async () => {
const duplicated = input({
options: [
{ label: 'Postgres', description: 'Relational storage' },
{ label: 'Postgres', description: 'Same label again' },
],
});
expect(AskUserQuestionInputSchema.safeParse(duplicated).success).toBe(false);
const { tool, requestQuestion } = makeTool();
const result = await executeTool(tool, {
turnId: '0',
toolCallId: 'call_dup_label',
args: duplicated,
signal,
});
expect(result.isError).toBe(true);
expect(result.output).toContain('unique');
expect(requestQuestion).not.toHaveBeenCalled();
});
it('allows the same option label to appear in different questions', async () => {
const args: AskUserQuestionInput = {
questions: [
input().questions[0]!,
input({ question: 'Which cache?' }).questions[0]!,
],
};
expect(AskUserQuestionInputSchema.safeParse(args).success).toBe(true);
const { tool, requestQuestion } = makeTool();
const result = await executeTool(tool, {
turnId: '0',
toolCallId: 'call_cross_label',
args,
signal,
});
expect(result.isError).toBe(false);
expect(requestQuestion).toHaveBeenCalledOnce();
});
it('rejects duplicate questions on the background path before starting a task', async () => {
const { manager } = createBackgroundManager();
const requestQuestion = vi.fn();
const agent = {
rpc: { requestQuestion },
telemetry: { track: vi.fn() },
background: manager,
} as unknown as Agent;
const tool = new AskUserQuestionTool(agent);
const result = await executeTool(tool, {
turnId: '0',
toolCallId: 'call_bg_dup',
args: {
questions: [input().questions[0]!, input().questions[0]!],
background: true,
},
signal,
});
expect(result.isError).toBe(true);
expect(result.output).toContain('unique');
expect(result.output).not.toContain('task_id:');
expect(requestQuestion).not.toHaveBeenCalled();
});
it('describes the no-Other rule on options and the Recommended hint on label', () => {
const { tool } = makeTool();
const params = tool.parameters as {

View file

@ -95,4 +95,35 @@ describe('builtin tool input JSON Schema', () => {
// The closed-object guard must hold at every nesting level.
expect(validateToolArgs(validator, { questions: [question] })).not.toBeNull();
});
it('rejects an empty option label through runtime validation (minLength reaches AJV)', () => {
const tool = askUserQuestionTool();
const validator = compileToolArgsValidator(tool.parameters);
const question = {
question: 'Which?',
options: [
{ label: '', description: '' },
{ label: 'B', description: '' },
],
};
expect(validateToolArgs(validator, { questions: [question] })).not.toBeNull();
});
it('keeps the AskUserQuestion JSON Schema valid despite the zod uniqueness refine', () => {
// The uniqueness constraint (unique question texts, unique labels per
// question) is a zod refine — unrepresentable in JSON Schema, so AJV must
// still ACCEPT duplicates here. Enforcement lives in the tool's execution
// path, not in the model-facing schema.
const tool = askUserQuestionTool();
expect(tool.parameters).toMatchObject({ type: 'object' });
const validator = compileToolArgsValidator(tool.parameters);
const question = {
question: 'Which?',
options: [
{ label: 'A', description: '' },
{ label: 'A', description: '' },
],
};
expect(validateToolArgs(validator, { questions: [question] })).toBeNull();
});
});

View file

@ -226,7 +226,11 @@ export function registerQuestionsRoutes(
}
const body = bodyParse.data;
const inProc = questionToAgentCoreResponse(body);
// Read the pending request BEFORE resolve() drops it: the translator
// needs the original items/options to map wire ids back to question
// text / option labels for the SDK-facing (model-facing) record.
const pendingRequest = broker.getPendingRequest(questionId);
const inProc = questionToAgentCoreResponse(body, pendingRequest);
broker.resolve(questionId, inProc);
broker.markResolved(questionId);

View file

@ -185,6 +185,16 @@ export class QuestionService extends Disposable implements IQuestionService {
return this._pending.has(questionId);
}
/**
* Protocol request for a still-pending question. The resolve route needs it
* to translate wire ids back to question text / option labels before the
* response reaches the SDK; must be read BEFORE `resolve()` settles (and
* thereby drops) the pending entry.
*/
getPendingRequest(questionId: string): ProtocolQuestionRequest | undefined {
return this._pending.get(questionId)?.protocolRequest;
}
listPending(sessionId: string): ProtocolQuestionRequest[] {
return Array.from(this._pending.values())
.filter((p) => p.sessionId === sessionId)

View file

@ -261,7 +261,8 @@ describe('Question reverse-RPC: WS broadcast → REST resolve → Promise settle
expect(env.code).toBe(0);
expect(env.data?.resolved).toBe(true);
// Promise resolves with the SCHEMAS §6.4 flattened shape.
// Promise resolves with the flattened shape: wire ids translated back to
// question text / option labels for the SDK-facing record.
const result = await pending;
expect(result).not.toBeNull();
const inProcResp = result as {
@ -269,10 +270,10 @@ describe('Question reverse-RPC: WS broadcast → REST resolve → Promise settle
method?: string;
};
expect(inProcResp.answers).toEqual({
q_0: 'opt_0_0',
q_1: 'opt_1_0,opt_1_2',
q_2: 'Hippopotamus',
// q_3 omitted entirely (kind: skipped)
'Animal?': 'Cat',
'Colors?': 'R, B',
'Custom?': 'Hippopotamus',
// 'Skip me' omitted entirely (kind: skipped)
});
expect(inProcResp.method).toBe('enter');
@ -384,19 +385,19 @@ describe('Question reverse-RPC: WS broadcast → REST resolve → Promise settle
'single kind',
[{ question: '?', options: [{ label: 'A' }, { label: 'B' }] }],
{ q_0: { kind: 'single', option_id: 'opt_0_1' } },
{ q_0: 'opt_0_1' },
{ '?': 'B' },
],
[
'multi kind',
[{ question: '?', options: [{ label: 'A' }, { label: 'B' }, { label: 'C' }], multiSelect: true }],
{ q_0: { kind: 'multi', option_ids: ['opt_0_0', 'opt_0_2'] } },
{ q_0: 'opt_0_0,opt_0_2' },
{ '?': 'A, C' },
],
[
'other kind',
[{ question: '?', options: [{ label: 'X' }, { label: 'Y' }], otherLabel: 'Other' }],
{ q_0: { kind: 'other', text: 'free' } },
{ q_0: 'free' },
{ '?': 'free' },
],
[
'multi_with_other kind',
@ -408,7 +409,7 @@ describe('Question reverse-RPC: WS broadcast → REST resolve → Promise settle
other_text: 'X',
},
},
{ q_0: 'opt_0_0,X' },
{ '?': 'A, X' },
],
[
'skipped kind (record entry omitted)',
@ -417,7 +418,7 @@ describe('Question reverse-RPC: WS broadcast → REST resolve → Promise settle
{},
],
] as const)(
'normalizes %s per SCHEMAS §6.4',
'normalizes %s into question-text/label form',
async (_label, questions, answers, expectedRecord) => {
const r = await bootDaemon();
const sid = await createSession(r);