mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-10 01:39:25 +00:00
fix(agent-core-v2): align AskUserQuestion tool chain with v1
- translate wire ids back to question text / option labels when resolving a question over REST, joining multi-select labels with ', ' - enforce unique question texts / option labels and non-empty strings at both the schema and the execution path - cancel pending questions on turn abort or background task stop by dismissing the parked entry (resolves null, v1 broker semantics) - restore the unsupported-client fallback and dismissed-error handling - pass empty header / option description through verbatim and align the model-facing tool description byte-for-byte with v1 - drop the synthetic expires_at field from the question wire shape
This commit is contained in:
Kaiyi
parent
4977ccb445
commit
0dfc99daf2
9 changed files with 925 additions and 71 deletions
6
.changeset/ask-user-question-v2-parity.md
Normal file
6
.changeset/ask-user-question-v2-parity.md
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
"@moonshot-ai/agent-core-v2": patch
|
||||
"@moonshot-ai/kap-server": patch
|
||||
---
|
||||
|
||||
Fix the v2 AskUserQuestion flow: answers now come back keyed by question text with option labels as values, aborting a turn or stopping a background question dismisses the pending question instead of leaking it, and duplicate question texts or option labels are rejected before the question is shown. The pending-question wire shape no longer carries a synthetic expires_at field.
|
||||
|
|
@ -15,6 +15,8 @@ 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
|
||||
- Set background=true when you can keep working without the answer. This starts a background question task and returns a task_id immediately. The answer arrives automatically in a later turn — you do not need to poll, sleep, or check on it. Continue with other work; never fabricate or predict the answer.
|
||||
- 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
|
||||
- Set background=true when you can keep working without the answer. This starts a background question task and returns a task_id immediately. The answer arrives automatically in a later turn — you do not need to poll, sleep, or check on it. Continue with other work; never fabricate or predict the answer.
|
||||
|
|
@ -10,7 +10,10 @@
|
|||
|
||||
import { z } from 'zod';
|
||||
|
||||
import { CoreErrors } from '#/_base/errors/codes';
|
||||
import { KimiError } from '#/_base/errors/errors';
|
||||
import { toInputJsonSchema } from '#/_base/tools/support/input-schema';
|
||||
import { errorMessage, isAbortError } from '#/agent/loop/errors';
|
||||
import { IAgentTaskService } from '#/agent/task/task';
|
||||
import { ITelemetryService } from '#/app/telemetry/telemetry';
|
||||
import type { TelemetryProperties } from '#/app/telemetry/telemetry';
|
||||
|
|
@ -37,12 +40,13 @@ import { QuestionTask } from './question-task';
|
|||
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('')
|
||||
|
|
@ -70,6 +74,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)
|
||||
|
|
@ -83,15 +117,23 @@ const AskUserQuestionInputSchemaWithBackground = AskUserQuestionInputBaseSchema.
|
|||
.boolean()
|
||||
.default(false)
|
||||
.describe(
|
||||
'Set true to ask in the background and return immediately with a background task_id. Use TaskOutput to read the answer later.',
|
||||
'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> =
|
||||
AskUserQuestionInputSchemaWithBackground;
|
||||
AskUserQuestionInputBaseSchema.refine(
|
||||
(data) => questionUniquenessError(data.questions) === null,
|
||||
{ message: QUESTION_UNIQUENESS_MESSAGE },
|
||||
);
|
||||
|
||||
const QUESTION_DISMISSED_MESSAGE = 'User dismissed the question without answering.';
|
||||
|
||||
const QUESTION_UNSUPPORTED_FAILURE_MESSAGE =
|
||||
'The connected client does not support interactive questions. Do NOT call this tool again. Ask the user directly in your text response instead.';
|
||||
|
||||
// ── Implementation ───────────────────────────────────────────────────
|
||||
|
||||
export class AskUserQuestionTool implements BuiltinTool<AskUserQuestionInput> {
|
||||
|
|
@ -122,45 +164,72 @@ export class AskUserQuestionTool implements BuiltinTool<AskUserQuestionInput> {
|
|||
args: AskUserQuestionInput,
|
||||
{ toolCallId, signal, 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 });
|
||||
}
|
||||
return this.executeQuestion(args, { toolCallId, turnId });
|
||||
return this.executeQuestion(args, { toolCallId, turnId, signal });
|
||||
}
|
||||
|
||||
private async executeQuestion(
|
||||
args: AskUserQuestionInput,
|
||||
{ toolCallId, turnId }: Pick<ExecutableToolContext, 'toolCallId' | 'turnId'>,
|
||||
): Promise<ExecutableToolResult> {
|
||||
const result = await this.question.request({
|
||||
turnId,
|
||||
{
|
||||
toolCallId,
|
||||
questions: args.questions.map((q) => ({
|
||||
question: q.question,
|
||||
header: q.header.length > 0 ? q.header : undefined,
|
||||
options: q.options.map((o) => ({
|
||||
label: o.label,
|
||||
description: o.description.length > 0 ? o.description : undefined,
|
||||
})),
|
||||
multiSelect: q.multi_select,
|
||||
})),
|
||||
});
|
||||
signal,
|
||||
turnId,
|
||||
}: Pick<ExecutableToolContext, 'toolCallId' | 'signal' | 'turnId'>,
|
||||
): Promise<ExecutableToolResult> {
|
||||
try {
|
||||
const result = await this.question.request(
|
||||
{
|
||||
turnId,
|
||||
toolCallId,
|
||||
questions: args.questions.map((q) => ({
|
||||
question: q.question,
|
||||
header: q.header,
|
||||
options: q.options.map((o) => ({
|
||||
label: o.label,
|
||||
description: o.description,
|
||||
})),
|
||||
multiSelect: q.multi_select,
|
||||
})),
|
||||
},
|
||||
{ signal },
|
||||
);
|
||||
|
||||
const normalized = normalizeQuestionResult(result);
|
||||
if (normalized === null || Object.keys(normalized.answers).length === 0) {
|
||||
this.telemetry.track('question_dismissed');
|
||||
return dismissedQuestionResult();
|
||||
}
|
||||
|
||||
const properties: TelemetryProperties =
|
||||
normalized.method !== undefined
|
||||
? { answered: Object.keys(normalized.answers).length, method: normalized.method }
|
||||
: { answered: Object.keys(normalized.answers).length };
|
||||
this.telemetry.track('question_answered', properties);
|
||||
return {
|
||||
isError: false,
|
||||
output: JSON.stringify({ answers: normalized.answers }),
|
||||
};
|
||||
} catch (error) {
|
||||
if (isAbortError(error) || signal.aborted) throw error;
|
||||
|
||||
if (error instanceof KimiError && error.code === CoreErrors.codes.NOT_IMPLEMENTED) {
|
||||
return {
|
||||
isError: true,
|
||||
output: QUESTION_UNSUPPORTED_FAILURE_MESSAGE,
|
||||
};
|
||||
}
|
||||
|
||||
const normalized = normalizeQuestionResult(result);
|
||||
if (normalized === null || Object.keys(normalized.answers).length === 0) {
|
||||
this.telemetry.track('question_dismissed');
|
||||
return dismissedQuestionResult();
|
||||
}
|
||||
|
||||
const properties: TelemetryProperties =
|
||||
normalized.method !== undefined
|
||||
? { answered: Object.keys(normalized.answers).length, method: normalized.method }
|
||||
: { answered: Object.keys(normalized.answers).length };
|
||||
this.telemetry.track('question_answered', properties);
|
||||
return {
|
||||
isError: false,
|
||||
output: JSON.stringify({ answers: normalized.answers }),
|
||||
};
|
||||
}
|
||||
|
||||
private executeInBackground(
|
||||
|
|
@ -180,7 +249,7 @@ export class AskUserQuestionTool implements BuiltinTool<AskUserQuestionInput> {
|
|||
try {
|
||||
taskId = this.tasks.registerTask(
|
||||
new QuestionTask(
|
||||
() => this.executeQuestion(args, { toolCallId, turnId }),
|
||||
(taskSignal) => this.executeQuestion(args, { toolCallId, turnId, signal: taskSignal }),
|
||||
description,
|
||||
{
|
||||
questionCount: args.questions.length,
|
||||
|
|
@ -250,7 +319,3 @@ function isQuestionResponse(result: Exclude<QuestionResult, null>): result is Qu
|
|||
const answers = (result as { readonly answers?: unknown }).answers;
|
||||
return typeof answers === 'object' && answers !== null && !Array.isArray(answers);
|
||||
}
|
||||
|
||||
function errorMessage(error: unknown): string {
|
||||
return error instanceof Error ? error.message : String(error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
* The model is the **in-process** representation (camelCase, options carry no
|
||||
* ids). The protocol wire shape (snake_case, synthesized item/option ids,
|
||||
* `expires_at`, 5-kind answer union) is produced at the edge — see the
|
||||
* 5-kind answer union) is produced at the edge — see the
|
||||
* `server-v2` questions route, which is the single protocol↔in-process
|
||||
* adapter for this domain. Session-scoped — one instance per session.
|
||||
*/
|
||||
|
|
@ -32,7 +32,11 @@ export interface QuestionItem {
|
|||
|
||||
export type QuestionAnswerMethod = 'enter' | 'space' | 'number_key';
|
||||
|
||||
/** Flattened answers keyed by item id — `true` marks a selected flag-style option. */
|
||||
/**
|
||||
* 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 {
|
||||
|
|
@ -54,7 +58,13 @@ export interface QuestionRequest {
|
|||
export interface ISessionQuestionService {
|
||||
readonly _serviceBrand: undefined;
|
||||
|
||||
request(req: QuestionRequest): Promise<QuestionResult>;
|
||||
/**
|
||||
* Post a question and block on the answer. When `options.signal` aborts
|
||||
* while the question is parked (or was already aborted), the pending entry
|
||||
* is dismissed and the promise resolves with `null` — the same dismissed
|
||||
* result as an explicit dismiss (v1 broker semantics).
|
||||
*/
|
||||
request(req: QuestionRequest, options?: { signal?: AbortSignal }): Promise<QuestionResult>;
|
||||
/**
|
||||
* Post a question without blocking on the answer. Returns the request with
|
||||
* its resolved `id`; the answer is delivered through the interaction
|
||||
|
|
|
|||
|
|
@ -20,13 +20,35 @@ export class SessionQuestionService implements ISessionQuestionService {
|
|||
|
||||
constructor(@ISessionInteractionService private readonly interaction: ISessionInteractionService) {}
|
||||
|
||||
request(req: QuestionRequest): Promise<QuestionResult> {
|
||||
return this.interaction.request<QuestionRequest, QuestionResult>({
|
||||
id: requestId(req),
|
||||
request(req: QuestionRequest, options?: { signal?: AbortSignal }): Promise<QuestionResult> {
|
||||
const id = requestId(req);
|
||||
const pending = this.interaction.request<QuestionRequest, QuestionResult>({
|
||||
id,
|
||||
kind: 'question',
|
||||
payload: req,
|
||||
origin: { turnId: req.turnId },
|
||||
});
|
||||
|
||||
// Mirrors the v1 broker: when the caller aborts (turn interrupted,
|
||||
// background task killed) — or was aborted before parking — the entry is
|
||||
// dismissed so listPending()/session status don't stay stuck in
|
||||
// awaiting_question, and the caller receives the same `null` (dismissed)
|
||||
// result as an explicit dismiss.
|
||||
const signal = options?.signal;
|
||||
if (signal !== undefined) {
|
||||
if (signal.aborted) {
|
||||
this.dismiss(id);
|
||||
} else {
|
||||
const onAbort = (): void => {
|
||||
this.dismiss(id);
|
||||
};
|
||||
signal.addEventListener('abort', onAbort, { once: true });
|
||||
void pending.finally(() => {
|
||||
signal.removeEventListener('abort', onAbort);
|
||||
});
|
||||
}
|
||||
}
|
||||
return pending;
|
||||
}
|
||||
|
||||
enqueue(req: QuestionRequest): QuestionRequest & { readonly id: string } {
|
||||
|
|
|
|||
|
|
@ -97,6 +97,52 @@ describe('ISessionQuestionService (Session scope facade over the interaction ker
|
|||
});
|
||||
});
|
||||
|
||||
it('request with a pre-aborted signal resolves null and parks nothing', async () => {
|
||||
const questions = session.accessor.get(ISessionQuestionService);
|
||||
const controller = new AbortController();
|
||||
controller.abort();
|
||||
|
||||
await expect(
|
||||
questions.request(makeRequest('q1'), { signal: controller.signal }),
|
||||
).resolves.toBeNull();
|
||||
expect(questions.listPending()).toEqual([]);
|
||||
});
|
||||
|
||||
it('aborting a parked request dismisses it and resolves the caller with null', async () => {
|
||||
const interaction = session.accessor.get(ISessionInteractionService);
|
||||
const questions = session.accessor.get(ISessionQuestionService);
|
||||
|
||||
const resolved: { id: string; response: unknown }[] = [];
|
||||
disposables.add(interaction.onDidResolve((r) => resolved.push(r)));
|
||||
|
||||
const controller = new AbortController();
|
||||
const pending = questions.request(makeRequest('q1'), { signal: controller.signal });
|
||||
expect(questions.listPending().map((r) => r.id)).toEqual(['q1']);
|
||||
|
||||
controller.abort();
|
||||
|
||||
// v1 broker semantics: the abort settles the entry as a dismissal, so the
|
||||
// caller sees the same `null` result (→ `event.question.dismissed`) as an
|
||||
// explicit dismiss instead of a rejection.
|
||||
await expect(pending).resolves.toBeNull();
|
||||
expect(questions.listPending()).toEqual([]);
|
||||
expect(resolved).toEqual([{ id: 'q1', response: null }]);
|
||||
expect(interaction.isRecentlyResolved('q1')).toBe(true);
|
||||
});
|
||||
|
||||
it('an answer that arrives before the abort still wins', async () => {
|
||||
const questions = session.accessor.get(ISessionQuestionService);
|
||||
|
||||
const controller = new AbortController();
|
||||
const pending = questions.request(makeRequest('q1'), { signal: controller.signal });
|
||||
questions.answer('q1', { answers: { q_0: 'Yes' } });
|
||||
|
||||
await expect(pending).resolves.toEqual({ answers: { q_0: 'Yes' } });
|
||||
// A late abort is a no-op: the entry is already settled.
|
||||
controller.abort();
|
||||
expect(questions.listPending()).toEqual([]);
|
||||
});
|
||||
|
||||
it('Session scope isolates brokers: a question parked in A is invisible to B', () => {
|
||||
const sessionB = host.child(LifecycleScope.Session, 'session-b');
|
||||
const questionsA = session.accessor.get(ISessionQuestionService);
|
||||
|
|
|
|||
574
packages/agent-core-v2/test/questionTools/ask-user.test.ts
Normal file
574
packages/agent-core-v2/test/questionTools/ask-user.test.ts
Normal file
|
|
@ -0,0 +1,574 @@
|
|||
/**
|
||||
* AskUserQuestionTool unit tests — ported from v1
|
||||
* `packages/agent-core/test/tools/ask-user.test.ts` and adapted to the v2 DI
|
||||
* constructor (`ISessionQuestionService` / `IAgentTaskService` /
|
||||
* `ITelemetryService` stubs instead of a fake `Agent`).
|
||||
*/
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { CoreErrors } from '#/_base/errors/codes';
|
||||
import { KimiError } from '#/_base/errors/errors';
|
||||
import { IAgentTaskService } from '#/agent/task/task';
|
||||
import type { AgentTask, AgentTaskInfoBase, AgentTaskSink, AgentTaskStatus } from '#/agent/task/types';
|
||||
import {
|
||||
AskUserQuestionInputSchema,
|
||||
AskUserQuestionTool,
|
||||
type AskUserQuestionInput,
|
||||
} from '#/agent/questionTools/tools/ask-user';
|
||||
import { ITelemetryService } from '#/app/telemetry/telemetry';
|
||||
import type {
|
||||
ISessionQuestionService,
|
||||
QuestionRequest,
|
||||
QuestionResult,
|
||||
} from '#/session/question/question';
|
||||
import { executeTool } from '../tools/fixtures/execute-tool';
|
||||
|
||||
const signal = new AbortController().signal;
|
||||
|
||||
function input(
|
||||
overrides: Partial<AskUserQuestionInput['questions'][number]> = {},
|
||||
): AskUserQuestionInput {
|
||||
return {
|
||||
questions: [
|
||||
{
|
||||
question: 'Which database?',
|
||||
header: 'Storage',
|
||||
options: [
|
||||
{ label: 'Postgres', description: 'Relational storage' },
|
||||
{ label: 'SQLite', description: 'Embedded storage' },
|
||||
],
|
||||
multi_select: false,
|
||||
...overrides,
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
interface FakeTaskEntry {
|
||||
readonly task: AgentTask;
|
||||
readonly controller: AbortController;
|
||||
output: string;
|
||||
status: AgentTaskStatus;
|
||||
stopReason?: string;
|
||||
readonly settled: Promise<void>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal in-memory stand-in for `IAgentTaskService`: runs a registered task
|
||||
* immediately with a real sink so the QuestionTask wiring (including the
|
||||
* task-signal passthrough) is exercised end to end.
|
||||
*/
|
||||
function createFakeTaskService(): {
|
||||
readonly tasks: IAgentTaskService;
|
||||
readonly registerTask: ReturnType<typeof vi.fn>;
|
||||
entry(taskId: string): FakeTaskEntry;
|
||||
kill(taskId: string): void;
|
||||
wait(taskId: string): Promise<void>;
|
||||
} {
|
||||
const entries = new Map<string, FakeTaskEntry>();
|
||||
let counter = 0;
|
||||
const registerTask = vi.fn((task: AgentTask): string => {
|
||||
counter += 1;
|
||||
const taskId = `${task.idPrefix}-${String(counter).padStart(8, '0')}`;
|
||||
const controller = new AbortController();
|
||||
let settleResolve!: () => void;
|
||||
const settled = new Promise<void>((resolve) => {
|
||||
settleResolve = resolve;
|
||||
});
|
||||
const entry: FakeTaskEntry = { task, controller, output: '', status: 'running', settled };
|
||||
const sink: AgentTaskSink = {
|
||||
signal: controller.signal,
|
||||
appendOutput: (chunk) => {
|
||||
entry.output += chunk;
|
||||
},
|
||||
settle: (settlement) => {
|
||||
entry.status = settlement.status;
|
||||
entry.stopReason = settlement.stopReason;
|
||||
settleResolve();
|
||||
return Promise.resolve(true);
|
||||
},
|
||||
};
|
||||
entries.set(taskId, entry);
|
||||
void Promise.resolve(task.start(sink)).catch(() => {});
|
||||
return taskId;
|
||||
});
|
||||
const tasks = {
|
||||
registerTask,
|
||||
getTask: (taskId: string) => {
|
||||
const entry = entries.get(taskId);
|
||||
if (entry === undefined) return undefined;
|
||||
const base: AgentTaskInfoBase = {
|
||||
taskId,
|
||||
description: entry.task.description,
|
||||
status: entry.status,
|
||||
startedAt: 0,
|
||||
endedAt: null,
|
||||
stopReason: entry.stopReason,
|
||||
};
|
||||
return entry.task.toInfo(base);
|
||||
},
|
||||
} as unknown as IAgentTaskService;
|
||||
return {
|
||||
tasks,
|
||||
registerTask,
|
||||
entry: (taskId) => {
|
||||
const entry = entries.get(taskId);
|
||||
if (entry === undefined) throw new Error(`unknown task ${taskId}`);
|
||||
return entry;
|
||||
},
|
||||
kill: (taskId) => {
|
||||
entries.get(taskId)?.controller.abort();
|
||||
},
|
||||
wait: async (taskId) => entries.get(taskId)?.settled,
|
||||
};
|
||||
}
|
||||
|
||||
function makeTool(
|
||||
options: {
|
||||
readonly request?: (
|
||||
req: QuestionRequest,
|
||||
requestOptions?: { readonly signal?: AbortSignal },
|
||||
) => Promise<QuestionResult>;
|
||||
} = {},
|
||||
): {
|
||||
readonly tool: AskUserQuestionTool;
|
||||
readonly request: ReturnType<typeof vi.fn>;
|
||||
readonly telemetryTrack: ReturnType<typeof vi.fn>;
|
||||
readonly taskService: ReturnType<typeof createFakeTaskService>;
|
||||
} {
|
||||
const request = vi.fn(options.request ?? (async () => ({ Postgres: true }) as QuestionResult));
|
||||
const telemetryTrack = vi.fn();
|
||||
const taskService = createFakeTaskService();
|
||||
const question = { request } as unknown as ISessionQuestionService;
|
||||
const telemetry = { track: telemetryTrack } as unknown as ITelemetryService;
|
||||
const tool = new AskUserQuestionTool(question, taskService.tasks, telemetry);
|
||||
return { tool, request, telemetryTrack, taskService };
|
||||
}
|
||||
|
||||
describe('AskUserQuestionTool', () => {
|
||||
it('exposes current metadata and schema', () => {
|
||||
const { tool } = makeTool();
|
||||
|
||||
expect(tool.name).toBe('AskUserQuestion');
|
||||
expect(tool.description).toContain('structured options');
|
||||
expect(tool.parameters).toMatchObject({
|
||||
type: 'object',
|
||||
properties: { questions: { type: 'array' } },
|
||||
});
|
||||
expect(AskUserQuestionInputSchema.safeParse(input()).success).toBe(true);
|
||||
expect(AskUserQuestionInputSchema.safeParse({ questions: [] }).success).toBe(false);
|
||||
expect(
|
||||
AskUserQuestionInputSchema.safeParse(
|
||||
input({
|
||||
options: [{ label: 'Only one', description: 'Not enough choices' }],
|
||||
}),
|
||||
).success,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('documents the answers shape and the uniqueness requirement to the model', () => {
|
||||
const { tool } = makeTool();
|
||||
|
||||
expect(tool.description).toContain('must be unique across the call');
|
||||
expect(tool.description).toContain('keyed by question text');
|
||||
});
|
||||
|
||||
it('tells the model not to poll a pending background question', () => {
|
||||
const { tool } = makeTool();
|
||||
const paramsJson = JSON.stringify(tool.parameters);
|
||||
|
||||
expect(paramsJson).toContain('do not poll with TaskOutput');
|
||||
expect(paramsJson).not.toContain('Use TaskOutput to read the answer later');
|
||||
});
|
||||
|
||||
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, request } = 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(request).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, request } = 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(request).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, request } = makeTool();
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_cross_label',
|
||||
args,
|
||||
signal,
|
||||
});
|
||||
expect(result.isError).toBe(false);
|
||||
expect(request).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('rejects duplicate questions on the background path before starting a task', async () => {
|
||||
const { tool, request, taskService } = makeTool();
|
||||
|
||||
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(request).not.toHaveBeenCalled();
|
||||
expect(taskService.registerTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('describes the no-Other rule on options and the Recommended hint on label', () => {
|
||||
const { tool } = makeTool();
|
||||
const params = tool.parameters as {
|
||||
properties: {
|
||||
questions: {
|
||||
items: {
|
||||
properties: {
|
||||
options: {
|
||||
description?: string;
|
||||
items: { properties: { label: { description?: string } } };
|
||||
};
|
||||
};
|
||||
};
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
const optionsSchema = params.properties.questions.items.properties.options;
|
||||
expect(optionsSchema.description).toContain("Do NOT include an 'Other' option");
|
||||
expect(optionsSchema.description).toContain('the system adds one automatically');
|
||||
|
||||
const labelSchema = optionsSchema.items.properties.label;
|
||||
expect(labelSchema.description).toContain("append '(Recommended)'");
|
||||
});
|
||||
|
||||
it('always builds the background-question schema', () => {
|
||||
const { tool } = makeTool();
|
||||
|
||||
expect(tool.description).toContain('Set background=true');
|
||||
expect(JSON.stringify(tool.parameters)).toContain('background');
|
||||
});
|
||||
|
||||
it('dispatches questions through the session question service', async () => {
|
||||
const { tool, request, telemetryTrack } = makeTool();
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_question',
|
||||
args: input({ multi_select: true }),
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(false);
|
||||
expect(result.output).toBe(JSON.stringify({ answers: { Postgres: true } }));
|
||||
expect(request).toHaveBeenCalledWith(
|
||||
{
|
||||
turnId: 0,
|
||||
toolCallId: 'call_question',
|
||||
questions: [
|
||||
{
|
||||
question: 'Which database?',
|
||||
header: 'Storage',
|
||||
options: [
|
||||
{ label: 'Postgres', description: 'Relational storage' },
|
||||
{ label: 'SQLite', description: 'Embedded storage' },
|
||||
],
|
||||
multiSelect: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
{ signal },
|
||||
);
|
||||
expect(telemetryTrack).toHaveBeenCalledWith('question_answered', {
|
||||
answered: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it('passes empty headers and option descriptions through verbatim (v1 wire parity)', async () => {
|
||||
const { tool, request } = makeTool();
|
||||
|
||||
await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_empty_fields',
|
||||
args: input({
|
||||
header: '',
|
||||
options: [
|
||||
{ label: 'Postgres', description: '' },
|
||||
{ label: 'SQLite', description: '' },
|
||||
],
|
||||
}),
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(request).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
questions: [
|
||||
expect.objectContaining({
|
||||
header: '',
|
||||
options: [
|
||||
{ label: 'Postgres', description: '' },
|
||||
{ label: 'SQLite', description: '' },
|
||||
],
|
||||
}),
|
||||
],
|
||||
}),
|
||||
{ signal },
|
||||
);
|
||||
});
|
||||
|
||||
it('tracks the structured question answer method without leaking it into output', async () => {
|
||||
const { tool, telemetryTrack } = makeTool({
|
||||
request: async () => ({
|
||||
answers: { 'Which database?': 'SQLite' },
|
||||
method: 'number_key',
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_question',
|
||||
args: input(),
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ isError: false });
|
||||
expect(result.output).toBe(JSON.stringify({ answers: { 'Which database?': 'SQLite' } }));
|
||||
expect(telemetryTrack).toHaveBeenCalledWith('question_answered', {
|
||||
answered: 1,
|
||||
method: 'number_key',
|
||||
});
|
||||
});
|
||||
|
||||
it('starts a background question task and stores the eventual answer in task output', async () => {
|
||||
let resolveQuestion!: (result: QuestionResult) => void;
|
||||
const questionResult = new Promise<QuestionResult>((resolve) => {
|
||||
resolveQuestion = resolve;
|
||||
});
|
||||
const { tool, taskService, telemetryTrack } = makeTool({
|
||||
request: async () => questionResult,
|
||||
});
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_background_question',
|
||||
args: { ...input(), background: true },
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(false);
|
||||
expect(result.output).toContain('task_id: question-');
|
||||
const outputText = typeof result.output === 'string' ? result.output : '';
|
||||
const taskId = /task_id: (?<taskId>question-[0-9a-z]{8})/.exec(outputText)?.groups?.['taskId'];
|
||||
expect(taskId).toBeDefined();
|
||||
expect(taskService.tasks.getTask(taskId!)).toMatchObject({
|
||||
kind: 'question',
|
||||
status: 'running',
|
||||
questionCount: 1,
|
||||
toolCallId: 'call_background_question',
|
||||
});
|
||||
|
||||
resolveQuestion({ answers: { 'Which database?': 'SQLite' }, method: 'enter' });
|
||||
await taskService.wait(taskId!);
|
||||
|
||||
expect(taskService.tasks.getTask(taskId!)).toMatchObject({ status: 'completed' });
|
||||
expect(taskService.entry(taskId!).output).toBe(
|
||||
JSON.stringify({ answers: { 'Which database?': 'SQLite' } }),
|
||||
);
|
||||
expect(telemetryTrack).toHaveBeenCalledWith('question_answered', {
|
||||
answered: 1,
|
||||
method: 'enter',
|
||||
});
|
||||
});
|
||||
|
||||
it('dismisses the underlying question when a background task is killed (v1 broker semantics)', async () => {
|
||||
const { tool, request, taskService, telemetryTrack } = makeTool({
|
||||
// Emulates the real question service: an abort dismisses the parked
|
||||
// entry and resolves with `null` instead of rejecting.
|
||||
request: async (_req, requestOptions) =>
|
||||
new Promise<QuestionResult>((resolve) => {
|
||||
requestOptions?.signal?.addEventListener('abort', () => resolve(null), {
|
||||
once: true,
|
||||
});
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_bg_kill',
|
||||
args: { ...input(), background: true },
|
||||
signal,
|
||||
});
|
||||
const outputText = typeof result.output === 'string' ? result.output : '';
|
||||
const taskId = /task_id: (?<taskId>question-[0-9a-z]{8})/.exec(outputText)?.groups?.['taskId'];
|
||||
expect(taskId).toBeDefined();
|
||||
|
||||
// The task signal must reach the question service so a TaskStop actually
|
||||
// cancels the pending question instead of leaking it.
|
||||
const requestOptions = request.mock.calls[0]?.[1] as { signal?: AbortSignal } | undefined;
|
||||
expect(requestOptions?.signal).toBeDefined();
|
||||
|
||||
taskService.kill(taskId!);
|
||||
await taskService.wait(taskId!);
|
||||
// The dismissal resolves the question thunk with the dismissed answers
|
||||
// payload, so the task itself completes — matching the v1 runtime.
|
||||
expect(taskService.entry(taskId!).status).toBe('completed');
|
||||
expect(taskService.entry(taskId!).output).toBe(
|
||||
JSON.stringify({ answers: {}, note: 'User dismissed the question without answering.' }),
|
||||
);
|
||||
expect(telemetryTrack).toHaveBeenCalledWith('question_dismissed');
|
||||
});
|
||||
|
||||
it('returns a dismissed message when every question is dismissed', async () => {
|
||||
const { tool, telemetryTrack } = makeTool({ request: async () => null });
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_question',
|
||||
args: {
|
||||
questions: [input().questions[0]!, input({ question: 'Which cache?' }).questions[0]!],
|
||||
},
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ isError: false });
|
||||
expect(result.output).toContain('dismissed');
|
||||
expect(result.output).toContain('answers');
|
||||
expect(telemetryTrack).toHaveBeenCalledWith('question_dismissed');
|
||||
});
|
||||
|
||||
it('resolves question service error responses as dismissed answers', async () => {
|
||||
const { tool } = makeTool({
|
||||
request: async () => {
|
||||
throw new KimiError(CoreErrors.codes.INTERNAL, 'question broker error');
|
||||
},
|
||||
});
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_question',
|
||||
args: input(),
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ isError: false });
|
||||
expect(result.output).toContain('dismissed');
|
||||
expect(typeof result.output).toBe('string');
|
||||
const output = typeof result.output === 'string' ? result.output : '';
|
||||
expect(JSON.parse(output)).toEqual({
|
||||
answers: {},
|
||||
note: 'User dismissed the question without answering.',
|
||||
});
|
||||
expect(result.output).not.toContain('Do NOT call this tool again');
|
||||
});
|
||||
|
||||
it('propagates aborts while waiting for the question service', async () => {
|
||||
const controller = new AbortController();
|
||||
const { tool } = makeTool({
|
||||
request: async (_req, requestOptions) =>
|
||||
new Promise<QuestionResult>((_resolve, reject) => {
|
||||
requestOptions?.signal?.addEventListener(
|
||||
'abort',
|
||||
() => {
|
||||
const error = new Error('Aborted');
|
||||
error.name = 'AbortError';
|
||||
reject(error);
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
}),
|
||||
});
|
||||
|
||||
const result = executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'call_question',
|
||||
args: input(),
|
||||
signal: controller.signal,
|
||||
});
|
||||
controller.abort();
|
||||
|
||||
await expect(result).rejects.toHaveProperty('name', 'AbortError');
|
||||
});
|
||||
|
||||
it('returns a distinct hard error when the host signals unsupported', async () => {
|
||||
const { tool } = makeTool({
|
||||
request: async () => {
|
||||
throw new KimiError(
|
||||
CoreErrors.codes.NOT_IMPLEMENTED,
|
||||
'Client does not support questions',
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
const result = await executeTool(tool, {
|
||||
turnId: 0,
|
||||
toolCallId: 'tc-ask-unsupported',
|
||||
args: input(),
|
||||
signal,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ isError: true });
|
||||
expect(result.output).toContain('connected client');
|
||||
expect(result.output).toContain('does not support interactive questions');
|
||||
expect(result.output).toContain('Do NOT call this tool again');
|
||||
expect(result.output).toContain('Ask the user directly in your text response instead');
|
||||
});
|
||||
});
|
||||
|
|
@ -35,8 +35,10 @@
|
|||
* **Anti-corruption**: this is the single protocol↔in-process adapter for
|
||||
* questions. The v2 domain stores the in-process `QuestionRequest` (camelCase,
|
||||
* options without ids); the wire shape (snake_case, synthesized item/option
|
||||
* ids, `expires_at`, 5-kind answer union) is derived here. No `agent-core`
|
||||
* (v1) imports.
|
||||
* ids, 5-kind answer union) is derived here. On resolve, wire
|
||||
* ids are translated back to question text / option labels (reading the
|
||||
* pending request before it settles) so the flattened record the model sees
|
||||
* is self-explanatory. No `agent-core` (v1) imports.
|
||||
*/
|
||||
|
||||
import {
|
||||
|
|
@ -100,13 +102,6 @@ const tailParamsSchema = z.object({
|
|||
|
||||
const detailsSchema = z.array(z.object({ path: z.string(), message: z.string() }));
|
||||
|
||||
/**
|
||||
* Wire `expires_at` horizon: v2 interactions never expire, so we emit the v1
|
||||
* broker's default timeout (60s) as a stable derived value, because the wire
|
||||
* schema requires an `expires_at`.
|
||||
*/
|
||||
const QUESTION_EXPIRY_MS = 60_000;
|
||||
|
||||
export function registerQuestionsRoutes(app: QuestionRouteHost, core: Scope): void {
|
||||
const listRoute = defineRoute(
|
||||
{
|
||||
|
|
@ -182,9 +177,11 @@ export function registerQuestionsRoutes(app: QuestionRouteHost, core: Scope): vo
|
|||
}
|
||||
|
||||
const interaction = handle.accessor.get(ISessionInteractionService);
|
||||
const isPending = interaction.listPending('question').some((i) => i.id === questionId);
|
||||
const pendingInteraction = interaction
|
||||
.listPending('question')
|
||||
.find((i) => i.id === questionId);
|
||||
|
||||
if (!isPending) {
|
||||
if (pendingInteraction === undefined) {
|
||||
if (interaction.isRecentlyResolved(questionId)) {
|
||||
reply.send({
|
||||
code: ErrorCode.APPROVAL_ALREADY_RESOLVED, // 40902 — shared "already_resolved"
|
||||
|
|
@ -238,7 +235,13 @@ export function registerQuestionsRoutes(app: QuestionRouteHost, core: Scope): vo
|
|||
return;
|
||||
}
|
||||
|
||||
const result = toInProcessResponse(bodyParse.data);
|
||||
// The pending request must be projected BEFORE answer() settles (and
|
||||
// thereby drops) the kernel entry — its synthesized wire ids are the
|
||||
// lookup table for the id → text translation below.
|
||||
const result = toInProcessResponse(
|
||||
bodyParse.data,
|
||||
toWireQuestion(pendingInteraction, session_id),
|
||||
);
|
||||
questions.answer(questionId, result);
|
||||
reply.send(
|
||||
okEnvelope({ resolved: true as const, resolved_at: new Date().toISOString() }, req.id),
|
||||
|
|
@ -286,15 +289,14 @@ function buildItem(item: QuestionItem, itemIdx: number): ProtocolQuestionItem {
|
|||
export function toWireQuestion(
|
||||
interaction: Interaction,
|
||||
sessionId: string,
|
||||
): ProtocolQuestionRequest & { expires_at: string } {
|
||||
): ProtocolQuestionRequest {
|
||||
const req = interaction.payload as QuestionRequest;
|
||||
const createdAt = new Date(interaction.createdAt).toISOString();
|
||||
const out: ProtocolQuestionRequest & { expires_at: string } = {
|
||||
const out: ProtocolQuestionRequest = {
|
||||
question_id: interaction.id,
|
||||
session_id: sessionId,
|
||||
questions: req.questions.map((q, i) => buildItem(q, i)),
|
||||
created_at: createdAt,
|
||||
expires_at: new Date(interaction.createdAt + QUESTION_EXPIRY_MS).toISOString(),
|
||||
};
|
||||
if (req.turnId !== undefined) out.turn_id = req.turnId;
|
||||
if (req.toolCallId !== undefined) out.tool_call_id = req.toolCallId;
|
||||
|
|
@ -302,29 +304,60 @@ export function toWireQuestion(
|
|||
}
|
||||
|
||||
/**
|
||||
* Protocol REST response body → in-process `QuestionResponse`. Normalization
|
||||
* rules (SCHEMAS §6.4):
|
||||
* - single → option_id
|
||||
* - multi → option_ids.join(',')
|
||||
* Protocol REST response body → in-process `QuestionResponse`.
|
||||
*
|
||||
* 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 pending wire `request` (ported from v1's `toAgentCoreResponse`):
|
||||
* - 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.
|
||||
*/
|
||||
function toInProcessResponse(resp: ProtocolQuestionResponse): QuestionResult {
|
||||
function toInProcessResponse(
|
||||
resp: ProtocolQuestionResponse,
|
||||
request?: ProtocolQuestionRequest,
|
||||
): QuestionResult {
|
||||
const itemsById = new Map<string, ProtocolQuestionItem>();
|
||||
for (const item of request?.questions ?? []) {
|
||||
itemsById.set(item.id, item);
|
||||
}
|
||||
|
||||
const flattened: QuestionAnswers = {};
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -46,7 +46,6 @@ interface QuestionWire {
|
|||
tool_call_id?: string;
|
||||
questions: QuestionItemWire[];
|
||||
created_at: string;
|
||||
expires_at: string;
|
||||
}
|
||||
|
||||
interface ListWire {
|
||||
|
|
@ -163,7 +162,8 @@ describe('server-v2 /api/v1/sessions/{sid}/questions', () => {
|
|||
},
|
||||
]);
|
||||
expect(Number.isNaN(Date.parse(item.created_at))).toBe(false);
|
||||
expect(Number.isNaN(Date.parse(item.expires_at))).toBe(false);
|
||||
// v1 parity: the question wire shape carries no synthetic expiry.
|
||||
expect(item).not.toHaveProperty('expires_at');
|
||||
});
|
||||
|
||||
it('resolves a pending question', async () => {
|
||||
|
|
@ -193,11 +193,107 @@ describe('server-v2 /api/v1/sessions/{sid}/questions', () => {
|
|||
method: 'click', // protocol-only method; dropped on the in-process side
|
||||
});
|
||||
|
||||
// Wire ids are translated back to question text / option labels so the
|
||||
// record the model sees is self-explanatory (v1 parity: multi joins with
|
||||
// ', ' to match the TUI reverse-RPC path).
|
||||
await expect(resultPromise).resolves.toEqual({
|
||||
answers: { q_0: 'opt_0_0,opt_0_1' },
|
||||
answers: { 'Pick one': 'Yes, No' },
|
||||
});
|
||||
});
|
||||
|
||||
function makeTwoQuestionRequest(id: string): QuestionRequest {
|
||||
return {
|
||||
id,
|
||||
toolCallId: `tc-${id}`,
|
||||
questions: [
|
||||
{
|
||||
question: 'Which animal?',
|
||||
options: [{ label: 'Cat' }, { label: 'Dog' }],
|
||||
},
|
||||
{
|
||||
question: 'Which colors?',
|
||||
options: [{ label: 'Red' }, { label: 'Green' }, { label: 'Blue' }],
|
||||
multiSelect: true,
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
it('translates ids to text across single / other / multi_with_other kinds', async () => {
|
||||
const sid = await createSession();
|
||||
const single: Promise<QuestionResult> = questionService(sid).request(
|
||||
makeTwoQuestionRequest('q-t1'),
|
||||
);
|
||||
await postJson<ResolveWire>(`/api/v1/sessions/${sid}/questions/q-t1`, {
|
||||
answers: {
|
||||
q_0: { kind: 'single', option_id: 'opt_0_1' },
|
||||
q_1: {
|
||||
kind: 'multi_with_other',
|
||||
option_ids: ['opt_1_0', 'opt_1_1'],
|
||||
other_text: 'Custom',
|
||||
},
|
||||
},
|
||||
});
|
||||
await expect(single).resolves.toEqual({
|
||||
answers: { 'Which animal?': 'Dog', 'Which colors?': 'Red, Green, Custom' },
|
||||
});
|
||||
|
||||
const other: Promise<QuestionResult> = questionService(sid).request(
|
||||
makeTwoQuestionRequest('q-t2'),
|
||||
);
|
||||
await postJson<ResolveWire>(`/api/v1/sessions/${sid}/questions/q-t2`, {
|
||||
answers: {
|
||||
q_0: { kind: 'other', text: 'Hippopotamus' },
|
||||
q_1: { kind: 'skipped' },
|
||||
},
|
||||
});
|
||||
await expect(other).resolves.toEqual({
|
||||
answers: { 'Which animal?': 'Hippopotamus' },
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps unknown and cross-question option ids verbatim (stale client)', async () => {
|
||||
const sid = await createSession();
|
||||
const resultPromise: Promise<QuestionResult> = questionService(sid).request(
|
||||
makeTwoQuestionRequest('q-t3'),
|
||||
);
|
||||
|
||||
await postJson<ResolveWire>(`/api/v1/sessions/${sid}/questions/q-t3`, {
|
||||
answers: {
|
||||
// opt_0_9 does not exist; q_9 is an unknown question id.
|
||||
q_0: { kind: 'single', option_id: 'opt_0_9' },
|
||||
q_9: { kind: 'single', option_id: 'opt_9_0' },
|
||||
// opt_0_0 belongs to question 0 — never offered for question 1, so it
|
||||
// must NOT be resolved to 'Cat'.
|
||||
q_1: { kind: 'multi', option_ids: ['opt_1_0', 'opt_0_0'] },
|
||||
},
|
||||
});
|
||||
|
||||
await expect(resultPromise).resolves.toEqual({
|
||||
answers: {
|
||||
'Which animal?': 'opt_0_9',
|
||||
q_9: 'opt_9_0',
|
||||
'Which colors?': 'Red, opt_0_0',
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('produces an empty answers record when all questions are skipped (not a dismissal)', async () => {
|
||||
const sid = await createSession();
|
||||
const resultPromise: Promise<QuestionResult> = questionService(sid).request(
|
||||
makeTwoQuestionRequest('q-t4'),
|
||||
);
|
||||
|
||||
await postJson<ResolveWire>(`/api/v1/sessions/${sid}/questions/q-t4`, {
|
||||
answers: {
|
||||
q_0: { kind: 'skipped' },
|
||||
q_1: { kind: 'skipped' },
|
||||
},
|
||||
});
|
||||
|
||||
await expect(resultPromise).resolves.toEqual({ answers: {} });
|
||||
});
|
||||
|
||||
it('dismisses a pending question', async () => {
|
||||
const sid = await createSession();
|
||||
const resultPromise: Promise<QuestionResult> = questionService(sid).request(makeRequest('q-4'));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue