fix(team): fix AskUserQuestion edge cases in ToolApprovalSheet
- Option keys now include question index prefix ("qi:label") to prevent
cross-question collisions when multiple questions share option labels
- Single-select clears only options from the same question, not all
- answersMessage built as per-question JSON map instead of flat join
- Backend parses JSON answers and maps to correct questions
- Fallback label "Option N" for options with undefined label
This commit is contained in:
parent
75a36b5cd2
commit
11506c6ea8
2 changed files with 37 additions and 12 deletions
|
|
@ -6043,17 +6043,21 @@ export class TeamProvisioningService {
|
|||
// IMPORTANT: request_id is NESTED inside response, NOT top-level
|
||||
// (asymmetry with control_request — confirmed by Python SDK, Elixir SDK and issue #29991)
|
||||
const allowResponse: Record<string, unknown> = { behavior: 'allow' };
|
||||
// For AskUserQuestion: pass user's answer via updatedInput so the CLI
|
||||
// can deliver it without re-prompting. Format follows --permission-prompt-tool spec.
|
||||
// For AskUserQuestion: pass user's answers via updatedInput so the CLI
|
||||
// can deliver them without re-prompting. Format follows --permission-prompt-tool spec.
|
||||
if (allow && message) {
|
||||
const pending = run.pendingApprovals.get(requestId);
|
||||
if (pending?.toolName === 'AskUserQuestion') {
|
||||
const questions = (pending.toolInput.questions as { question?: string }[]) ?? [];
|
||||
const answers: Record<string, string> = {};
|
||||
for (const q of questions) {
|
||||
if (q.question) answers[q.question] = message;
|
||||
try {
|
||||
const answers = JSON.parse(message) as Record<string, string>;
|
||||
allowResponse.updatedInput = { ...pending.toolInput, answers };
|
||||
} catch {
|
||||
// If message isn't JSON, use as-is for the first question
|
||||
const questions = (pending.toolInput.questions as { question?: string }[]) ?? [];
|
||||
const answers: Record<string, string> = {};
|
||||
if (questions[0]?.question) answers[questions[0].question] = message;
|
||||
allowResponse.updatedInput = { ...pending.toolInput, answers };
|
||||
}
|
||||
allowResponse.updatedInput = { ...pending.toolInput, answers };
|
||||
}
|
||||
}
|
||||
const response = allow
|
||||
|
|
|
|||
|
|
@ -170,10 +170,26 @@ export const ToolApprovalSheet: React.FC = () => {
|
|||
setDisabled(true);
|
||||
setError(null);
|
||||
|
||||
// For AskUserQuestion, build answers from selected options
|
||||
// For AskUserQuestion, build per-question answers from selected options
|
||||
// Key format in selectedOptions: "qi:label" — parse question index to map correctly
|
||||
const answersMessage =
|
||||
allow && current.toolName === 'AskUserQuestion' && selectedOptions.size > 0
|
||||
? Array.from(selectedOptions).join(', ')
|
||||
? (() => {
|
||||
const questions = Array.isArray(current.toolInput.questions)
|
||||
? (current.toolInput.questions as { question?: string }[])
|
||||
: [];
|
||||
const answersByQuestion: Record<string, string> = {};
|
||||
for (const key of selectedOptions) {
|
||||
const colonIdx = key.indexOf(':');
|
||||
if (colonIdx < 0) continue;
|
||||
const qi = parseInt(key.slice(0, colonIdx), 10);
|
||||
const label = key.slice(colonIdx + 1);
|
||||
const questionText = questions[qi]?.question ?? `Question ${qi + 1}`;
|
||||
const existing = answersByQuestion[questionText];
|
||||
answersByQuestion[questionText] = existing ? `${existing}, ${label}` : label;
|
||||
}
|
||||
return JSON.stringify(answersByQuestion);
|
||||
})()
|
||||
: undefined;
|
||||
|
||||
// Safety timeout — if IPC hangs (e.g. stdin.write callback never fires),
|
||||
|
|
@ -210,7 +226,12 @@ export const ToolApprovalSheet: React.FC = () => {
|
|||
|
||||
const handleOptionSelect = useCallback((label: string, multiSelect: boolean) => {
|
||||
setSelectedOptions((prev) => {
|
||||
const next = multiSelect ? new Set(prev) : new Set<string>();
|
||||
// For single-select: clear all options from the SAME question (same prefix)
|
||||
// Key format: "qi:label" where qi is the question index
|
||||
const prefix = label.split(':')[0] + ':';
|
||||
const next = multiSelect
|
||||
? new Set(prev)
|
||||
: new Set(Array.from(prev).filter((k) => !k.startsWith(prefix)));
|
||||
if (next.has(label)) {
|
||||
next.delete(label);
|
||||
} else {
|
||||
|
|
@ -495,7 +516,7 @@ const ToolInputPreview = ({
|
|||
{Array.isArray(q.options) && (
|
||||
<div className="space-y-1.5">
|
||||
{q.options.map((opt, oi) => {
|
||||
const optKey = opt.label ?? `opt-${oi}`;
|
||||
const optKey = `${qi}:${opt.label ?? `opt-${oi}`}`;
|
||||
const isSelected = selectedOptions?.has(optKey) ?? false;
|
||||
return (
|
||||
<button
|
||||
|
|
@ -525,7 +546,7 @@ const ToolInputPreview = ({
|
|||
className="text-xs font-medium"
|
||||
style={{ color: isSelected ? 'rgb(52, 211, 153)' : 'var(--color-text)' }}
|
||||
>
|
||||
{opt.label}
|
||||
{opt.label ?? `Option ${oi + 1}`}
|
||||
</span>
|
||||
{opt.description && (
|
||||
<p
|
||||
|
|
|
|||
Loading…
Reference in a new issue