Repository: endojs/endo
Pull request #2227 (2024-04-19 00:22:30)

feat(cli,daemon): Add `move` CLI command
Adds a command `move` corresponding to the daemon directory's `move()`, with pet name path support. Replaces the existing `rename` command, which had been broken for some time.

Also ensures that the directory's `move()` method immediately deletes the "from" name mapping. Adds a test for the same. This mapping was already removed from disk and would not persist after restarts.

kriskowal: The design space for `move` interacts with the representation of identifiers in the pet dæmon network. Moving should be approximately equivalnt to `E(powers).locate(...toPath).write(toName, E(powers).locate(...fromPath))`. We currently use `identify` so this operation fails to introduce the addresses of the node housing the `from` entity to the node housing the `to` hub/directory/agent.

I’m imagining that `locate` and `write` will conspire at the level of a pet-sitter to take a “pet locator” and divide its (overlapping) parts between a “pet id” and “peer locator”, submitting the “pet id” to the underlying “pet store” and submitting the “peer locator” to the node’s “peer store”. So `locate` would combine these components on-the-fly and `write` would separate them on-the-fly. I think that would allow a reference to move from one node to another robustly.

Patch:
diff --git a/packages/cli/src/commands/rename.js b/packages/cli/src/commands/move.js
similarity index 50%
rename from packages/cli/src/commands/rename.js
rename to packages/cli/src/commands/move.js
index 28e76b48a..83d57641c 100644
--- a/packages/cli/src/commands/rename.js
+++ b/packages/cli/src/commands/move.js
@@ -2,8 +2,9 @@
 import os from 'os';
 import { E } from '@endo/far';
 import { withEndoAgent } from '../context.js';
+import { parsePetNamePath } from '../pet-name.js';
 
-export const rename = async ({ fromName, toName, agentNames }) =>
+export const move = async ({ fromPath, toPath, agentNames }) =>
   withEndoAgent(agentNames, { os, process }, async ({ agent }) => {
-    await E(agent).rename(fromName, toName);
+    await E(agent).move(parsePetNamePath(fromPath), parsePetNamePath(toPath));
   });
diff --git a/packages/cli/src/endo.js b/packages/cli/src/endo.js
index 78fb665a0..54b01d692 100644
--- a/packages/cli/src/endo.js
+++ b/packages/cli/src/endo.js
@@ -297,13 +297,14 @@ export const main = async rawArgs => {
     });
 
   program
-    .command('rename <from> <to>')
+    .command('move <from> <to>')
+    .alias('mv')
     .description('change the name for a value')
     .option(...commonOptions.as)
