test(auth-server): clean up test mocking leaked state

.claude/skills/fxa-testing-shared/GUIDELINES.md

@@ -236,6 +236,16 @@ No shared mutable state between tests. Each test owns its setup; use `beforeEach
 
 **Mid-test mock resets are a smell.** Calling `mockClear()`, `mockReset()`, `jest.clearAllMocks()`, or `jest.resetAllMocks()` inside an `it()` body is almost always a band-aid hiding state leakage from a prior test or from shared setup — fix the leak (scope the mock per-test, move it into `beforeEach`, stop sharing module-level mocks across describes) instead of clearing inline. The narrow legitimate case is asserting "no further calls happened after this point" in a multi-phase test; in that case, leave a comment explaining why.
 
+**TypeDI Container cleanup is conditional**, not mechanical. Pair `Container.set()` with cleanup *when leakage could change another test's observable behavior*:
+
+- **`afterEach` reset required** when the consumer resolves from Container at test execution time — most unit tests, route handlers tested directly, and lazy `mocks.js`-style factory helpers. A leaked set in test A is observable in test B.
+- **`afterAll` cleanup sufficient** when the consumer captures deps at construction (server-boot integration tests). The running server holds its own captured references; per-test reset has no effect on its behavior. File-level cleanup still matters for cross-file isolation.
+- **Cleanup optional** for empty-stub "satisfy DI" sets (`Container.set(Token, {})`) and intentionally global tokens (file-wide loggers, configs) — both lack test-observable meaning to leak. Prefer fixing the consumer to tolerate the absent dep over leaving the unpaired set.
+
+When in doubt, default to `afterEach` + `Container.reset()` — cheap, complete, and catches tokens you'd otherwise forget.
+
+**Container reference identity matters across `jest.resetModules()`.** A fresh `require('typedi')` after a module reset hands back a different Container; calling `remove()` on it silently no-ops because the entry was set on the prior instance. Capture `const { Container } = require('typedi')` at module or describe scope so both hooks reference the same instance.
+
 ```ts
 // Violation — shared mutable state, order-dependent
 let account: Account;

CLAUDE.md

@@ -115,7 +115,7 @@ Suggest these proactively when the task matches — do not wait to be asked.
 - **Trust, but verify.** When tests and implementation disagree, investigate — don't blindly fix one to match the other.
 - **Mock at system boundaries.** No mocking same-package helpers, no tautological mocks (mock returns `X` → assert `X`). When in doubt, mock less.
 - **Type your mocks.** Prefer `jest.Mocked<T>`; `any` is an escape hatch that loses contract-drift detection.
-- **Tests must be independent.** No shared mutable state; reset in `beforeEach`. `jest.clearAllMocks()` in `beforeEach` is redundant _only_ if the package's `jest.config.*` sets `clearMocks: true` (currently `fxa-auth-server`, `fxa-profile-server`). Mid-`it()` `mockClear`/`mockReset` is a band-aid for leakage — fix the leak.
+- **Tests must be independent.** No shared mutable state; reset in `beforeEach`. `jest.clearAllMocks()` in `beforeEach` is redundant _only_ if the package's `jest.config.*` sets `clearMocks: true` (currently `fxa-auth-server`, `fxa-profile-server`). Mid-`it()` `mockClear`/`mockReset` is a band-aid for leakage — fix the leak. TypeDI `Container.set()` cleanup is conditional — `afterEach` reset for per-call consumers (most unit tests), `afterAll` for boot-captured deps (server-boot integration tests), optional for empty-stub satisfy-DI sets. Watch out for Container identity across `jest.resetModules()`.
 - **Assert explicitly.** Exact values and shapes; avoid `toBeTruthy()`, `expect.anything()`, broad `objectContaining({})`. `expect.any(<Type>)` only for genuinely non-deterministic fields.
 - **Test behavior, not implementation.** Public interfaces only — no asserting on private state.
 - **Test async correctly.** Always `await`; use `jest.useFakeTimers()` + `jest.setSystemTime()`. Bumped per-test timeouts usually mask real-timer or unmocked-I/O bugs.

