fix(commands): reject an in-process dispatch that overlaps a running one

Each in-process dispatch primes its command's options into the CLI-wide
options service and restores the table when it ends. The restore closes
over a snapshot, which is only right when dispatches nest. Two dispatches
overlapping without nesting - a plugin awaiting two runCommand calls
under one Promise.all - restored each other's snapshots and left an
option declared by only one of them in the table for the rest of the
process.

A dispatch may now start only when nothing is in flight or from inside
the innermost dispatch in flight, which the dispatcher tells apart with
an AsyncLocalStorage context per dispatch. Anything else is rejected,
before any state is touched, with an error naming both commands and
saying that dispatches must nest.

Author: edusperoni

defining-commands.md

@@ -889,6 +889,15 @@ declarations into it rewrites the values the host process is still running on
 — `open|ios` declares `watch: false`, which would otherwise leave an `ns start`
 out of watch mode for the rest of its life.
 
+In-process dispatches nest; they never overlap. The options are put back in
+the order the dispatches were entered, which only restores the right values
+when each one finishes before the dispatch it was started from. A dispatch
+started while another is in flight, and not from inside it — two
+`runCommand` calls under one `Promise.all`, say — is rejected with
+`Cannot dispatch '…' in process while '…' is still running: in-process
+dispatches must nest, not overlap; await the running one first.` Await one
+before starting the next.
+
 There is deliberately no free `runCommand()` function: one that silently fell
 back to the CLI's root injector outside an injection context would dispatch
 through the wrong scope from exactly the places — after an `await`, inside a

lib/common/contracts/commands-service.ts

