openvibers/openvibe.bot Commits

Report delivery safety for machine commands (#396)

OP OpenVibers committed 2 days ago · 83c4ca8b43 · 4 files, +33 −2

Changes

ddiff --git a/server/api/v1.js b/server/api/v1.js
iindex 14e814a..e0866af 100644
---- a/server/api/v1.js
++++ b/server/api/v1.js
@@@ -639,7 +639,9 @@ function v1Router({ domain, apiAuth, limits, hub, jobs: jobService, config, subm
if (/^desk-/.test(named) && await domain.deskLeases.get(device.id, named)) fail(409, 'bot.desk_leased', 'that desk is held by an OpenVibe.Run session');
const principal = req.principal.kind === 'service' ? { ...req.principal, subject } : req.principal;
const result = await hub.machineCommand(device.id, principal, subject, { id: req.body?.id, kind: req.body?.kind, value: req.body?.value, ms: req.body?.ms });
-- if (result.result === 'refused') fail(commandRefusalStatus(result.code), result.code, result.reason || null);
++ if (result.result === 'refused') return http.sendProblem(res, commandRefusalStatus(result.code), result.code, {
++ detail: result.reason || null, ctx: req.ov, extra: { delivery: result.delivery, retry: result.retry },
++ });
res.json({ machine_id: device.id, ...result });
}));
// Who has a desk right now (Desks plan D4): an agent that asked its person to take over a step (a sign-in, a
ddiff --git a/server/command-delivery.js b/server/command-delivery.js
nnew file mode 100644
iindex 0000000..d289978
---- /dev/null
++++ b/server/command-delivery.js
@@@ -0,0 +1,18 @@
++'use strict';
++
++// A repeat is safe only when Bot knows it never sent the command. A timeout, an
++// in-flight id and a lost reply cannot prove that the machine did nothing.
++function withDelivery(result) {
++ const delivery = result.result === 'ack' || result.result === 'nack' ? 'delivered'
++ : result.result === 'refused' && result.code !== 'bot.command_pending' ? 'not_delivered'
++ : 'uncertain';
++ return {
++ ...result,
++ delivery,
++ retry: { safe: delivery === 'not_delivered', reason: delivery === 'not_delivered'
++ ? 'Bot refused the command before sending it' : delivery === 'uncertain'
++ ? 'The machine may already have run this command' : 'The machine already answered this command' },
++ };
++}
++
++module.exports = { withDelivery };
ddiff --git a/server/realtime.js b/server/realtime.js
iindex 948a80e..c50085f 100644
---- a/server/realtime.js
++++ b/server/realtime.js
@@@ -756,7 +756,8 @@ function createRealtime({ config, keys, userAuth, log = console, now = () => Dat
* → { id, result, code?, reason?, latency_ms?, data?, cached? }
*/
function machineCommand(deviceId, principal, subject, msg) {
-- return machineSend(deviceId, principal, subject, { id: msg.id, kind: msg.kind, value: msg.value, ms: msg.ms });
++ return machineSend(deviceId, principal, subject, { id: msg.id, kind: msg.kind, value: msg.value, ms: msg.ms })
++ .then(require('./command-delivery').withDelivery);
}
/**
* The live screen to the machine (api/v1.js and the web routes, the owner only): `value` is screen_stream's,
ddiff --git a/test/machine-computer.test.js b/test/machine-computer.test.js
iindex 5b8d023..7e9bb31 100644
---- a/test/machine-computer.test.js
++++ b/test/machine-computer.test.js
@@@ -6,8 +6,14 @@
// send these kinds; a machine whose plugin is off answers bot.computer_off.
const assert = require('assert');
const { boot, check, done } = require('./helpers/app');
++const { withDelivery } = require('../server/command-delivery');
(async () => {
++ await check('a timeout or in-flight repeat never claims a safe retry', async () => {
++ for (const r of [{ result: 'expired' }, { result: 'pending' }, { result: 'refused', code: 'bot.command_pending' }]) {
++ assert.deepStrictEqual([withDelivery(r).delivery, withDelivery(r).retry.safe], ['uncertain', false]);
++ }
++ });
const t = await boot({ env: { BOT_ROUTINE_TICK_MS: '0', BOT_WEBHOOK_TICK_MS: '0', BOT_PERSON_TURN_MS: '400' } });
const alex = t.network.newUser('alex');
const bob = t.network.newUser('bob');
@@@ -57,6 +63,7 @@ const { boot, check, done } = require('./helpers/app');
const r = await cmd(mine.deviceId, { id: 'a1', kind: 'exec', value: { argv: ['echo', 'hi'], stdin: 'secret input' } }, { user: alex });
assert.strictEqual(r.status, 200, r.text);
assert.deepStrictEqual([r.json.result, r.json.data.stdout, r.json.machine_id], ['ack', 'hi\n', mine.deviceId]);
++ assert.deepStrictEqual([r.json.delivery, r.json.retry.safe], ['delivered', false]);
const sent = seen.at(-1);
assert.deepStrictEqual([sent.kind, sent.operator.role, sent.value.argv], ['exec', 'owner', ['echo', 'hi']]);
assert.ok(sent.deadline_ms - Date.now() > 50000, 'a long reply window by default');
@@@ -64,6 +71,7 @@ const { boot, check, done } = require('./helpers/app');
assert.deepStrictEqual([ls.status, ls.json.data.path], [200, '/home/alex']);
const again = await cmd(mine.deviceId, { id: 'a1', kind: 'exec', value: { argv: ['echo', 'hi'] } }, { user: alex });
assert.deepStrictEqual([again.json.result, again.json.cached, again.json.data], ['ack', true, undefined], 'a repeated id: its result, never the data again');
++ assert.deepStrictEqual([again.json.delivery, again.json.retry.safe], ['delivered', false]);
});
await check('an answer larger than 64 KB comes through; a refusal on the machine says why', async () => {
@@@ -71,6 +79,7 @@ const { boot, check, done } = require('./helpers/app');
assert.deepStrictEqual([big.status, big.json.data.stdout.length], [200, 200 * 1024]);
const nack = await cmd(mine.deviceId, { kind: 'screen', value: {} }, { user: alex });
assert.deepStrictEqual([nack.status, nack.json.result, nack.json.reason], [200, 'nack', 'this machine allows observing only']);
++ assert.deepStrictEqual([nack.json.delivery, nack.json.retry.safe], ['delivered', false], 'a machine refusal proves delivery');
});
await check('mouse and keyboard reach the machine as they were asked; a key is a key, never a command line', async () => {
@@@ -300,6 +309,7 @@ const { boot, check, done } = require('./helpers/app');
const off = await machine(bob, null);
const r = await cmd(off.deviceId, { kind: 'fs', value: { op: 'list', path: '/' } }, { user: bob });
assert.deepStrictEqual([r.status, r.json.code], [409, 'bot.computer_off']);
++ assert.deepStrictEqual([r.json.delivery, r.json.retry.safe], ['not_delivered', true]);
off.c.close();
});