packages/fxa-auth-server/jest.oauth-api.config.js

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // Standalone config for oauth_api.in.spec.ts which manages its own
-// in-process server via test/lib/server.js (server.inject).  It must
+// in-process server via test/lib/server.ts (server.inject).  It must
 // NOT share the jest-global-setup shared server because both sync
 // different client configs to the same database, causing race conditions.
 

packages/fxa-auth-server/test/lib/server.ts

@@ -68,16 +68,24 @@ function wrapServer(server: any, close: () => Promise<void>) {
 }
 
 export async function start() {
+  let didSetCapabilityService = false;
   if (!Container.has(CapabilityService)) {
     Container.set(CapabilityService, {
       subscriptionCapabilities: jest.fn().mockResolvedValue([]),
       determineClientVisibleSubscriptionCapabilities: jest
         .fn()
         .mockResolvedValue(''),
     });
+    didSetCapabilityService = true;
   }
   const { server, close } = await createServer(testConfig);
-  return wrapServer(server, close);
+  const wrappedClose = async () => {
+    await close();
+    if (didSetCapabilityService) {
+      Container.remove(CapabilityService);
+    }
+  };
+  return wrapServer(server, wrappedClose);
 }
 
 export { config };

packages/fxa-auth-server/test/remote/mfa_totp.in.spec.ts