@@ -27,6 +27,9 @@ export abstract class CommandsService {
 	 * registered — the typed way to refer to a command. A parent name is routed
 	 * to its subcommand as the command line routes it, and a subcommand fires
 	 * its full hook name (`before-open-ios`) as well as its parent's.
+	 *
+	 * Dispatches nest: one may start from inside a running dispatch, but one
+	 * started while another is in flight and not from inside it is rejected.
 	 */
 	abstract runCommand(
 		command: CommandReference,

lib/common/services/commands-service.ts

@@ -5,6 +5,7 @@ import {
 	ERROR_NO_VALID_SUBCOMMAND_FORMAT,
 } from "../constants";
 import { EOL } from "os";
+import { AsyncLocalStorage } from "node:async_hooks";
 import * as _ from "lodash";
 import { IOptions, IOptionsTracker } from "../../declarations";
 import { IErrors, IHooksService, IAnalyticsService } from "../declarations";
@@ -22,6 +23,10 @@ import {
 	ISimilarCommand,
 } from "../definitions/commands";
 
+interface IInProcessDispatch {
+	commandName: string;
+}
+
 class CommandArgumentsValidationHelper {
 	constructor(
 		public isValid: boolean,
@@ -42,10 +47,19 @@ export class CommandsService
 	}
 
 	private commands: ICommandData[] = [];
-	private inProcessDepth: number = 0;
+
+	/**
+	 * The in-process dispatches in flight, innermost last, and the one whose
+	 * async context the running code belongs to. Each dispatch primes the
+	 * options service and puts it back when it ends, which only restores the
+	 * right table when dispatches nest; a dispatch may therefore start only from
+	 * inside the innermost one in flight.
+	 */
+	private inProcessDispatches: IInProcessDispatch[] = [];
+	private dispatchContext = new AsyncLocalStorage<IInProcessDispatch>();
 
 	public get isExecutingInProcess(): boolean {
-		return this.inProcessDepth > 0;
+		return this.inProcessDispatches.length > 0;
 	}
 
 	constructor(
@@ -299,50 +313,49 @@ export class CommandsService
 		// Known before the lookup, so a failure to resolve reports under the name
 		// the caller used.
 		let commandName = typeof reference === "string" ? reference : undefined;
-		this.inProcessDepth++;
 		try {
-			const resolved = this.resolveReference(reference, commandArguments);
-			const command = resolved.command;
-			commandName = resolved.commandName;
-			commandArguments = resolved.commandArguments;
+			await this.dispatchInProcess(reference, async (dispatch) => {
+				const resolved = this.resolveReference(reference, commandArguments);
+				const command = resolved.command;
+				commandName = dispatch.commandName = resolved.commandName;
+				commandArguments = resolved.commandArguments;
+
+				this.commands.push({ commandName, commandArguments });
+				const restoreOptions = this.primeOptions(command);
+				try {
+					if (
+						!(await this.canExecuteResolvedCommand(
+							commandName,
+							commandArguments,
+							undefined,
+							command,
+						))
+					) {
+						let commandWithArgs = commandName;
+						if (commandArguments && commandArguments.length) {
+							commandWithArgs += ` ${commandArguments.join(" ")}`;
+						}
+						this.$errors.failWithHelp(
+							`Command '${commandWithArgs}' cannot be executed.`,
+						);
+					}
 
-			this.commands.push({ commandName, commandArguments });
-			const restoreOptions = this.primeOptions(command);
-			try {
-				if (
-					!(await this.canExecuteResolvedCommand(
+					await this.runResolvedCommandInProcess(
+						command,
 						commandName,
 						commandArguments,
-						undefined,
-						command,
-					))
-				) {
-					let commandWithArgs = commandName;
-					if (commandArguments && commandArguments.length) {
-						commandWithArgs += ` ${commandArguments.join(" ")}`;
-					}
-					this.$errors.failWithHelp(
-						`Command '${commandWithArgs}' cannot be executed.`,
 					);
+				} finally {
+					restoreOptions();
+					this.commands.pop();
 				}
-
-				await this.runResolvedCommandInProcess(
-					command,
-					commandName,
-					commandArguments,
-				);
-			} finally {
-				restoreOptions();
-				this.commands.pop();
-			}
+			});
 		} catch (ex) {
 			await this.$errors.reportCommandError(ex, () =>
 				this.printHelpSuggestion(commandName),
 			);
 
 			throw ex;
-		} finally {
-			this.inProcessDepth--;
 		}
 	}
 
@@ -357,10 +370,10 @@ export class CommandsService
 		reference: CommandReference,
 		commandArguments: string[] = [],
 	): Promise<boolean> {
-		this.inProcessDepth++;
-		try {
+		return this.dispatchInProcess(reference, async (dispatch) => {
 			const resolved = this.resolveReference(reference, commandArguments);
 			const { commandName, command } = resolved;
+			dispatch.commandName = commandName;
 			commandArguments = resolved.commandArguments;
 
 			this.commands.push({ commandName, commandArguments });
@@ -376,11 +389,47 @@ export class CommandsService
 				restoreOptions();
 				this.commands.pop();
 			}
+		});
+	}
+
+	/**
+	 * Runs `body` as an in-process dispatch, rejecting it when another dispatch
+	 * is in flight and this one was not started from inside it.
+	 */
+	private async dispatchInProcess<T>(
+		reference: CommandReference,
+		body: (dispatch: IInProcessDispatch) => Promise<T>,
+	): Promise<T> {
+		const running = _.last(this.inProcessDispatches);
+		if (running && this.dispatchContext.getStore() !== running) {
+			throw new Error(
+				`Cannot dispatch '${this.describeReference(reference)}' in process ` +
+					`while '${helpers.stringReplaceAll(running.commandName, "|", " ")}' ` +
+					"is still running: in-process dispatches must nest, not overlap; " +
+					"await the running one first.",
+			);
+		}
+
+		const dispatch: IInProcessDispatch = {
+			commandName: this.describeReference(reference),
+		};
+		this.inProcessDispatches.push(dispatch);
+		try {
+			return await this.dispatchContext.run(dispatch, () => body(dispatch));
 		} finally {
-			this.inProcessDepth--;
+			this.inProcessDispatches.pop();
 		}
 	}
 
+	private describeReference(reference: CommandReference): string {
+		const definition =
+			typeof reference === "string" ? null : toCommandDefinition(reference);
+		const name = definition
+			? _.castArray(definition.name)[0]
+			: String(reference);
+		return helpers.stringReplaceAll(name, "|", " ");
+	}
+
 	/** @deprecated Use {@link runCommand}. */
 	public executeCommandInProcess(
 		commandName: string,