[workers-auth] Use client-specific temporary-account guidance and structured auth errors

.changeset/auth-consumer-guidance.md

@@ -0,0 +1,7 @@
+---
+"@cloudflare/workers-auth": minor
+---
+
+Use each CLI's temporary-account flag in authentication errors
+
+Default to `--temporary-account` while keeping Wrangler's `--temporary` spelling. Expose account-lookup authentication failure reasons so consumers can distinguish missing credentials from expired tokens without matching error messages.

packages/workers-auth/src/cf/index.ts

@@ -68,6 +68,7 @@ export type { UserPreferences } from "../core/preferences";
 export const CF_CLI: CliDescriptor = {
 	cliName: CF_CLI_NAME,
 	displayName: CF_DISPLAY_NAME,
+	temporaryAccountFlag: "--temporary-account",
 	commands: {
 		login: "cf auth login",
 		logout: "cf auth logout",
@@ -142,3 +143,5 @@ export function setKeyringPreference(
 ): SetKeyringPreferenceResult {
 	return cfKeyringPreference.setKeyringPreference(enabled, ctx);
 }
+
+export { AuthenticationError } from "../core/authentication-error";

packages/workers-auth/src/context.ts

@@ -13,6 +13,8 @@ export interface OAuthFlowTemporaryContext {
 	storage: TemporaryAccountStorage;
 	/** Consumer name used in temporary-account errors. */
 	cliDisplayName?: string;
+	/** Consumer temporary-account flag spelling. Defaults to `--temporary-account`. */
+	temporaryAccountFlag?: string;
 	/** Consumer command used to clear a cached temporary account. */
 	logoutCommand?: string;
 	/**

packages/workers-auth/src/core/authentication-error.ts

@@ -0,0 +1,13 @@
+import { UserError } from "@cloudflare/workers-utils";
+import type { LoginOrRefreshFailureReason } from "../flow";
+
+/** Authentication failed before an API request, with a machine-readable reason. */
+export class AuthenticationError extends UserError {
+	constructor(
+		message: string,
+		public readonly reason: LoginOrRefreshFailureReason,
+		telemetryMessage: string
+	) {
+		super(message, { telemetryMessage });
+	}
+}

packages/workers-auth/src/core/factory.ts

@@ -32,6 +32,8 @@ import { createCredentialStorageContext } from "../credential-store";
 import { getAuthFromEnv } from "../credentials";
 import { getCloudflareAccountIdFromEnv as getAccountIdFromEnv } from "../env-vars";
 import { createOAuthFlow } from "../flow";
+import { DEFAULT_TEMPORARY_ACCOUNT_FLAG } from "../temporary";
+import { AuthenticationError } from "./authentication-error";
 import { createFileStorage } from "./file-storage";
 import { createPreferences } from "./preferences";
 import { createTemporaryTermsPrompt } from "./temporary-terms";
@@ -84,7 +86,7 @@ export interface CloudflareAuth {
 	getActiveProfile: () => string;
 	/** The currently-active credential store for the active profile. */
 	getCredentialStore: () => CredentialStore;
-	/** Mark whether `--temporary` is permitted for the current invocation. */
+	/** Mark whether temporary-account mode is permitted for the current invocation. */
 	setTemporaryAllowed: (
 		allowed: boolean,
 		request?: TemporaryAccountRequest
@@ -197,6 +199,8 @@ export function createCloudflareAuth(
 	const { logger } = ctx;
 	const temporaryAccountLogger = ctx.temporaryAccountLogger ?? logger;
 	const cliName = descriptor.cliName;
+	const temporaryAccountFlag =
+		descriptor.temporaryAccountFlag ?? DEFAULT_TEMPORARY_ACCOUNT_FLAG;
 	const NOT_LOGGED_IN_ERROR_BODIES = notLoggedInErrorBodies(
 		descriptor.commands.login
 	);
@@ -267,6 +271,7 @@ export function createCloudflareAuth(
 			),
 			cliDisplayName: descriptor.displayName,
 			logoutCommand: descriptor.commands.logout,
+			temporaryAccountFlag,
 			prompt: createTemporaryTermsPrompt({
 				temporaryAccountLogger,
 				prompt: ctx.prompt,
@@ -428,9 +433,10 @@ export function createCloudflareAuth(
 		const { throwOnEmpty = true } = options;
 		const loginResult = await loginOrRefreshIfRequired(complianceConfig);
 		if (!loginResult.loggedIn) {
-			throw new UserError(
+			throw new AuthenticationError(
 				`Not logged in. ${NOT_LOGGED_IN_ERROR_BODIES[loginResult.reason]}${NOT_LOGGED_IN_WHOAMI_TIP}`,
-				{ telemetryMessage: "cfetch auth login required" }
+				loginResult.reason,
+				"cfetch auth login required"
 			);
 		}
 		const credentials = requireApiToken();
@@ -598,12 +604,12 @@ ${accounts
 				return latchedTemporaryAccount.account.id;
 			}
 
-			// `--temporary` is only for unauthenticated use. If any credentials are
+			// Temporary-account mode is only for unauthenticated use. If credentials are
 			// already available (env, global key, or a stored OAuth token), refuse
 			// rather than silently provisioning a throwaway account alongside them.
 			if (getAPIToken()) {
 				throw new UserError(
-					"You're already authenticated with Cloudflare, so `--temporary` can't be used. Temporary preview accounts are only for unauthenticated use. Either remove `--temporary` to use your existing account, or log out (and unset CLOUDFLARE_API_TOKEN) first.",
+					`You're already authenticated with Cloudflare, so \`${temporaryAccountFlag}\` can't be used. Temporary preview accounts are only for unauthenticated use. Either remove \`${temporaryAccountFlag}\` to use your existing account, or log out (and unset CLOUDFLARE_API_TOKEN) first.`,
 					{
 						telemetryMessage: "user temporary account already authenticated",
 					}

packages/workers-auth/src/core/types.ts

@@ -70,6 +70,9 @@ export interface CliDescriptor {
 	 */
 	displayName: string;
 
+	/** Temporary-account flag spelling in user guidance. Defaults to `--temporary-account`. */
+	temporaryAccountFlag?: string;
+
 	/** Commands surfaced in auth guidance. */
 	commands: {
 		login: string;

packages/workers-auth/src/index.ts

@@ -111,3 +111,5 @@ export type {
 	NpmRunner,
 	ScrubEncryptedCredentialsResult,
 } from "./credential-store";
+
+export { AuthenticationError } from "./core/authentication-error";

packages/workers-auth/src/temporary.ts

@@ -16,6 +16,9 @@ import type {
 } from "./context";
 import type { PowSolution } from "./pow";
 
+/** Flag spelling used by consumers that don't specify their own. */
+export const DEFAULT_TEMPORARY_ACCOUNT_FLAG = "--temporary-account";
+
 export const TEMPORARY_TERMS_URLS = {
 	termsOfService: "https://www.cloudflare.com/terms/",
 	privacyPolicy: "https://www.cloudflare.com/privacypolicy/",
@@ -24,8 +27,6 @@ export const TEMPORARY_TERMS_URLS = {
 export const TEMPORARY_TERMS_PROMPT = `You must accept Cloudflare's Terms of Service (${TEMPORARY_TERMS_URLS.termsOfService}) and Privacy Policy (${TEMPORARY_TERMS_URLS.privacyPolicy}) in order to continue. By typing "yes", you agree to these terms. Type "yes" to continue.`;
 export const TEMPORARY_TERMS_NOTICE = `Continuing means you accept Cloudflare's Terms of Service (${TEMPORARY_TERMS_URLS.termsOfService}) and Privacy Policy (${TEMPORARY_TERMS_URLS.privacyPolicy}).`;
 
-const TEMPORARY_TERMS_ERROR = `You must accept Cloudflare's Terms of Service (${TEMPORARY_TERMS_URLS.termsOfService}) and Privacy Policy (${TEMPORARY_TERMS_URLS.privacyPolicy}) to use --temporary.`;
-
 type TemporaryAccountPayload = {
 	account?: {
 		id?: string;
@@ -386,6 +387,8 @@ export async function getOrCreateTemporaryPreviewAccount(options: {
 	temporaryAccountLogger: TemporaryAccountLogger;
 	/** Consumer name used in temporary-account errors. */
 	cliDisplayName?: string;
+	/** Consumer temporary-account flag spelling. Defaults to `--temporary-account`. */
+	temporaryAccountFlag?: string;
 	/** Consumer command used to clear a cached temporary account. */
 	logoutCommand?: string;
 	request?: TemporaryAccountRequest;
@@ -414,9 +417,10 @@ export async function getOrCreateTemporaryPreviewAccount(options: {
 	);
 
 	if (!termsAccepted) {
-		throw new UserError(TEMPORARY_TERMS_ERROR, {
-			telemetryMessage: "user temporary terms not accepted",
-		});
+		throw new UserError(
+			`You must accept Cloudflare's Terms of Service (${TEMPORARY_TERMS_URLS.termsOfService}) and Privacy Policy (${TEMPORARY_TERMS_URLS.privacyPolicy}) to use ${options.temporaryAccountFlag ?? DEFAULT_TEMPORARY_ACCOUNT_FLAG}.`,
+			{ telemetryMessage: "user temporary terms not accepted" }
+		);
 	}
 
 	const temporaryPreviewAccount = await createTemporaryPreviewAccount(

packages/workers-auth/src/wrangler/index.ts

@@ -70,6 +70,7 @@ export type WranglerLoginProps = CloudflareLoginProps;
 export const WRANGLER_CLI: CliDescriptor = {
 	cliName: WRANGLER_CLI_NAME,
 	displayName: WRANGLER_DISPLAY_NAME,
+	temporaryAccountFlag: "--temporary",
 	commands: {
 		login: "wrangler login",
 		logout: "wrangler logout",
@@ -153,3 +154,5 @@ export function setKeyringPreference(
 }
 
 export type { KeyringPreferenceContext } from "../core/keyring-preference";
+
+export { AuthenticationError } from "../core/authentication-error";

packages/workers-auth/tests/core/factory.test.ts

@@ -4,17 +4,20 @@ import { CF_CLI } from "../../src/cf";
 import { createCloudflareAuth } from "../../src/core/factory";
 import { createOAuthFlow } from "../../src/flow";
 import type { AuthContext } from "../../src/core/types";
+import type { LoginOrRefreshResult } from "../../src/flow";
 import type { ComplianceConfig } from "@cloudflare/workers-utils";
 
 const fetchInternalBase = vi.hoisted(() => vi.fn());
 const flow = vi.hoisted(() => ({
 	activateTemporaryAccount: vi.fn(),
 	getActiveProfile: vi.fn(() => "default"),
 	getActiveTemporaryAccount: vi.fn(() => undefined),
-	getAPIToken: vi.fn(() => undefined),
+	getAPIToken: vi.fn((): { apiToken: string } | undefined => undefined),
 	isTemporaryAllowed: vi.fn(() => false),
 	login: vi.fn(async () => true),
-	loginOrRefreshIfRequired: vi.fn(async () => ({ loggedIn: true as const })),
+	loginOrRefreshIfRequired: vi.fn(async (): Promise<LoginOrRefreshResult> => ({
+		loggedIn: true,
+	})),
 	requireApiToken: vi.fn(() => ({ apiToken: "test-token" })),
 }));
 
@@ -185,3 +188,44 @@ describe("temporary account status messages", () => {
 		);
 	});
 });
+
+describe("consumer auth guidance", () => {
+	runInTempDir();
+
+	beforeEach(() => {
+		vi.clearAllMocks();
+	});
+
+	it("defaults to cf's temporary-account flag in auth guidance", async ({
+		expect,
+	}) => {
+		flow.isTemporaryAllowed.mockReturnValue(true);
+		flow.getAPIToken.mockReturnValue({ apiToken: "existing-token" });
+		const auth = createCloudflareAuth(
+			{ ...CF_CLI, temporaryAccountFlag: undefined },
+			createTestContext()
+		);
+
+		await expect(auth.requireAuth(COMPLIANCE_CONFIG)).rejects.toThrow(
+			"Either remove `--temporary-account` to use your existing account"
+		);
+	});
+
+	it("preserves the authentication failure reason on account lookup", async ({
+		expect,
+	}) => {
+		flow.isTemporaryAllowed.mockReturnValue(false);
+		flow.loginOrRefreshIfRequired.mockResolvedValue({
+			loggedIn: false,
+			reason: "no-credentials-non-interactive",
+		});
+		const auth = createCloudflareAuth(CF_CLI, createTestContext());
+
+		await expect(
+			auth.fetchAllAccounts(COMPLIANCE_CONFIG)
+		).rejects.toMatchObject({
+			reason: "no-credentials-non-interactive",
+			telemetryMessage: "cfetch auth login required",
+		});
+	});
+});

packages/wrangler/src/__tests__/api/startDevWorker/BundleController.test.ts

@@ -1,3 +1,4 @@
+import assert from "node:assert";
 import { existsSync, readdirSync, readFileSync } from "node:fs";
 import path from "node:path";
 import { setTimeout } from "node:timers/promises";
@@ -6,15 +7,22 @@ import {
 	runInTempDir,
 	seed,
 } from "@cloudflare/workers-utils/test-helpers";
+import * as chokidar from "chokidar";
 import * as esbuild from "esbuild";
 import dedent from "ts-dedent";
 import { afterEach, beforeEach, describe, test, vi } from "vitest";
 import { BundlerController } from "../../../api/startDevWorker/BundlerController";
+import * as additionalModules from "../../../deployment-bundle/find-additional-modules";
 import { runCustomBuild } from "../../../deployment-bundle/run-custom-build";
 import { FakeBus } from "../../helpers/fake-bus";
 import { mockConsoleMethods } from "../../helpers/mock-console";
 import type { StartDevWorkerOptions } from "../../../api";
 
+vi.mock("chokidar", async (importOriginal) => {
+	const actual = await importOriginal<typeof import("chokidar")>();
+	return { ...actual, watch: vi.fn(actual.watch) };
+});
+
 // Find the bundled result of a particular source file
 function findSourceFile(source: string, name: string): string {
 	const startIndex = source.indexOf(`// ${name}`);
@@ -285,6 +293,46 @@ describe("BundleController", { retry: 5, timeout: 10_000 }, () => {
 			}
 		);
 
+		test("teardown waits for an in-flight unbundled module scan", async ({
+			expect,
+		}) => {
+			await seed({ "src/index.ts": "export class Api {}" });
+			const watch = vi.spyOn(chokidar, "watch");
+			const initialBuild = bus.waitFor("bundleComplete");
+			controller.onConfigUpdate({
+				type: "configUpdate",
+				config: configDefaults({ build: { bundle: false } }),
+			});
+			await initialBuild;
+			const watcher = watch.mock.results.at(-1)?.value;
+			assert(watcher);
+
+			const started = Promise.withResolvers<void>();
+			const resume = Promise.withResolvers<void>();
+			const findModules = additionalModules.findAdditionalModules;
+			const scan = vi
+				.spyOn(additionalModules, "findAdditionalModules")
+				.mockImplementationOnce(async (...args) => {
+					started.resolve();
+					await resume.promise;
+					return findModules(...args);
+				});
+			watcher.emit("change", path.resolve("src/index.ts"));
+			await started.promise;
+			const finished = vi.fn();
+			const teardown = controller.teardown().then(finished);
+			try {
+				await watcher.close();
+				await setTimeout(0);
+				expect(finished).not.toHaveBeenCalled();
+			} finally {
+				resume.resolve();
+				await teardown;
+				scan.mockRestore();
+				watch.mockRestore();
+			}
+		});
+
 		test("a watch-mode rebuild failure emits an error event and recovers", async ({
 			expect,
 		}) => {

packages/wrangler/src/__tests__/core/handle-errors.test.ts

@@ -1,3 +1,4 @@
+import { AuthenticationError } from "@cloudflare/workers-auth/wrangler";
 import { APIError, ParseError } from "@cloudflare/workers-utils";
 import { Cloudflare } from "cloudflare";
 import { beforeEach, describe, it, vi } from "vitest";
@@ -263,6 +264,18 @@ describe("getErrorType", () => {
 	});
 
 	describe("Authentication errors", () => {
+		it("preserves the UserError telemetry type for account lookup failures", ({
+			expect,
+		}) => {
+			const error = new AuthenticationError(
+				"Not logged in.",
+				"no-credentials-non-interactive",
+				"cfetch auth login required"
+			);
+
+			expect(getErrorType(error)).toBe("UserError");
+		});
+
 		it("should return 'AuthenticationError' for code 10000 (expired/revoked token)", ({
 			expect,
 		}) => {

packages/wrangler/src/core/handle-errors.ts

@@ -1,5 +1,6 @@
 import assert from "node:assert";
 import { ApiError } from "@cloudflare/containers-shared";
+import { AuthenticationError } from "@cloudflare/workers-auth/wrangler";
 import {
 	APIError,
 	CommandLineArgsError,
@@ -305,6 +306,10 @@ export function getErrorType(e: unknown): string | undefined {
 	if (isBuildFailure(e) || isBuildFailureFromCause(e)) {
 		return "BuildFailure";
 	}
+	// Account lookup used UserError before exposing structured auth failures.
+	if (e instanceof AuthenticationError) {
+		return "UserError";
+	}
 	return e instanceof Error ? e.constructor.name : undefined;
 }
 

packages/wrangler/src/dev/use-esbuild.ts

@@ -287,6 +287,7 @@ export function runBuild(
 		// if "noBundle" is true, then we need to manually watch all modules and
 		// trigger "builds" when any change
 		if (noBundle && watch) {
+			let pendingRebuild = Promise.resolve();
 			const watching = [path.resolve(entry.moduleRoot)];
 			const watcher = watchPaths(watching, {
 				persistent: true,
@@ -296,12 +297,22 @@ export function runBuild(
 				// chokidar v4 normalises paths to forward slashes before
 				// matching, so a regex on path segments works cross-platform.
 				ignored: /[/\\](\.git|node_modules|\.wrangler)([/\\]|$)/,
-			}).on("change", async (_event) => {
-				await refreshExports();
-				await updateBundle();
+			}).on("change", () => {
+				pendingRebuild = pendingRebuild
+					.then(async () => {
+						await refreshExports();
+						await updateBundle();
+					})
+					.catch(onErr);
 			});
 
-			stopWatching = () => watcher.close();
+			stopWatching = async () => {
+				await watcher.close();
+				// Closing chokidar doesn't wait for asynchronous change handlers.
+				// Drain them before callers remove the source and build directories.
+				await pendingRebuild;
+				await bundleResult?.stop?.();
+			};
 		}
 		const entrypointPath = realpathSync(
 			bundleResult?.resolvedEntryPointPath ?? entry.file