fix(cli): stub the API client over its real exports in tests (#11363)

* fix(cli): stub the API client over its real exports in tests

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix(cli): mock the API client once and dispatch to per-suite stubs

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

cli/TESTING.md

@@ -53,9 +53,17 @@ in-process suite imports.** Check with `grep -rl "<exported fn>" test/` before r
 for one. A suite that drives the CLI through a spawned process is out of reach of a
 module mock and doesn't count.
 
-`raw_app_push_policy_unit.test.ts` is the worked example: it stubs `gen/services.gen.ts`,
-which passes the rule because nothing else in `test/` imports the three API functions it
-replaces, and deliberately does not stub `bundle.ts`, which failed it.
+The API client `gen/services.gen.ts` is the exception, and is stubbed only through
+`mockServices` in `test/mock_services.ts`. Bun fixes a mocked module's export names at
+the first `mock.module` call of the run, so a stub that replaced the module with just the
+functions its suite needed left every other name undefined for the rest of the run, even
+for suites that stubbed that name themselves. `test/mock_services.ts` therefore calls
+`mock.module` exactly once, with every real export behind a dispatcher; `mockServices`
+only swaps which stubs the dispatcher routes to, so it doesn't rely on the unreliable
+`afterAll` re-mock described above.
+
+`raw_app_push_policy_unit.test.ts` deliberately does not stub `bundle.ts`, which failed
+the rule.
 
 ## AI Benchmark Caveats
 

cli/test/instance_configs_unit.test.ts

@@ -8,7 +8,8 @@
  * - pushInstanceConfigs skips unchanged configs
  */
 
-import { expect, test, describe, beforeEach, afterEach, mock } from "bun:test";
+import { expect, test, describe, beforeEach, afterEach } from "bun:test";
+import { mockServices } from "./mock_services.ts";
 import { writeFile, readFile, mkdir, rm } from "node:fs/promises";
 import { mkdtempSync } from "node:fs";
 import { tmpdir } from "node:os";
@@ -21,7 +22,7 @@ let updateConfigCalls: { name: string; requestBody: any }[] = [];
 let deleteConfigCalls: { name: string }[] = [];
 
 // Mock the wmill module before importing settings.ts
-mock.module("../gen/services.gen.ts", () => ({
+mockServices({
   listWorkerGroups: async () => listWorkerGroupsResult,
   updateConfig: async (args: { name: string; requestBody: any }) => {
     updateConfigCalls.push(args);
@@ -32,7 +33,7 @@ mock.module("../gen/services.gen.ts", () => ({
   listConfigs: async () => {
     throw new Error("listConfigs should not be called");
   },
-}));
+});
 
 import {
   pullInstanceConfigs,

cli/test/mock_services.ts

@@ -0,0 +1,35 @@
+import { afterAll, mock } from "bun:test";
+
+// Every in-process suite that stubs the API client goes through here. Bun
+// fixes a mocked module's export names at the first `mock.module` call of the
+// run, so a stub missing a name leaves it undefined for every later suite, even
+// ones that stub it themselves. The module is therefore mocked once, with every
+// real export behind a dispatcher: a suite swaps its stubs in and out of
+// `active`, and never depends on a later `mock.module` call taking effect.
+const real = { ...(await import("../gen/services.gen.ts")) } as Record<
+  string,
+  unknown
+>;
+let active: Record<string, unknown> = {};
+
+const dispatched: Record<string, unknown> = {};
+for (const [name, value] of Object.entries(real)) {
+  dispatched[name] =
+    typeof value === "function"
+      ? (...args: unknown[]) =>
+          ((active[name] ?? value) as (...a: unknown[]) => unknown)(...args)
+      : value;
+}
+mock.module("../gen/services.gen.ts", () => dispatched);
+
+export function mockServices(stubs: Record<string, unknown>): void {
+  for (const name of Object.keys(stubs)) {
+    if (!(name in real)) {
+      throw new Error(`mockServices: services.gen.ts has no export ${name}`);
+    }
+  }
+  active = stubs;
+  afterAll(() => {
+    active = {};
+  });
+}

cli/test/push_workspace_key_unit.test.ts

@@ -13,7 +13,8 @@
  * - WMILL_NO_REENCRYPT_ON_KEY_CHANGE=true does the same via env var
  */
 
-import { expect, test, describe, beforeEach, afterEach, mock } from "bun:test";
+import { expect, test, describe, beforeEach, afterEach } from "bun:test";
+import { mockServices } from "./mock_services.ts";
 
 // Track calls to mocked wmill functions
 let remoteKey = "";
@@ -23,7 +24,7 @@ let setEncryptionKeyCalls: {
 }[] = [];
 
 // Mock the wmill module before importing settings.ts
-mock.module("../gen/services.gen.ts", () => ({
+mockServices({
   getWorkspaceEncryptionKey: async (_args: { workspace: string }) => ({
     key: remoteKey,
   }),
@@ -33,7 +34,7 @@ mock.module("../gen/services.gen.ts", () => ({
   }) => {
     setEncryptionKeyCalls.push(args);
   },
-}));
+});
 
 import { pushWorkspaceKey } from "../src/core/settings.ts";
 

cli/test/push_workspace_settings_auto_invite_unit.test.ts

@@ -4,7 +4,8 @@
  * instance groups that settings.yaml does not declare.
  */
 
-import { expect, test, describe, beforeEach, mock } from "bun:test";
+import { expect, test, describe, beforeEach } from "bun:test";
+import { mockServices } from "./mock_services.ts";
 
 let editAutoInviteCalls: unknown[] = [];
 let editInstanceGroupsCalls: unknown[] = [];
@@ -17,10 +18,9 @@ const remoteAutoInvite = {
   instance_groups_roles: { eng: "developer" },
 };
 
-// Every wmill.* call reachable from pushWorkspaceSettings is stubbed: bun shares one
-// mocked module across test files, and names missing from whichever mock loads first
-// stay missing for the others.
-mock.module("../gen/services.gen.ts", () => ({
+// Every wmill.* call reachable from pushWorkspaceSettings is stubbed so the
+// function runs without a backend.
+mockServices({
   getSettings: async () => ({ auto_invite: remoteAutoInvite }),
   getWorkspaceName: async () => "phoenix",
   changeWorkspaceName: async () => {},
@@ -45,7 +45,7 @@ mock.module("../gen/services.gen.ts", () => ({
   editSlackCommand: async () => {},
   setWorkspaceSlackOauthConfig: async () => {},
   deleteWorkspaceSlackOauthConfig: async () => {},
-}));
+});
 
 const { pushWorkspaceSettings } = await import("../src/core/settings.ts");
 

cli/test/push_workspace_settings_identity_unit.test.ts

@@ -4,7 +4,8 @@
  * the file carries one. Rationale lives at the apply sites in settings.ts.
  */
 
-import { expect, test, describe, beforeEach, mock } from "bun:test";
+import { expect, test, describe, beforeEach } from "bun:test";
+import { mockServices } from "./mock_services.ts";
 
 let changeWorkspaceNameCalls: unknown[] = [];
 let changeWorkspaceColorCalls: unknown[] = [];
@@ -15,7 +16,7 @@ let remoteWebhook: string | undefined = undefined;
 
 // Every wmill.* call reachable from pushWorkspaceSettings is stubbed so the
 // function runs without a backend; only the three we assert on record calls.
-mock.module("../gen/services.gen.ts", () => ({
+mockServices({
   getSettings: async (_a: { workspace: string }) => ({
     webhook: remoteWebhook,
     color: remoteColor,
@@ -45,7 +46,7 @@ mock.module("../gen/services.gen.ts", () => ({
   editSlackCommand: async () => {},
   setWorkspaceSlackOauthConfig: async () => {},
   deleteWorkspaceSlackOauthConfig: async () => {},
-}));
+});
 
 const { pushWorkspaceSettings } = await import("../src/core/settings.ts");
 

cli/test/raw_app_push_policy_unit.test.ts

@@ -6,26 +6,20 @@
  * file states, and that the markers still close a deployed open app back down.
  */
 
-import { afterAll, beforeEach, expect, mock, test } from "bun:test";
+import { beforeEach, expect, test } from "bun:test";
 import { mkdtemp, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
+import { mockServices } from "./mock_services.ts";
 
 let calls: any[] = [];
 let deployedPolicy: any;
 /** No app deployed at the path: `getAppByPath` 404s and the push creates one. */
 let deployed = true;
 
-// Stub only what no other in-process suite imports, and treat a stub as
-// permanent for the run (see "Module mocks" in cli/TESTING.md). These three API
-// functions qualify — nothing else in `test/` imports them. `bundle.ts` did not:
-// stubbing it left `raw_app_svelte_plugin_unit.test.ts` asserting against an
-// empty bundle, which an `afterAll` hand-back did not prevent. So the real
-// bundler runs instead, on the app each push writes below.
-const realServices = await import("../gen/services.gen.ts");
-
-mock.module("../gen/services.gen.ts", () => ({
-  ...realServices,
+// `bundle.ts` is deliberately not stubbed (see "Module mocks" in
+// cli/TESTING.md): the real bundler runs on the app each push writes below.
+mockServices({
   getAppByPath: async () => {
     if (!deployed) throw new Error("not found");
     return {
@@ -41,12 +35,6 @@ mock.module("../gen/services.gen.ts", () => ({
   createAppRaw: async (a: unknown) => {
     calls.push(a);
   },
-}));
-
-// Belt and braces: nothing else in-process calls these, and a hand-back is not
-// what makes that safe.
-afterAll(() => {
-  mock.module("../gen/services.gen.ts", () => realServices);
 });
 
 const { pushRawApp } = await import("../src/commands/app/raw_apps.ts");

cli/test/schedule_push_permissioned_as_unit.test.ts

@@ -9,6 +9,7 @@ import { expect, test, describe, afterAll, beforeEach, mock } from "bun:test";
 import { mkdtemp, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
+import { mockServices } from "./mock_services.ts";
 
 let updateScheduleCalls: any[] = [];
 let remotePermissionedAs: string | undefined = "u/svc";
@@ -45,7 +46,7 @@ afterAll(() => {
   }
 });
 
-await mockModule("../gen/services.gen.ts", () => ({
+mockServices({
   getSchedule: async () => REMOTE_SCHEDULE(),
   updateSchedule: async (a: unknown) => {
     updateScheduleCalls.push(a);
@@ -56,7 +57,7 @@ await mockModule("../gen/services.gen.ts", () => ({
     is_admin: true,
     groups: [],
   }),
-}));
+});
 
 await mockModule("../src/core/context.ts", (real) => ({
   ...real,

cli/test/shared_ui_diff_unit.test.ts

@@ -5,7 +5,8 @@
  * (including no local folder) when the apply would be a no-op.
  */
 
-import { expect, test, describe, beforeEach, afterEach, mock } from "bun:test";
+import { expect, test, describe, beforeEach, afterEach } from "bun:test";
+import { mockServices } from "./mock_services.ts";
 import * as fs from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
@@ -14,7 +15,7 @@ let remoteFiles: Record<string, string> = {};
 let remoteUnreadable = false;
 let pushedFiles: Record<string, string> | undefined;
 
-mock.module("../gen/services.gen.ts", () => ({
+mockServices({
   getSharedUi: async (_args: { workspace: string }) => {
     if (remoteUnreadable) throw new Error("shared UI store unreadable");
     return { files: remoteFiles };
@@ -25,7 +26,7 @@ mock.module("../gen/services.gen.ts", () => ({
   }) => {
     pushedFiles = args.requestBody.files;
   },
-}));
+});
 
 const { diffSharedUi, pushSharedUi } = await import(
   "../src/commands/shared_ui.ts"