refactor: insert await null;s needed for await safety (#2347)

Closes: #XXXX
Refs: #XXXX

## Description

Prior to this PR, `yarn lint` warns about many violations of our `await`
safety rules. In vscode, I was very pleasantly surprised to learn about
the automated fixes offered interactively for these:


![image](https://github.com/user-attachments/assets/6be443de-dcd8-4e1a-b714-6a1f1106bd61)

For each of these, I accepted the "insert `await null;` before..."
option. Uniquely for the one shown in the above screenshot I needed to
do an additional manual readjustment to move the `//
eslint-disable-next-line ...` so it would still be about the next line.

Caveats:

- I applied the automated fix manually, without reading the code in
question or in any other way validating that this change is correctness
preserving. My only evidence at the moment is that all the tests still
pass.
- I would have loved to mark this PR with `refactor:` or at least
`fix:`. But I marked it with `fix!` because there's no reason yet to
think these changes are either correctness preserving or compat.
- I applied the automated change blindly even to the test files, even
though we're not normally concerned about `await` safety there. Indeed,
we should also change our lint configuration so `await` safety is
unenforced for those. After this PR, we should also change our `await`
safety enforcement for non-test files from "warning" to "error", to keep
these from creeping back in.
- I applied the automated fix to the test files anyway since test code
still often serves as precedent-by-example for production code. Since
the automated fix made it easy to do this, I decided it was better to
improve our test-code-as-precedent.

Obviously, this should not be merged until we're confident that these
changes are correctness preserving. I don't know of any way to do so
other than careful manual review. Perhaps as part of the same review, we
might also conclude that it is compat, in which case we can change back
from `fix!` to `fix:` or even hopefully `refactor:`.

For the various "considerations" below, I'll answer under the assumption
that we have already reviewed these changes for correctness, and deemed
them all to be correctness preserving. Obviously, until that happens,
this PR is fatal re all the considerations below.

Update: As suggested by review comments, I moved those inserted `await
null;`s to the top of their respective functions.

Update: Between @kriskowal 's evaluation comments below and my own
inspection, I'm reasonably confident that this PR should count as a
refactor, so I'm changing the title from `fix!...` to `refactor:...`.


### Security Considerations

Assuming the changes are correctness preserving, `await` safety aids
security.

### Scaling Considerations

none
### Documentation Considerations

we already need to explain `await` safety. This PR doesn't change that,
but repairs our code so that we practice what we will preach.
### Testing Considerations

A bit distressing that all tests pass without any further intervention.
This perhaps indicates a failure to test for the semantics that are
changed by these extra awaits.
### Compatibility Considerations

If this change is correctness preserving, then it is likely also compat.
But not necessarily. The extra awaits introduce extra turn delays and
extra interleaving points.
### Upgrade Considerations

If this PR is correctness preserving and compat, then there are unlikely
to be any upgrade considerations. But I don't know that for sure, so we
should also worry about this during review.

packages/base64/test/bench-main.js

@@ -16,6 +16,7 @@ async function main() {
   const log = (typeof console !== 'undefined' && console.log) || print;
   /** @type {typeof Date.now} */
   const now = await (async () => {
+    await null;
     try {
       const { performance } = await import('perf_hooks');
       if (performance.now) {

packages/captp/test/export-hook.test.js

@@ -6,6 +6,7 @@ import { Far } from '@endo/marshal';
 import { E, makeLoopback } from '../src/loopback.js';
 
 test('exportHook', async t => {
+  await null;
   const exports = [];
 
   const { makeFar } = makeLoopback('us', {

packages/captp/test/gc.test.js

@@ -7,6 +7,7 @@ import { detectEngineGC } from './engine-gc.js';
 import { makeGcAndFinalize } from './gc-and-finalize.js';
 
 const isolated = async (t, makeFar) => {
+  await null;
   const local = Far('local', {
     method: () => 'local',
   });

packages/captp/test/trap.test.js

@@ -16,6 +16,7 @@ import {
 const dirname = url.fileURLToPath(new URL('./', import.meta.url));
 
 const makeWorkerTests = isHost => async t => {
+  await null;
   const initFn = isHost ? makeHost : makeGuest;
   for (let len = 0; len < MIN_TRANSFER_BUFFER_LENGTH; len += 1) {
     t.throws(() => initFn(() => {}, new SharedArrayBuffer(len)), {

packages/captp/test/traplib.js

@@ -24,6 +24,7 @@ export const createHostBootstrap = makeTrapHandler => {
 };
 
 export const runTrapTests = async (t, Trap, bs, unwrapsPromises) => {
+  await null;
   // Demonstrate async compatibility of traps.
   const pn = E(E(bs).getTraps(3)).getN();
   t.is(Promise.resolve(pn), pn);

packages/check-bundle/lite.js

@@ -22,6 +22,7 @@ export const checkBundle = async (
   computeSha512,
   bundleName = '<unknown-bundle>',
 ) => {
+  await null;
   assert.typeof(
     bundle,
     'object',

packages/cli/src/client.js

@@ -13,6 +13,7 @@ export const provideEndoClient = async (
   cancelled,
   bootstrap,
 ) => {
+  await null;
   try {
     // It is okay to fail to connect because the daemon is not running.
     return await makeEndoClient(name, sockPath, cancelled, bootstrap);

packages/cli/src/commands/accept.js

@@ -12,6 +12,7 @@ const fromAsync = async iterable => {
 };
 
 export const accept = async ({ guestName, agentNames }) => {
+  await null;
   process.stdin.setEncoding('utf-8');
   const invitationLocator = (await fromAsync(process.stdin)).join('').trim();
   return withEndoAgent(agentNames, { os, process }, async ({ agent }) => {

packages/cli/src/commands/list.js

@@ -36,6 +36,7 @@ const pad = (fieldVal, width, minPad = 2) => {
 
 export const list = async ({ directory, follow, json, verbose }) =>
   withEndoHost({ os, process }, async ({ host: agent }) => {
+    await null;
     if (directory !== undefined) {
       const directoryPath = parsePetNamePath(directory);
       agent = E(agent).lookup(...directoryPath);

packages/cli/src/commands/log.js

@@ -24,6 +24,7 @@ const delay = async (ms, cancelled) => {
 
 export const log = async ({ follow, ping }) =>
   withInterrupt(async ({ cancelled }) => {
+    await null;
     const logCheckIntervalMs = ping !== undefined ? Number(ping) : 5_000;
 
     const { username, homedir } = os.userInfo();

packages/cli/src/commands/make.js

@@ -22,6 +22,7 @@ export const makeCommand = async ({
   agentNames,
   powersName,
 }) => {
+  await null;
   if (filePath !== undefined && importPath !== undefined) {
     console.error('Specify only one of [file] or --UNCONFINED <file>');
     process.exitCode = 1;
@@ -61,6 +62,7 @@ export const makeCommand = async ({
   }
 
   await withEndoAgent(agentNames, { os, process }, async ({ agent }) => {
+    await null;
     // Prepare a bundle, with the given name.
     if (bundleReaderRef !== undefined) {
       await E(agent).storeBlob(bundleReaderRef, bundleName);

packages/cli/src/commands/run.js

@@ -44,6 +44,7 @@ export const run = async ({
     agentNames,
     { os, process },
     async ({ bootstrap, agent }) => {
+      await null;
       let powersP;
       if (powersName === 'NONE') {
         powersP = E(bootstrap).leastAuthority();

packages/cli/src/commands/store.js

@@ -76,6 +76,7 @@ export const store = async ({
   const parsedName = parsePetNamePath(name);
 
   await withEndoAgent(agentNames, { os, process }, async ({ agent }) => {
+    await null;
     if (storeText !== undefined) {
       await E(agent).storeValue(storeText, parsedName);
     } else if (storeJson !== undefined) {

packages/cli/src/context.js

@@ -7,6 +7,7 @@ import { provideEndoClient } from './client.js';
 import { parsePetNamePath } from './pet-name.js';
 
 export const withInterrupt = async callback => {
+  await null;
   const { promise: cancelled, reject: cancel } = makePromiseKit();
   cancelled.catch(() => {});
   process.once('SIGINT', () => cancel(Error('SIGINT')));

packages/common/test/apply-labeling-error.test.js

@@ -4,6 +4,7 @@ import { Fail } from '@endo/errors';
 import { applyLabelingError } from '../apply-labeling-error.js';
 
 test('test applyLabelingError', async t => {
+  await null;
   t.is(
     applyLabelingError(x => x * 2, [8]),
     16,

packages/compartment-mapper/src/archive-lite.js

@@ -226,6 +226,7 @@ const renameSources = (sources, compartmentRenames) => {
  * @param {Sources} sources
  */
 const addSourcesToArchive = async (archive, sources) => {
+  await null;
   for (const compartment of keys(sources).sort()) {
     const modules = sources[compartment];
     const compartmentLocation = resolveLocation(`${compartment}/`, 'file:///');

packages/compartment-mapper/src/import-archive-lite.js

@@ -105,6 +105,7 @@ const makeArchiveImportHookMaker = (
     const { modules } = compartmentDescriptor;
     /** @type {ImportHook} */
     const importHook = async moduleSpecifier => {
+      await null;
       // per-module:
       const module = modules[moduleSpecifier];
       if (module === undefined) {
@@ -280,6 +281,7 @@ export const parseArchive = async (
   archiveLocation = '<unknown>',
   options = {},
 ) => {
+  await null;
   const {
     computeSha512 = undefined,
     expectedSha512 = undefined,

packages/compartment-mapper/src/import-hook.js

@@ -209,6 +209,7 @@ export const makeImportHookMaker = (
 
     /** @type {ImportHook} */
     const importHook = async moduleSpecifier => {
+      await null;
       compartmentDescriptor.retained = true;
 
       // per-module:

packages/compartment-mapper/src/link.js

@@ -84,6 +84,7 @@ const makeExtensionParser = (
   moduleTransforms,
 ) => {
   return async (bytes, specifier, location, packageLocation, options) => {
+    await null;
     let language;
     const extension = parseExtension(location);
 

packages/compartment-mapper/src/node-modules.js

@@ -152,6 +152,7 @@ const readDescriptorWithMemo = async (memo, maybeRead, packageLocation) => {
  * } | undefined>}
  */
 const findPackage = async (readDescriptor, canonical, directory, name) => {
+  await null;
   for (;;) {
     // eslint-disable-next-line no-await-in-loop
     const packageLocation = await canonical(

packages/compartment-mapper/src/node-powers.js

@@ -104,6 +104,7 @@ const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
    * @param {string} location
    */
   const canonical = async location => {
+    await null;
     try {
       if (location.endsWith('/')) {
         const realPath = await fs.promises.realpath(
@@ -157,6 +158,7 @@ const makeWritePowersSloppy = ({ fs, url = undefined }) => {
    * @param {Uint8Array} data
    */
   const write = async (location, data) => {
+    await null;
     try {
       return await fs.promises.writeFile(fileURLToPath(location), data);
     } catch (error) {

packages/compartment-mapper/src/search.js

@@ -33,6 +33,7 @@ const resolveLocation = (rel, abs) => new URL(rel, abs).toString();
  * @returns {Promise<{data:T, directory: string, location:string, packageDescriptorLocation: string}>}
  */
 export const searchDescriptor = async (location, readDescriptor) => {
+  await null;
   let directory = resolveLocation('./', location);
   for (;;) {
     const packageDescriptorLocation = resolveLocation(

packages/compartment-mapper/test/policy.test.js

@@ -5,6 +5,7 @@ import { moduleify, scaffold, sanitizePaths } from './scaffold.js';
 
 function combineAssertions(...assertionFunctions) {
   return async (...args) => {
+    await null;
     for (const assertion of assertionFunctions) {
       // eslint-disable-next-line no-await-in-loop
       await assertion(...args);

packages/compartment-mapper/test/scaffold.js

@@ -62,6 +62,7 @@ const builtinLocation = new URL(
 // application package.
 
 export async function setup() {
+  await null;
   if (modules === undefined) {
     const utility = await loadLocation(readPowers, builtinLocation);
     const { namespace } = await utility.import({ globals });
@@ -98,6 +99,7 @@ export function scaffold(
       testFunc = testFunc.failing || testFunc;
     }
     return testFunc(title, async t => {
+      await null;
       const compartmentInstrumentation = compartmentInstrumentationFactory();
       let namespace;
       try {

packages/compartment-mapper/test/snapshots/error-handling.test.js.md

@@ -84,15 +84,23 @@ Generated by [AVA](https://avajs.dev).
 
     `Error: Cannot find external module "missing" in package file://.../compartment-mapper/test/fixtures-error-handling/node_modules/cjs/␊
         at importHook (file://.../compartment-mapper/src/import-hook.js:…)␊
-        at async asyncTrampoline (file://.../ses/src/module-load.js:…)`
+        at async asyncTrampoline (file://.../ses/src/module-load.js:…)␊
+        at async drainQueue (file://.../ses/src/module-load.js:…)␊
+        at async load (file://.../ses/src/module-load.js:…)␊
+        at async file://.../compartment-mapper/test/scaffold.js:…␊
+        at async file://.../compartment-mapper/test/scaffold.js:…`
 
 ## fixtures-error-handling / cjs / importLocation
 
 > Snapshot 1
 
     `Error: Cannot find external module "missing" in package file://.../compartment-mapper/test/fixtures-error-handling/node_modules/cjs/␊
         at importHook (file://.../compartment-mapper/src/import-hook.js:…)␊
-        at async asyncTrampoline (file://.../ses/src/module-load.js:…)`
+        at async asyncTrampoline (file://.../ses/src/module-load.js:…)␊
+        at async drainQueue (file://.../ses/src/module-load.js:…)␊
+        at async load (file://.../ses/src/module-load.js:…)␊
+        at async file://.../compartment-mapper/test/scaffold.js:…␊
+        at async file://.../compartment-mapper/test/scaffold.js:…`
 
 ## fixtures-error-handling / cjs / makeArchive / parseArchive
 

packages/compartment-mapper/test/stack.test.js

@@ -41,6 +41,7 @@ test('archive stack trace source', async t => {
 // Whereas, we expect the same program executed directly from local files to
 // have a fully qualified file URL in the stack trace.
 test('disk stack trace source', async t => {
+  await null;
   let error;
   try {
     await importLocation(readPowers, fixtureLocation);

packages/daemon/index.js

@@ -144,6 +144,7 @@ const enoentOk = error => {
 };
 
 export const clean = async (config = defaultConfig) => {
+  await null;
   if (process.platform !== 'win32') {
     await removePath(config.sockPath).catch(enoentOk);
   }

packages/daemon/src/daemon-node-powers.js

@@ -367,7 +367,8 @@ export const makeDaemonicPersistencePowers = (
           return filePowers.readFileText(storagePath);
         };
         const json = async () => {
-          return JSON.parse(await text());
+          const jsonSrc = await text();
+          return JSON.parse(jsonSrc);
         };
         return harden({
           sha512: () => sha512,

packages/daemon/src/serve-private-port-http.js

@@ -18,6 +18,7 @@ export const servePrivatePortHttp = (
   const connectionClosedPromises = new Set();
 
   const respond = async request => {
+    await null;
     if (request.method === 'GET') {
       if (request.url === '/') {
         return {

packages/daemon/src/web-server-node-powers.js

@@ -79,6 +79,7 @@ export const makeHttpPowers = ({ http, ws }) => {
 
       server.on('request', (req, res) => {
         (async () => {
+          await null;
           if (req.method === undefined || req.url === undefined) {
             return;
           }

packages/daemon/src/web-server-node.js

@@ -110,6 +110,7 @@ export const make = async (_powers, context) => {
       }
 
       (async () => {
+        await null;
         const {
           reader: frameReader,
           writer: frameWriter,
@@ -137,7 +138,6 @@ export const make = async (_powers, context) => {
         // TODO set up heart monitor
         E.sendOnly(webletBootstrap).ping();
 
-        // eslint-disable-next-line no-use-before-define
         E(webletBootstrap)
           .makeBundle(
             await E(/** @type {any} */ (webletBundle)).json(),

packages/daemon/test/context-consumer.js

@@ -11,6 +11,7 @@ export const ContextConsumerInterface = M.interface(
 export const make = async (_powers, context) => {
   return makeExo('Context consumer', ContextConsumerInterface, {
     async awaitCancellation() {
+      await null;
       try {
         await E(context).whenCancelled();
       } catch {

packages/eventual-send/test/e.test.js

@@ -39,6 +39,7 @@ test('E.when', async t => {
 });
 
 test('E method calls', async t => {
+  await null;
   const x = {
     double(n) {
       return 2 * n;
@@ -144,6 +145,7 @@ test('E method call undefined receiver', async t => {
 });
 
 test('E shortcuts', async t => {
+  await null;
   const x = {
     name: 'buddy',
     val: 123,
@@ -166,6 +168,7 @@ test('E shortcuts', async t => {
 });
 
 test('E.get', async t => {
+  await null;
   const x = {
     name: 'buddy',
     val: 123,

packages/eventual-send/test/eventual-send.test.js

@@ -155,6 +155,7 @@ test('handlers are always async', async t => {
 });
 
 test('new HandledPromise expected errors', async t => {
+  await null;
   const handler = {
     get(o, _key) {
       return o;
@@ -259,6 +260,7 @@ test('new HandledPromise expected errors', async t => {
 });
 
 test('new HandledPromise(executor, undefined)', async t => {
+  await null;
   const handledP = new HandledPromise((_, _2, resolveWithPresence) => {
     setTimeout(() => {
       const o = {
@@ -308,6 +310,7 @@ test('handled promises are promises', t => {
 });
 
 test('eventual send expected errors', async t => {
+  await null;
   t.is(
     await HandledPromise.get(true, 'toString'),
     true.toString,

packages/eventual-send/test/thenable.test.js

@@ -20,6 +20,7 @@ test(
 );
 
 const verifyThenAttack = async (t, resolve) => {
+  await null;
   const p = new Promise(_ => {});
   let getHappened = false;
   let callHappened = false;

packages/exo/test/exo-only-throwables.test.js

@@ -24,6 +24,7 @@ const thrower = makeExo(
 );
 
 test('exo only throwables', async t => {
+  await null;
   const e = makeError('test error', undefined, {
     sanitize: false,
   });

packages/exo/test/legacy-guard-tolerance.test.js

@@ -16,6 +16,7 @@ import {
 import { makeExo } from '../src/exo-makers.js';
 
 test('legacy guard tolerance', async t => {
+  await null;
   const aag = M.await(88);
   const laag = makeLegacyAwaitArgGuard({
     argGuard: 88,