diff --git a/changelog.d/2771-ask-user-two-to-four-answers.fixed.md b/changelog.d/2771-ask-user-two-to-four-answers.fixed.md new file mode 100644 index 000000000..ec0118025 --- /dev/null +++ b/changelog.d/2771-ask-user-two-to-four-answers.fixed.md @@ -0,0 +1 @@ +`ask_user` now enforces the two-to-four answers its own card promises: a question with one option, or with more than four, is refused with `Give two to four answers.` and no card is stored, instead of accepting a single "choice" or silently dropping a seventh option (labels dropped as empty are not counted, so five given with two blanks is a three-button card and passes). Two to four options produce the same card and the same answer string as before. See [#2771](https://github.com/ThinkFlowLab/FlyBot/issues/2771). diff --git a/core/src/host/tools.js b/core/src/host/tools.js index d0849ca8d..9df72da2a 100644 --- a/core/src/host/tools.js +++ b/core/src/host/tools.js @@ -650,8 +650,10 @@ export function install(rt) { ...(option.description ? { description: String(option.description).trim() } : {}), })) .filter(option => option.label !== '') - .slice(0, 6) - if (options.length === 0) return 'Give at least one option.' + // The card shows two to four answers and says so in its own parameter description. A fifth + // is not cut away silently and a lone one is not a choice, so both ends are refused here, + // on the labels that survived the filter above (#2771). + if (options.length < 2 || options.length > 4) return 'Give two to four answers.' // Work handed out from a group (turns.js): the user handed it out there, so a question // about it is asked there too, never in the Bot's own chat (#550). const handed = rt.handedFromGroup(id) diff --git a/core/test/questions.test.js b/core/test/questions.test.js index 82c4ff3fa..8ba555a1a 100644 --- a/core/test/questions.test.js +++ b/core/test/questions.test.js @@ -45,20 +45,27 @@ async function start() { if (listener.length >= 2) listener({ id }, { type, seq: ++seq, data }) } } - const ask = (sessionId, callId) => ctx.registeredTools.get('ask_user').execute( - { question: 'Which name?', options: [{ label: 'Lookout' }, { label: 'Scout' }] }, + const askOptions = (sessionId, options, callId) => ctx.registeredTools.get('ask_user').execute( + { question: 'Which name?', options }, { agent: { id: sessionId }, callId, concludeTurn() {} }, ) + const ask = (sessionId, callId) => askOptions(sessionId, [{ label: 'Lookout' }, { label: 'Scout' }], callId) const questions = async () => (await (await route.fetch(new Request('http://localhost/api/bot', { method: 'POST', body: JSON.stringify({ endpoint: 'state', payload: {} }), }))).json()).value.questions // Loading once fills the Session-owner indexes turns.js filters events by. await ctx.registeredTools.get('list_bots').execute({}, { agent: { id: 'chief' } }) - return { emit, ask, questions, stop: async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) } } + return { emit, ask, askOptions, questions, stop: async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) } } } const text = value => [{ type: 'text', text: value }] +const RULE = 'Give two to four answers.' +const CARD = 'The card is on screen; the user\'s answer arrives as their next message.' +const labels = (...names) => names.map(label => ({ label })) +// Everything the client renders, without the card's own identity. +const cardJson = card => JSON.stringify(card, (key, value) => (key === 'id' || key === 'askedAt' ? undefined : value)) + test('a card asked in a hidden Bot-to-Bot turn goes to the dock, without its call', async (t) => { const { emit, ask, questions, stop } = await start() t.after(stop) @@ -85,3 +92,46 @@ test('a card asked in a visible turn stays at its call', async (t) => { await ask('writer', 'call-3') assert.equal((await questions()).writer.callId, 'call-3') }) + +// #2771: the card's own contract says two to four answers, and the host enforces exactly that. +test('#2771: one option is refused with the rule and stores no card', async (t) => { + const { askOptions, questions, stop } = await start() + t.after(stop) + assert.equal(await askOptions('writer', labels('Only'), 'call-1'), RULE) + assert.equal((await questions()).writer, undefined) +}) + +test('#2771: six options are refused with the rule and store no card', async (t) => { + const { askOptions, questions, stop } = await start() + t.after(stop) + assert.equal(await askOptions('writer', labels('A', 'B', 'C', 'D', 'E', 'F'), 'call-1'), RULE) + assert.equal((await questions()).writer, undefined) +}) + +test('#2771: seven options are refused, never cut to six', async (t) => { + const { askOptions, questions, stop } = await start() + t.after(stop) + assert.equal(await askOptions('writer', labels('A', 'B', 'C', 'D', 'E', 'F', 'G'), 'call-1'), RULE) + assert.equal((await questions()).writer, undefined) +}) + +test('#2771: two and four options keep the card and the answer string, byte for byte', async (t) => { + const { askOptions, questions, stop } = await start() + t.after(stop) + assert.equal(await askOptions('writer', labels('Lookout', 'Scout'), 'call-2'), CARD) + assert.equal(cardJson((await questions()).writer), + '{"callId":"call-2","question":"Which name?","options":[{"label":"Lookout"},{"label":"Scout"}],"multiSelect":false}') + assert.equal(await askOptions('writer', labels('A', 'B', 'C', 'D'), 'call-4'), CARD) + assert.equal(cardJson((await questions()).writer), + '{"callId":"call-4","question":"Which name?","options":[{"label":"A"},{"label":"B"},{"label":"C"},{"label":"D"}],"multiSelect":false}') +}) + +// The count is taken after the empty-label filter: five given with two blanks is three buttons. +test('#2771: labels dropped as empty do not count, so five given with two blanks is three and passes', async (t) => { + const { askOptions, questions, stop } = await start() + t.after(stop) + const options = [{ label: 'A' }, { label: ' ' }, { label: 'B' }, { label: '' }, { label: 'C' }] + assert.equal(await askOptions('writer', options, 'call-3'), CARD) + assert.equal(cardJson((await questions()).writer), + '{"callId":"call-3","question":"Which name?","options":[{"label":"A"},{"label":"B"},{"label":"C"}],"multiSelect":false}') +}) diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 6d2af4b64..ec6678576 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -2319,10 +2319,16 @@ diff --git a/src/host/tools.js b/src/host/tools.js } catch (failure) { return `Could not post in "${room.name}": ${failure.message}` } -@@ -445,6 +445,10 @@ export function install(rt) { +@@ -443,8 +443,14 @@ export function install(rt) { + ...(option.description ? { description: String(option.description).trim() } : {}), + })) .filter(option => option.label !== '') - .slice(0, 6) - if (options.length === 0) return 'Give at least one option.' +- .slice(0, 6) +- if (options.length === 0) return 'Give at least one option.' ++ // The card shows two to four answers and says so in its own parameter description. A fifth ++ // is not cut away silently and a lone one is not a choice, so both ends are refused here, ++ // on the labels that survived the filter above (#2771). ++ if (options.length < 2 || options.length > 4) return 'Give two to four answers.' + // Work handed out from a group (turns.js): the user handed it out there, so a question + // about it is asked there too, never in the Bot's own chat (#550). + const handed = rt.handedFromGroup(id) @@ -2691,6 +2697,87 @@ diff --git a/test/legacy-state.test.js b/test/legacy-state.test.js assert.equal(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).prefs.theme, 'mono') }) +diff --git a/test/questions.test.js b/test/questions.test.js +--- a/test/questions.test.js ++++ b/test/questions.test.js +@@ -45,20 +45,27 @@ async function start() { + if (listener.length >= 2) listener({ id }, { type, seq: ++seq, data }) + } + } +- const ask = (sessionId, callId) => ctx.registeredTools.get('ask_user').execute( +- { question: 'Which name?', options: [{ label: 'Lookout' }, { label: 'Scout' }] }, ++ const askOptions = (sessionId, options, callId) => ctx.registeredTools.get('ask_user').execute( ++ { question: 'Which name?', options }, + { agent: { id: sessionId }, callId, concludeTurn() {} }, + ) ++ const ask = (sessionId, callId) => askOptions(sessionId, [{ label: 'Lookout' }, { label: 'Scout' }], callId) + const questions = async () => (await (await route.fetch(new Request('http://localhost/api/bot', { + method: 'POST', body: JSON.stringify({ endpoint: 'state', payload: {} }), + }))).json()).value.questions + // Loading once fills the Session-owner indexes turns.js filters events by. + await ctx.registeredTools.get('list_bots').execute({}, { agent: { id: 'chief' } }) +- return { emit, ask, questions, stop: async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) } } ++ return { emit, ask, askOptions, questions, stop: async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) } } + } + + const text = value => [{ type: 'text', text: value }] + ++const RULE = 'Give two to four answers.' ++const CARD = 'The card is on screen; the user\'s answer arrives as their next message.' ++const labels = (...names) => names.map(label => ({ label })) ++// Everything the client renders, without the card's own identity. ++const cardJson = card => JSON.stringify(card, (key, value) => (key === 'id' || key === 'askedAt' ? undefined : value)) ++ + test('a card asked in a hidden Bot-to-Bot turn goes to the dock, without its call', async (t) => { + const { emit, ask, questions, stop } = await start() + t.after(stop) +@@ -85,3 +85,46 @@ test('a card asked in a visible turn stays at its call', async (t) => { + await ask('writer', 'call-3') + assert.equal((await questions()).writer.callId, 'call-3') + }) ++ ++// #2771: the card's own contract says two to four answers, and the host enforces exactly that. ++test('#2771: one option is refused with the rule and stores no card', async (t) => { ++ const { askOptions, questions, stop } = await start() ++ t.after(stop) ++ assert.equal(await askOptions('writer', labels('Only'), 'call-1'), RULE) ++ assert.equal((await questions()).writer, undefined) ++}) ++ ++test('#2771: six options are refused with the rule and store no card', async (t) => { ++ const { askOptions, questions, stop } = await start() ++ t.after(stop) ++ assert.equal(await askOptions('writer', labels('A', 'B', 'C', 'D', 'E', 'F'), 'call-1'), RULE) ++ assert.equal((await questions()).writer, undefined) ++}) ++ ++test('#2771: seven options are refused, never cut to six', async (t) => { ++ const { askOptions, questions, stop } = await start() ++ t.after(stop) ++ assert.equal(await askOptions('writer', labels('A', 'B', 'C', 'D', 'E', 'F', 'G'), 'call-1'), RULE) ++ assert.equal((await questions()).writer, undefined) ++}) ++ ++test('#2771: two and four options keep the card and the answer string, byte for byte', async (t) => { ++ const { askOptions, questions, stop } = await start() ++ t.after(stop) ++ assert.equal(await askOptions('writer', labels('Lookout', 'Scout'), 'call-2'), CARD) ++ assert.equal(cardJson((await questions()).writer), ++ '{"callId":"call-2","question":"Which name?","options":[{"label":"Lookout"},{"label":"Scout"}],"multiSelect":false}') ++ assert.equal(await askOptions('writer', labels('A', 'B', 'C', 'D'), 'call-4'), CARD) ++ assert.equal(cardJson((await questions()).writer), ++ '{"callId":"call-4","question":"Which name?","options":[{"label":"A"},{"label":"B"},{"label":"C"},{"label":"D"}],"multiSelect":false}') ++}) ++ ++// The count is taken after the empty-label filter: five given with two blanks is three buttons. ++test('#2771: labels dropped as empty do not count, so five given with two blanks is three and passes', async (t) => { ++ const { askOptions, questions, stop } = await start() ++ t.after(stop) ++ const options = [{ label: 'A' }, { label: ' ' }, { label: 'B' }, { label: '' }, { label: 'C' }] ++ assert.equal(await askOptions('writer', options, 'call-3'), CARD) ++ assert.equal(cardJson((await questions()).writer), ++ '{"callId":"call-3","question":"Which name?","options":[{"label":"A"},{"label":"B"},{"label":"C"}],"multiSelect":false}') ++}) diff --git a/test/update-bot.test.js b/test/update-bot.test.js --- a/test/update-bot.test.js +++ b/test/update-bot.test.js diff --git a/patches/ds-bot/topics.json b/patches/ds-bot/topics.json index f4eda696f..c8b6d644b 100644 --- a/patches/ds-bot/topics.json +++ b/patches/ds-bot/topics.json @@ -70,7 +70,8 @@ "test/fake-ctx.js", "test/blocking-question.test.js", "test/update-bot.test.js", - "test/legacy-state.test.js" + "test/legacy-state.test.js", + "test/questions.test.js" ], "0003-flybot-api-client": [ "src/client/index.js",