refactor(daemon): Coordinate host provideX implementations (merge #2139)

Progresses: #2086 > coordinate, verb, "to combine in harmonious relation or action." Synchronizes the host's `provideWorker()`. In addition, coordinates the implementations of the host's `provideX` methods such that they all: 1. Attempt to get the existing formula id for the provided name, if any. 2. If there is no existing formula id, incarnate and return a new value. All type checks during step 1 have been removed. In other words, if you attempt to provide something under an existing name that resolves to a different value type, you're on your own. We may revisit this decision in the future.
endojs · Mar 16, 2024 · fdc6989 · fdc6989
2 parents f162e19 + 7483103
commit fdc6989
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 59 deletions.
diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js
@@ -1004,9 +1004,21 @@ const makeDaemonCore = async (
   /**
    * @type {import('./types.js').DaemonCore['incarnateWorker']}
    */
-  const incarnateWorker = async () => {
-    const formulaNumber = await formulaGraphJobs.enqueue(randomHex512);
-    return incarnateNumberedWorker(formulaNumber);
+  const incarnateWorker = async deferredTasks => {
+    return incarnateNumberedWorker(
+      await formulaGraphJobs.enqueue(async () => {
+        const formulaNumber = await randomHex512();
+
+        await deferredTasks.execute({
+          workerFormulaIdentifier: formatId({
+            number: formulaNumber,
+            node: ownNodeIdentifier,
+          }),
+        });
+
+        return formulaNumber;
+      }),
+    );
   };
 
   /**

diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js
@@ -125,60 +125,52 @@ export const makeHostMaker = ({
      * @param {string} workerName
      */
     const provideWorker = async workerName => {
-      if (typeof workerName !== 'string') {
-        throw new Error('worker name must be string');
-      }
-      let workerFormulaIdentifier = petStore.identifyLocal(workerName);
-      if (workerFormulaIdentifier === undefined) {
-        ({ formulaIdentifier: workerFormulaIdentifier } =
-          await incarnateWorker());
-        assertPetName(workerName);
-        await petStore.write(workerName, workerFormulaIdentifier);
-      } else if (!workerFormulaIdentifier.startsWith('worker:')) {
-        throw new Error(`Not a worker ${q(workerName)}`);
+      /** @type {import('./types.js').DeferredTasks<import('./types.js').WorkerDeferredTaskParams>} */
+      const tasks = makeDeferredTasks();
+      // eslint-disable-next-line no-use-before-define
+      const workerFormulaIdentifier = tryGetWorkerFormulaIdentifier(workerName);
+      // eslint-disable-next-line no-use-before-define
+      prepareWorkerIncarnation(workerName, workerFormulaIdentifier, tasks.push);
+
+      if (workerFormulaIdentifier !== undefined) {
+        return /** @type {Promise<import('./types.js').EndoWorker>} */ (
+          // Behold, recursion:
+          provideValueForFormulaIdentifier(workerFormulaIdentifier)
+        );
       }
-      return /** @type {Promise<import('./types.js').EndoWorker>} */ (
-        // Behold, recursion:
-        // eslint-disable-next-line no-use-before-define
-        provideValueForFormulaIdentifier(workerFormulaIdentifier)
-      );
+
+      const { value } = await incarnateWorker(tasks);
+      return value;
     };
 
     /**
-     * @param {string | 'MAIN' | 'NEW'} workerName
-     * @param {import('./types.js').DeferredTasks<{ workerFormulaIdentifier: string }>['push']} deferTask
+     * @param {string} workerName
      * @returns {string | undefined}
      */
-    const prepareWorkerFormulaIdentifier = (workerName, deferTask) => {
+    const tryGetWorkerFormulaIdentifier = workerName => {
       if (workerName === 'MAIN') {
         return mainWorkerFormulaIdentifier;
       } else if (workerName === 'NEW') {
         return undefined;
       }
-
-      assertPetName(workerName);
-      const workerFormulaIdentifier = petStore.identifyLocal(workerName);
-      if (workerFormulaIdentifier === undefined) {
-        deferTask(identifiers =>
-          petStore.write(workerName, identifiers.workerFormulaIdentifier),
-        );
-      }
-      return workerFormulaIdentifier;
+      return petStore.identifyLocal(workerName);
     };
 
     /**
-     * @param {string | 'NONE' | 'SELF' | 'ENDO'} agentName
-     * @param {import('./types.js').DeferredTasks<{ powersFormulaIdentifier: string }>['push']} deferTask
-     * @returns {string | undefined}
+     * @param {string} workerName
+     * @param {string | undefined} workerFormulaIdentifier
+     * @param {import('./types.js').DeferredTasks<{ workerFormulaIdentifier: string }>['push']} deferTask
      */
-    const preparePowersFormulaIdentifier = (agentName, deferTask) => {
-      const powersFormulaIdentifier = petStore.identifyLocal(agentName);
-      if (powersFormulaIdentifier === undefined) {
+    const prepareWorkerIncarnation = (
+      workerName,
+      workerFormulaIdentifier,
+      deferTask,
+    ) => {
+      if (workerFormulaIdentifier === undefined) {
         deferTask(identifiers =>
-          petStore.write(agentName, identifiers.powersFormulaIdentifier),
+          petStore.write(workerName, identifiers.workerFormulaIdentifier),
         );
       }
-      return powersFormulaIdentifier;
     };
 
     /**
@@ -205,10 +197,8 @@ export const makeHostMaker = ({
       /** @type {import('./types.js').DeferredTasks<import('./types.js').EvalDeferredTaskParams>} */
       const tasks = makeDeferredTasks();
 
-      const workerFormulaIdentifier = prepareWorkerFormulaIdentifier(
-        workerName,
-        tasks.push,
-      );
+      const workerFormulaIdentifier = tryGetWorkerFormulaIdentifier(workerName);
+      prepareWorkerIncarnation(workerName, workerFormulaIdentifier, tasks.push);
 
       /** @type {(string | string[])[]} */
       const endowmentFormulaIdsOrPaths = petNamePaths.map(
@@ -259,15 +249,15 @@ export const makeHostMaker = ({
       /** @type {import('./types.js').DeferredTasks<import('./types.js').MakeCapletDeferredTaskParams>} */
       const tasks = makeDeferredTasks();
 
-      const workerFormulaIdentifier = prepareWorkerFormulaIdentifier(
-        workerName,
-        tasks.push,
-      );
+      const workerFormulaIdentifier = tryGetWorkerFormulaIdentifier(workerName);
+      prepareWorkerIncarnation(workerName, workerFormulaIdentifier, tasks.push);
 
-      const powersFormulaIdentifier = preparePowersFormulaIdentifier(
-        powersName,
-        tasks.push,
-      );
+      const powersFormulaIdentifier = petStore.identifyLocal(powersName);
+      if (powersFormulaIdentifier === undefined) {
+        tasks.push(identifiers =>
+          petStore.write(powersName, identifiers.powersFormulaIdentifier),
+        );
+      }
 
       if (resultName !== undefined) {
         tasks.push(identifiers =>
@@ -398,7 +388,7 @@ export const makeHostMaker = ({
      * @returns {Promise<{formulaIdentifier: string, value: Promise<import('./types.js').EndoHost>}>}
      */
     const makeHost = async (petName, { introducedNames = {} } = {}) => {
-      let host = await getNamedAgent(petName);
+      let host = getNamedAgent(petName);
       if (host === undefined) {
         const { value, formulaIdentifier } =
           // Behold, recursion:
@@ -428,7 +418,7 @@ export const makeHostMaker = ({
      * @returns {Promise<{formulaIdentifier: string, value: Promise<import('./types.js').EndoGuest>}>}
      */
     const makeGuest = async (petName, { introducedNames = {} } = {}) => {
-      let guest = await getNamedAgent(petName);
+      let guest = getNamedAgent(petName);
       if (guest === undefined) {
         const { value, formulaIdentifier } =
           // Behold, recursion:

diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts
@@ -72,6 +72,10 @@ type WorkerFormula = {
   type: 'worker';
 };
 
+export type WorkerDeferredTaskParams = {
+  workerFormulaIdentifier: string;
+};
+
 /**
  * Deferred tasks parameters for `host` and `guest` formulas.
  */
@@ -789,7 +793,9 @@ export interface DaemonCore {
   incarnateEndoBootstrap: (
     specifiedFormulaNumber: string,
   ) => IncarnateResult<FarEndoBootstrap>;
-  incarnateWorker: () => IncarnateResult<EndoWorker>;
+  incarnateWorker: (
+    deferredTasks: DeferredTasks<WorkerDeferredTaskParams>,
+  ) => IncarnateResult<EndoWorker>;
   incarnateDirectory: () => IncarnateResult<EndoDirectory>;
   incarnateHost: (
     endoFormulaIdentifier: string,

diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js
@@ -149,7 +149,8 @@ test('anonymous spawn and evaluate', async t => {
   await stop(locator);
 });
 
-test('cannot spawn worker with existing non-worker name', async t => {
+// Regression test for https://github.com/endojs/endo/issues/2147
+test('spawning a worker does not overwrite existing non-worker name', async t => {
   const { promise: cancelled, reject: cancel } = makePromiseKit();
   t.teardown(() => cancel(Error('teardown')));
   const locator = makeLocator('tmp', 'spawn-eval-name-reuse');
@@ -165,10 +166,15 @@ test('cannot spawn worker with existing non-worker name', async t => {
   );
   const bootstrap = getBootstrap();
   const host = E(bootstrap).host();
-  const ten = await E(host).evaluate('MAIN', '10', [], [], 'ten');
-  t.is(ten, 10);
-  await t.throwsAsync(() => E(host).provideWorker('ten'), {
-    message: 'Not a worker "ten"',
+  const foo = await E(host).evaluate('MAIN', '10', [], [], 'foo');
+  t.is(foo, 10);
+
+  // This resolves with the existing value of 'foo' rather than overwriting it
+  // with a new worker.
+  await E(host).provideWorker('foo');
+  await t.throwsAsync(() => E(host).evaluate('foo', '20', [], [], 'bar'), {
+    message:
+      'Cannot deliver "evaluate" to target; typeof target is "undefined"',
   });
 
   await stop(locator);