Skip to content

Commit 00a9346

Browse files
Droyder7Droyder7
committed
fix(server): preserve choice whitespace, reject over-limit answers, decline on missing required fields
1 parent 3199621 commit 00a9346

2 files changed

Lines changed: 45 additions & 17 deletions

File tree

‎apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ describe("ACP elicitation question parsing and answer serialization", () => {
306306
]);
307307
});
308308

309-
it("skips empty choice options and does not emit blank values or labels", () => {
309+
it("skips empty choice options and does not emit blank values or labels while preserving nonblank whitespace", () => {
310310
const questions = parseElicitationQuestions({
311311
message: "Select option",
312312
requestedSchema: {
@@ -317,9 +317,13 @@ describe("ACP elicitation question parsing and answer serialization", () => {
317317
oneOf: [
318318
{ const: "", title: "" },
319319
{ const: " ", title: "Blank" },
320-
{ const: "valid", title: "Valid Option", description: "Good choice" },
320+
{ const: " valid ", title: "Valid Option", description: "Good choice" },
321321
],
322322
},
323+
enumChoice: {
324+
type: "string",
325+
enum: [" ", " option with space "],
326+
},
323327
},
324328
},
325329
});
@@ -329,7 +333,20 @@ describe("ACP elicitation question parsing and answer serialization", () => {
329333
id: "choice",
330334
header: "Question 1",
331335
question: "Select option",
332-
options: [{ label: "Valid Option", description: "Good choice", value: "valid" }],
336+
options: [{ label: "Valid Option", description: "Good choice", value: " valid " }],
337+
allowCustomAnswer: false,
338+
},
339+
{
340+
id: "enumChoice",
341+
header: "Question 2",
342+
question: "Select option",
343+
options: [
344+
{
345+
label: " option with space ",
346+
description: " option with space ",
347+
value: " option with space ",
348+
},
349+
],
333350
allowCustomAnswer: false,
334351
},
335352
]);
@@ -378,7 +395,8 @@ describe("ACP elicitation question parsing and answer serialization", () => {
378395
invalidPort: { type: "number" },
379396
tags: { type: "array" },
380397
singleTag: { type: "array" },
381-
limitedTags: { type: "array", maxItems: 2 },
398+
limitedTagsFail: { type: "array", maxItems: 2 },
399+
limitedTagsPass: { type: "array", maxItems: 2 },
382400
minTagsPass: { type: "array", minItems: 2 },
383401
minTagsFail: { type: "array", minItems: 2 },
384402
title: { type: "string" },
@@ -397,7 +415,8 @@ describe("ACP elicitation question parsing and answer serialization", () => {
397415
invalidPort: "not-a-number",
398416
tags: ["a", "b"],
399417
singleTag: "single",
400-
limitedTags: ["tag1", "tag2", "tag3"],
418+
limitedTagsFail: ["tag1", "tag2", "tag3"],
419+
limitedTagsPass: ["tag1", "tag2"],
401420
minTagsPass: ["tag1", "tag2"],
402421
minTagsFail: ["tag1"],
403422
title: "my-title",
@@ -414,7 +433,7 @@ describe("ACP elicitation question parsing and answer serialization", () => {
414433
boundedNumber: 25,
415434
tags: ["a", "b"],
416435
singleTag: ["single"],
417-
limitedTags: ["tag1", "tag2"],
436+
limitedTagsPass: ["tag1", "tag2"],
418437
minTagsPass: ["tag1", "tag2"],
419438
title: "my-title",
420439
fromArray: "first",
@@ -424,6 +443,7 @@ describe("ACP elicitation question parsing and answer serialization", () => {
424443
assert.isFalse("underMin" in content);
425444
assert.isFalse("overMax" in content);
426445
assert.isFalse("invalidPort" in content);
446+
assert.isFalse("limitedTagsFail" in content);
427447
assert.isFalse("minTagsFail" in content);
428448
assert.isFalse("unknownKey" in content);
429449
});

‎apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,8 +1113,8 @@ function parseChoiceOptions(
11131113
if (!entryRecord) continue;
11141114
const rawValue = entryRecord.const ?? entryRecord.value;
11151115
if (rawValue === undefined || rawValue === null) continue;
1116-
const valueStr = String(rawValue).trim();
1117-
if (valueStr.length === 0) continue;
1116+
const valueStr = String(rawValue);
1117+
if (valueStr.trim().length === 0) continue;
11181118
const rawLabel = entryRecord.title ?? entryRecord.label;
11191119
const labelStr = nonEmptyText(rawLabel, valueStr);
11201120
const rawDesc = entryRecord.description;
@@ -1130,8 +1130,8 @@ function parseEnumOptions(
11301130
const options: Array<{ label: string; description: string; value: string }> = [];
11311131
for (const entry of enumValues) {
11321132
if (typeof entry === "string" || typeof entry === "number" || typeof entry === "boolean") {
1133-
const str = String(entry).trim();
1134-
if (str.length > 0) {
1133+
const str = String(entry);
1134+
if (str.trim().length > 0) {
11351135
options.push({ label: str, description: str, value: str });
11361136
}
11371137
}
@@ -1247,9 +1247,8 @@ export function elicitationContent(
12471247
items = [rawValue];
12481248
}
12491249
if (typeof propSchema?.maxItems === "number" && items.length > propSchema.maxItems) {
1250-
items = items.slice(0, propSchema.maxItems);
1251-
}
1252-
if (typeof propSchema?.minItems === "number" && items.length < propSchema.minItems) {
1250+
// Discard answer if it exceeds the allowed maximum item count
1251+
} else if (typeof propSchema?.minItems === "number" && items.length < propSchema.minItems) {
12531252
// Discard answer if it fails the required minimum item count
12541253
} else {
12551254
content[key] = items;
@@ -5969,10 +5968,19 @@ export function makeAcpAdapterV2(
59695968
const response =
59705969
userInput.answers === null
59715970
? ({ action: "cancel" } as const)
5972-
: ({
5973-
action: "accept",
5974-
content: elicitationContent(userInput.answers, properties),
5975-
} as const);
5971+
: (() => {
5972+
const content = elicitationContent(userInput.answers, properties);
5973+
const requiredKeys = Array.isArray(requestedSchema?.required)
5974+
? requestedSchema.required.filter(
5975+
(item): item is string => typeof item === "string",
5976+
)
5977+
: [];
5978+
return requiredKeys.some(
5979+
(key) => !Object.prototype.hasOwnProperty.call(content, key),
5980+
)
5981+
? ({ action: "decline" } as const)
5982+
: ({ action: "accept", content } as const);
5983+
})();
59765984
yield* userInput.acknowledgeNativeResponse;
59775985
return response;
59785986
}),

0 commit comments

Comments
 (0)