-    .action(async (fromName, toName, cmd) => {
+    .action(async (fromPath, toPath, cmd) => {
       const { as: agentNames } = cmd.opts();
-      const { rename } = await import('./commands/rename.js');
-      return rename({ fromName, toName, agentNames });
+      const { move } = await import('./commands/move.js');
+      return move({ fromPath, toPath, agentNames });
     });
 
   program
diff --git a/packages/daemon/src/directory.js b/packages/daemon/src/directory.js
index c3600026e..114dac1dc 100644
--- a/packages/daemon/src/directory.js
+++ b/packages/daemon/src/directory.js
@@ -174,22 +174,25 @@ export const makeDirectoryMaker = ({
       const { hub: fromHub, name: fromName } =
         await lookupTailNameHub(fromPath);
       const { hub: toHub, name: toName } = await lookupTailNameHub(toPath);
+
       if (fromHub === toHub) {
         // eslint-disable-next-line no-use-before-define
         if (fromHub === directory) {
           await petStore.rename(fromName, toName);
         } else {
-          await fromHub.move([fromName], [toName]);
+          await E(fromHub).move([fromName], [toName]);
         }
         return;
       }
-      const id = await fromHub.identify(fromName);
+
+      const id = await E(fromHub).identify(fromName);
       if (id === undefined) {
         throw new Error(`Unknown name: ${q(fromPath)}`);
       }
-      const removeP = fromHub.remove(fromName);
-      const addP = toHub.write([toName], id);
-      await Promise.all([addP, removeP]);
+      // First write to the "to" hub so that the original name is preserved on the
+      // "from" hub in case of failure.
+      await E(toHub).write([toName], id);
+      await E(fromHub).remove(fromName);
     };
 
     /** @type {EndoDirectory['copy']} */
diff --git a/packages/daemon/src/pet-store.js b/packages/daemon/src/pet-store.js
index 8c7ef48e9..d8fd7a968 100644
--- a/packages/daemon/src/pet-store.js
+++ b/packages/daemon/src/pet-store.js
@@ -195,6 +195,7 @@ export const makePetStoreMaker = (filePowers, config) => {
       }
       assertValidId(formulaIdentifier, fromName);
 
+      // Updated persisted name mapping
       const fromPath = filePowers.joinPath(petNameDirectoryPath, fromName);
       const toPath = filePowers.joinPath(petNameDirectoryPath, toName);
       await filePowers.renamePath(fromPath, toPath);
@@ -206,6 +207,7 @@ export const makePetStoreMaker = (filePowers, config) => {
       }
 
       // Update the mapping for the pet name.
+      idsToPetNames.delete(formulaIdentifier, fromName);
       idsToPetNames.add(formulaIdentifier, toName);
 
       publishNameRemoval(formulaIdentifier, fromName);


Test patch:
diff --git a/packages/daemon/test/failed-hub.js b/packages/daemon/test/failed-hub.js
new file mode 100644
index 000000000..0ad941e20
--- /dev/null
+++ b/packages/daemon/test/failed-hub.js
@@ -0,0 +1,14 @@
+import { makeExo } from '@endo/exo';
+import { M } from '@endo/patterns';
+
+export const make = () => {
+  return makeExo(
+    'FailedHub',
+    M.interface('FailedHub', {}, { defaultGuards: 'passable' }),
+    {
+      write() {
+        throw new Error('I had one job.');
+      },
+    },
+  );
+};
diff --git a/packages/daemon/test/move-hub.js b/packages/daemon/test/move-hub.js
new file mode 100644
index 000000000..aedc0b1c8
--- /dev/null
+++ b/packages/daemon/test/move-hub.js
@@ -0,0 +1,96 @@
+// @ts-check
+
+import { makeExo } from '@endo/exo';
+import { M } from '@endo/patterns';
+
+const { quote: q } = assert;
+
+/** @import {NameHub} from '../src/types.js' */
+
+// This caplet is a mock name hub for testing NameHub.move().
+export const make = () => {
+  /** @type {Map<string, string>} */
+  const idToName = new Map();
+  /** @type {Map<string, string>} */
+  const nameToId = new Map();
+
+  /**
+   * We only support paths of length 1.
+   * @param {Array<string>} petNamePath
+   */
+  const parsePetNamePath = petNamePath => {
+    if (!Array.isArray(petNamePath) || petNamePath.length !== 1) {
+      throw new Error(`Unexpected pet name path ${q(petNamePath)}`);
+    }
+    return petNamePath[0];
+  };
+
+  /**
+   * @param {string} petName
+   */
+  const expectGetId = petName => {
+    const id = nameToId.get(petName);
+    if (id === undefined) {
+      throw new Error(`Unknown pet name ${q(petName)}`);
+    }
+    return id;
+  };
+
+  /**
+   * @type {NameHub['write']}
+   */
+  const write = async (petNamePath, id) => {
+    const petName = parsePetNamePath(petNamePath);
+    idToName.set(id, petName);
+    nameToId.set(petName, id);
+  };
+
+  /**
+   * @type {NameHub['remove']}
+   */
+  const remove = async (...petNamePath) => {
+    const petName = parsePetNamePath(petNamePath);
+    const id = expectGetId(petName);
+
+    nameToId.delete(petName);
+    idToName.delete(id);
+  };
+
+  return makeExo(
+    'MoveHub',
+    M.interface('MoveHub', {}, { defaultGuards: 'passable' }),
+    {
+      write,
+      remove,
+
+      /**
+       * @type {NameHub['identify']}
+       */
+      async identify(...petNamePath) {
+        const petName = parsePetNamePath(petNamePath);
+        return nameToId.get(petName);
+      },
+
+      /**
+       * @type {NameHub['move']}
+       */
+      async move(fromPath, toPath) {
+        const fromName = parsePetNamePath(fromPath);
+        const id = expectGetId(fromName);
+
+        const toName = parsePetNamePath(toPath);
+
+        await remove(fromName);
+        await write([toName], id);
+      },
+
+      /**
+       * @type {NameHub['has']}
+       */
+      async has(...petNamePath) {
+        const petName = parsePetNamePath(petNamePath);
+        return nameToId.has(petName);
+      },
+    },
+  );
+};
diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js
index f8a6b2f31..2e18e3d71 100644
--- a/packages/daemon/test/test-endo.js
+++ b/packages/daemon/test/test-endo.js
@@ -439,6 +439,162 @@ test('store with name', async t => {
   }
 });
 
+test('move renames value', async t => {
+  const { host } = await prepareHost(t);
+
+  await E(host).storeValue(10, 'ten');
+
+  t.true(await E(host).has('ten'));
+  t.false(await E(host).has('zehn'));
+
+  await E(host).move(['ten'], ['zehn']);
+
+  t.false(await E(host).has('ten'));
+  t.true(await E(host).has('zehn'));
+});
+
+test('move renames value, overwriting the "to" name', async t => {
+  const { host } = await prepareHost(t);
+
+  await E(host).storeValue(10, 'ten');
+  await E(host).storeValue('"X"', 'decimus');
+
+  t.true(await E(host).has('ten'));
+  t.true(await E(host).has('decimus'));
+
+  await E(host).move(['ten'], ['decimus']);
+
+  t.false(await E(host).has('ten'));
+  t.true(await E(host).has('decimus'));
+
+  const decimusValue = await E(host).lookup('decimus');
+  t.is(decimusValue, 10);
+});
+
+test('move moves value, from the host to a different name hub', async t => {
+  const { host } = await prepareHost(t);
+  const directory = await E(host).makeDirectory('directory');
+
+  await E(host).storeValue(10, 'ten');
+
+  t.true(await E(host).has('ten'));
+  t.false(await E(directory).has('ten'));
+
+  await E(host).move(['ten'], ['directory', 'ten']);
+
+  t.false(await E(host).has('ten'));
+  t.true(await E(directory).has('ten'));
+});
+
+test('move renames value, for a single guest', async t => {
+  const { host } = await prepareHost(t);
+
+  const guest = await E(host).provideGuest('guest', {
+    agentName: 'guest-agent',
+  });
+
+  await E(host).storeValue(10, 'ten');
+  await E(host).move(['ten'], ['guest-agent', 'ten']);
+
+  t.true(await E(guest).has('ten'));
+
+  await E(host).move(['guest-agent', 'ten'], ['guest-agent', 'zehn']);
+
+  t.false(await E(guest).has('ten'));
+  t.true(await E(guest).has('zehn'));
+});
+
+test('move moves value, between different guests', async t => {
+  const { host } = await prepareHost(t);
+
+  const guest1 = await E(host).provideGuest('guest1', {
+    agentName: 'guest1-agent',
+  });
+  const guest2 = await E(host).provideGuest('guest2', {
+    agentName: 'guest2-agent',
+  });
+
+  await E(host).storeValue(10, 'ten');
+  await E(host).move(['ten'], ['guest1-agent', 'ten']);
+
+  t.true(await E(guest1).has('ten'));
+
+  await E(host).move(['guest1-agent', 'ten'], ['guest2-agent', 'ten']);
+
+  t.false(await E(guest1).has('ten'));
+  t.true(await E(guest2).has('ten'));
+});
+
+test('move renames value, for a single caplet name hub', async t => {
+  const { host } = await prepareHost(t);
+
+  const nameHubPath = path.join(dirname, 'test', 'move-hub.js');
+  const nameHub = await E(host).makeUnconfined(
+    'MAIN',
+    nameHubPath,
+    'NONE',
+    'name-hub',
+  );
+
+  await E(host).storeValue(10, 'ten');
+  const tenId = await E(host).identify('ten');
+  await E(nameHub).write(['ten'], tenId);
+
+  t.true(await E(nameHub).has('ten'));
+
+  await E(host).move(['name-hub', 'ten'], ['name-hub', 'zehn']);
+
+  t.false(await E(nameHub).has('ten'));
+  t.true(await E(nameHub).has('zehn'));
+});
+
+test('move moves value, between different caplet name hubs', async t => {
+  const { host } = await prepareHost(t);
+
+  const nameHubPath = path.join(dirname, 'test', 'move-hub.js');
+  const nameHub1 = await E(host).makeUnconfined(
+    'MAIN',
+    nameHubPath,
+    'NONE',
+    'name-hub1',
+  );
+  const nameHub2 = await E(host).makeUnconfined(
+    'MAIN',
+    nameHubPath,
+    'NONE',
+    'name-hub2',
+  );
+
+  await E(host).storeValue(10, 'ten');
+  const tenId = await E(host).identify('ten');
+  await E(nameHub1).write(['ten'], tenId);
+
+  t.true(await E(nameHub1).has('ten'));
+
+  await E(host).move(['name-hub1', 'ten'], ['name-hub2', 'ten']);
+
+  t.false(await E(nameHub1).has('ten'));
+  t.true(await E(nameHub2).has('ten'));
+});
+
+test('move preserves original name if writing to new name hub fails', async t => {
+  const { host } = await prepareHost(t);
+
+  await E(host).storeValue(10, 'ten');
+
+  t.true(await E(host).has('ten'));
+
+  const failedHubPath = path.join(dirname, 'test', 'failed-hub.js');
+  await E(host).makeUnconfined('MAIN', failedHubPath, 'NONE', 'failed-hub');
+
+  await t.throwsAsync(E(host).move(['ten'], ['failed-hub', 'ten']), {
+    message: 'I had one job.',
+  });
+
+  const tenValue = await E(host).lookup('ten');
+  t.is(tenValue, 10);
+});
+
 test('closure state lost by restart', async t => {
   const { cancelled, config } = await prepareConfig(t);