@@ -42,6 +42,8 @@ beforeAll(async () => {
 
 afterAll(async () => {
   await server.stop();
+  Container.remove(PlaySubscriptions);
+  Container.remove(AppStoreSubscriptions);
 });
 
 const testVersions = [

packages/fxa-auth-server/test/remote/oauth_token_route.in.spec.ts

@@ -78,10 +78,14 @@ const tokenRoutesArgMocks = {
   glean: mockGlean,
 };
 
+// Captured at module scope so beforeAll's set and afterAll's remove use the
+// same Container instance; beforeAll calls jest.resetModules() after the set,
+// and a fresh require would otherwise hand back a different Container.
+const { Container } = require('typedi');
+
 let tokenRoutes: any;
 
 beforeAll(() => {
-  const { Container } = require('typedi');
   Container.set('OAuthClientInfo', {
     async fetch() {
       return 'sync';
@@ -115,6 +119,10 @@ beforeAll(() => {
   tokenRoutes = tokenRouteFactory(tokenRoutesArgMocks);
 });
 
+afterAll(() => {
+  Container.remove('OAuthClientInfo');
+});
+
 describe('/token POST (integration)', () => {
   let route: any;
 

packages/fxa-auth-server/test/remote/passkeys.in.spec.ts

@@ -90,10 +90,6 @@ afterAll(async () => {
   Container.remove(PasskeyService);
 });
 
-beforeEach(() => {
-  jest.clearAllMocks();
-});
-
 const password = 'pssssst';
 
 async function getMfaAccessTokenForPasskey(clientInstance: any) {

packages/fxa-auth-server/test/remote/payments/configuration/manager.in.spec.ts

@@ -162,7 +162,6 @@ describe('#integration - PaymentConfigManager', () => {
       planConfigDbRef,
       100
     );
-    jest.clearAllMocks();
     Container.reset();
   });
 

packages/fxa-auth-server/test/remote/subscription_tests.in.spec.ts

@@ -55,8 +55,7 @@ interface OAuthClient {
 }
 
 // Load config to extract client IDs and constants
-const configPath = require.resolve('../../config');
-delete require.cache[configPath];
+jest.resetModules();
 const localConfig = require('../../config').default.getProperties();
 
 const validClients: OAuthClient[] = localConfig.oauthServer.clients.filter(
@@ -100,36 +99,30 @@ describe('#integration - remote subscriptions (enabled)', () => {
   // eslint-disable-next-line @typescript-eslint/no-require-imports
   const { OAUTH_SCOPE_SUBSCRIPTIONS } = require('fxa-shared/oauth/constants');
   // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { default: Container } = require('typedi');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { CapabilityService } = require('../../lib/payments/capability');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { AuthLogger, AppConfig } = require('../../lib/types');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { ProfileClient } = require('@fxa/profile/client');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const {
-    PlaySubscriptions,
-  } = require('../../lib/payments/iap/google-play/subscriptions');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const {
-    AppStoreSubscriptions,
-  } = require('../../lib/payments/iap/apple-app-store/subscriptions');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { CapabilityManager } = require('@fxa/payments/capability');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { BackupCodeManager } = require('@fxa/accounts/two-factor');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { RecoveryPhoneService } = require('@fxa/accounts/recovery-phone');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { PriceManager } = require('@fxa/payments/customer');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  const { ProductConfigurationManager } = require('@fxa/shared/cms');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
   const { AppError: error } = require('@fxa/accounts/errors');
   // eslint-disable-next-line @typescript-eslint/no-require-imports
   const EventEmitter = require('events');
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
+
+  // Container and its token classes are required INSIDE beforeAll after
+  // jest.resetModules() so they share module identity with key_server's
+  // runtime view of the same modules. Requiring them here at describe scope
+  // (before any resetModules) would let the Container.set token classes
+  // drift out of sync with what key_server re-requires after the reset,
+  // making Container.get() silently miss on class identity.
+  // See GUIDELINES.md Rule 7.
+  let Container: any;
+  let CapabilityService: any;
+  let StripeHelper: any;
+  let AuthLogger: any;
+  let AppConfig: any;
+  let ProfileClient: any;
+  let PlaySubscriptions: any;
+  let AppStoreSubscriptions: any;
+  let CapabilityManager: any;
+  let BackupCodeManager: any;
+  let RecoveryPhoneService: any;
+  let PriceManager: any;
+  let ProductConfigurationManager: any;
 
   let server: any;
   let serverUrl: string;
@@ -162,8 +155,29 @@ describe('#integration - remote subscriptions (enabled)', () => {
   const mockProfileClient: Record<string, any> = {};
 
   beforeAll(async () => {
-    // Fresh config for the in-process server
-    delete require.cache[require.resolve('../../config')];
+    // Fresh config for the in-process server. resetModules() before any
+    // typedi or token-class requires ensures the modules we capture here
+    // share identity with what key_server re-requires below — Container.set
+    // tokens line up with Container.get inside the in-process server.
+    jest.resetModules();
+    /* eslint-disable @typescript-eslint/no-require-imports */
+    ({ default: Container } = require('typedi'));
+    ({ CapabilityService } = require('../../lib/payments/capability'));
+    ({ StripeHelper } = require('../../lib/payments/stripe'));
+    ({ AuthLogger, AppConfig } = require('../../lib/types'));
+    ({ ProfileClient } = require('@fxa/profile/client'));
+    ({
+      PlaySubscriptions,
+    } = require('../../lib/payments/iap/google-play/subscriptions'));
+    ({
+      AppStoreSubscriptions,
+    } = require('../../lib/payments/iap/apple-app-store/subscriptions'));
+    ({ CapabilityManager } = require('@fxa/payments/capability'));
+    ({ BackupCodeManager } = require('@fxa/accounts/two-factor'));
+    ({ RecoveryPhoneService } = require('@fxa/accounts/recovery-phone'));
+    ({ PriceManager } = require('@fxa/payments/customer'));
+    ({ ProductConfigurationManager } = require('@fxa/shared/cms'));
+    /* eslint-enable @typescript-eslint/no-require-imports */
     const config = require('../../config').default.getProperties();
 
     // Override specific subscription properties (preserve defaults)
@@ -256,8 +270,6 @@ describe('#integration - remote subscriptions (enabled)', () => {
     // Register mocks in Container.
     // StripeHelper must be set BEFORE creating CapabilityService, since the
     // CapabilityService constructor reads it from the Container.
-    // With jest.mock, StripeHelper export === mockStripeHelper (the token IS the mock object).
-    const { StripeHelper } = require('../../lib/payments/stripe');
     Container.set(AppConfig, config);
     Container.set(AuthLogger, { error: () => {} });
     Container.set(StripeHelper, mockStripeHelper);
@@ -302,9 +314,10 @@ describe('#integration - remote subscriptions (enabled)', () => {
 
     serverUrl = `http://localhost:${port}`;
 
-    // Load key_server — jest.mock above ensures the stripe module returns our mock.
-    // Clear cache to get a fresh load.
-    delete require.cache[require.resolve('../../bin/key_server')];
+    // Load key_server. jest.mock at module scope ensures stripe-module
+    // requires return our mock. No resetModules here: key_server's deps
+    // share the module graph established at the top of beforeAll, so their
+    // token classes line up with the ones we registered in Container.
     const createAuthServer = require('../../bin/key_server');
     server = await createAuthServer(config);
 
@@ -319,6 +332,18 @@ describe('#integration - remote subscriptions (enabled)', () => {
     if (profileServer) {
       await profileServer.close();
     }
+    Container.remove(AppConfig);
+    Container.remove(AuthLogger);
+    Container.remove(StripeHelper);
+    Container.remove(PlaySubscriptions);
+    Container.remove(AppStoreSubscriptions);
+    Container.remove(ProfileClient);
+    Container.remove(CapabilityManager);
+    Container.remove(BackupCodeManager);
+    Container.remove(RecoveryPhoneService);
+    Container.remove(PriceManager);
+    Container.remove(ProductConfigurationManager);
+    Container.remove(CapabilityService);
   });
 
   const testVersions = [

packages/fxa-auth-server/test/remote/totp.in.spec.ts

@@ -57,6 +57,8 @@ beforeAll(async () => {
 
 afterAll(async () => {
   await server.stop();
+  Container.remove(PlaySubscriptions);
+  Container.remove(AppStoreSubscriptions);
 });
 
 const testVersions = [

packages/fxa-auth-server/test/scripts/stripe-products-and-plans-converter.in.spec.ts

@@ -234,7 +234,6 @@ describe('#integration - convert', () => {
       100
     );
     Container.reset();
-    jest.clearAllMocks();
   });
 
   it('processes new products and plans', async () => {

packages/fxa-auth-server/test/support/helpers/test-server.ts

@@ -229,8 +229,7 @@ export async function createTestServer(
   const ports = await allocatePorts();
   const publicUrl = `http://localhost:${ports.authServerPort}`;
 
-  const baseConfigPath = require.resolve('../../../config');
-  delete require.cache[baseConfigPath];
+  jest.resetModules();
   const baseConfig = require('../../../config').default.getProperties();
   const mailHelperConfig = getMailHelperConfig(baseConfig);
 
@@ -386,8 +385,7 @@ export async function getSharedTestServer(): Promise<TestServerInstance> {
   // Verify the shared server is running
   await waitForServer(publicUrl, 5, 500);
 
-  const baseConfigPath = require.resolve('../../../config');
-  delete require.cache[baseConfigPath];
+  jest.resetModules();
   const baseConfig = require('../../../config').default.getProperties();
   const mailHelperConfig = getMailHelperConfig(baseConfig);
 

packages/fxa-auth-server/test/support/helpers/test-utils.ts

@@ -14,8 +14,7 @@ export interface AuthServerError extends Error {
 }
 
 function getFlowIdKey(): string {
-  const configPath = require.resolve('../../../config');
-  delete require.cache[configPath];
+  jest.resetModules();
   const config = require('../../../config').default.getProperties();
   return config.metrics.flow_id_key;
 }