fix(host): refuse an ask_user card outside two to four answers (#2771)

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).

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)

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}')
+})

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

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",