test: convert mock-module-lint baseline call sites (closes #1238)

packages/integration/src/paste-focus-isolation.test.tsx

@@ -6,37 +6,27 @@
  */
 import { describe, it, expect, mock, afterEach, beforeEach } from 'bun:test';
 import { fireEvent, cleanup, act, within } from '@testing-library/react';
-
-// Mock dependencies required by MessagePanel
-const mockSendWorkerMessage = mock(() =>
-  Promise.resolve({
-    message: {
-      id: 'msg-1',
-      sessionId: 'session-1',
-      fromWorkerId: 'user',
-      fromWorkerName: 'User',
-      toWorkerId: 'worker-1',
-      toWorkerName: 'Worker 1',
-      content: '',
-      timestamp: new Date().toISOString(),
-    },
-  })
-);
-mock.module('@agent-console/client/src/lib/api', () => ({
-  sendWorkerMessage: mockSendWorkerMessage,
-}));
-mock.module('@agent-console/client/src/lib/worker-websocket', () => ({
-  sendInput: mock(() => true),
-}));
-
 import { MessagePanel } from '@agent-console/client/src/components/sessions/MessagePanel';
 import { renderWithRouter } from '@agent-console/client/src/test/renderWithRouter';
 import { _getDraftsMap } from '@agent-console/client/src/hooks/useDraftMessage';
 
+// MessagePanel resolves its send action via an injected `onSend` prop, not
+// via a module-level import of `lib/api` / `lib/worker-websocket` -- the
+// prior `mock.module()` of those two modules was a leftover from before
+// that DI seam existed and mocked exports MessagePanel no longer reads.
+// Neither test below triggers a send, but `onSend` is a required prop.
+// Using the real DI seam (Pattern 1) instead of `mock.module()` avoids
+// process-globally poisoning sibling integration tests that import
+// `lib/api` / `lib/worker-websocket` for real in the same bun:test
+// process (e.g. system-api-boundary.test.ts) -- the live #1225-class
+// poisoner this file used to be (`.claude/rules/testing.md` Anti-Pattern #2).
+const mockOnSend = mock(() => Promise.resolve());
+
 const defaultProps = {
   sessionId: 'session-1',
   targetWorkerId: 'worker-1',
   newMessage: null,
+  onSend: mockOnSend,
 };
 
 describe('Paste Focus Isolation (#523)', () => {

packages/server/src/__tests__/api.test.ts

@@ -11,7 +11,7 @@ import type {
 } from '@agent-console/shared';
 import { setupMemfs, cleanupMemfs, createMockGitRepoFiles } from './utils/mock-fs-helper.js';
 import { mockProcess, resetProcessMock } from './utils/mock-process-helper.js';
-import { MockPty, createMockPtyFactory } from './utils/mock-pty.js';
+import { createMockPtyFactory } from './utils/mock-pty.js';
 import { mockGit, GitError } from './utils/mock-git-helper.js';
 
 // Set up test config directory BEFORE any service imports to ensure
@@ -23,29 +23,8 @@ process.env.AGENT_CONSOLE_HOME = TEST_CONFIG_DIR;
 // Infrastructure Mocks (must be before any service imports)
 // =============================================================================
 
-// Track PTY instances
-const mockPtyInstances: MockPty[] = [];
-let nextPtyPid = 10000;
-
-// Mock pty-provider module to avoid spawning real PTY processes in tests
-mock.module('../lib/pty-provider.js', () => ({
-  bunPtyProvider: {
-    spawn: () => {
-      const pty = new MockPty(nextPtyPid++);
-      mockPtyInstances.push(pty);
-      return pty;
-    },
-  },
-}));
-
 // Note: process-utils is mocked via mock-process-helper.js (imported above)
 
-// Mock open package to prevent actual file opening
-const mockOpen = mock(async () => {});
-mock.module('open', () => ({
-  default: mockOpen,
-}));
-
 // Mock session-metadata-suggester to avoid running actual agent commands
 const mockSuggestSessionMetadata = mock(async () => ({
   branch: 'suggested-branch',
@@ -114,7 +93,7 @@ import { WorkerOutputFileManager } from '../lib/worker-output-file.js';
 import { SystemCapabilitiesService } from '../services/system-capabilities-service.js';
 import { WorktreeService } from '../services/worktree-service.js';
 import type { AppBindings } from '../app-context.js';
-import { asAppContext, TEST_AUTH_USER, ensureTestAuthUser } from './test-utils.js';
+import { asAppContext, TEST_AUTH_USER, ensureTestAuthUser, mockOpen } from './test-utils.js';
 import { SingleUserMode } from '../services/user-mode.js';
 import { McpTokenRegistry } from '../mcp/mcp-auth.js';
 
@@ -201,8 +180,6 @@ describe('API Routes Integration', () => {
     process.env.AGENT_CONSOLE_HOME = TEST_CONFIG_DIR;
 
     // Reset PTY tracking
-    mockPtyInstances.length = 0;
-    nextPtyPid = 10000;
     ptyFactory.reset();
 
     // Reset process tracking

packages/server/src/routes/__tests__/system.test.ts

@@ -1,15 +1,8 @@
-import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test';
+import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
 import { Hono } from 'hono';
 import type { AppBindings } from '../../app-context.js';
 import { asAppContext } from '../../__tests__/test-utils.js';
 
-// Mock open package BEFORE importing mock-fs-helper
-// The open package internally uses fs and needs to be mocked first
-const mockOpen = mock(async () => {});
-mock.module('open', () => ({
-  default: mockOpen,
-}));
-
 // Import mock-fs-helper to set up memfs mocks
 import { setupMemfs, cleanupMemfs } from '../../__tests__/utils/mock-fs-helper.js';
 import { createMockSystemCapabilities } from '../../__tests__/utils/mock-system-capabilities-helper.js';

packages/server/src/services/__tests__/worktree-creation-service.test.ts

@@ -50,17 +50,11 @@ const mockWorktreeService = {
   executeHookCommand: mockExecuteHookCommand,
 };
 
-// --- Mock logger ---
-mock.module('../../lib/logger.js', () => ({
-  createLogger: () => ({
-    info: () => {},
-    warn: () => {},
-    error: () => {},
-    debug: () => {},
-  }),
-}));
-
-// Import after mocks
+// No logger mock needed: `lib/logger.js`'s pino instance is already disabled
+// (`enabled: !isTest`) whenever `NODE_ENV === 'test'`, which Bun sets by
+// default for `bun test` runs. The real (silent) logger is used directly,
+// avoiding a process-global `mock.module()` of a module other test files
+// import for real (`.claude/rules/testing.md` Anti-Pattern #2).
 const { createWorktreeWithSession } = await import('../worktree-creation-service.js');
 
 // --- Helpers ---

packages/server/src/services/__tests__/worktree-deletion-service.test.ts

@@ -48,15 +48,11 @@ const mockWorktreeService = {
   removeOrphanedWorktree: mockRemoveOrphanedWorktree,
 };
 
-// --- Mock logger ---
-mock.module('../../lib/logger.js', () => ({
-  createLogger: () => ({
-    info: () => {},
-    warn: () => {},
-    error: () => {},
-    debug: () => {},
-  }),
-}));
+// No logger mock needed: `lib/logger.js`'s pino instance is already disabled
+// (`enabled: !isTest`) whenever `NODE_ENV === 'test'`, which Bun sets by
+// default for `bun test` runs. The real (silent) logger is used directly,
+// avoiding a process-global `mock.module()` of a module other test files
+// import for real (`.claude/rules/testing.md` Anti-Pattern #2).
 
 // Note: The Bun shell ($) tagged template literal cannot be reliably mocked
 // via mock.module('bun', ...). The gitStatus capture path runs a real `git -C`

packages/server/src/services/inbound/__tests__/diff-worker-handler.test.ts

@@ -1,13 +1,25 @@
-import { describe, it, expect, mock } from 'bun:test';
+import { describe, it, expect, mock, spyOn, beforeEach, afterEach } from 'bun:test';
 import type { InboundSystemEvent, Session, Worker } from '@agent-console/shared';
 import { createInboundHandlers, type InboundEventHandler, type InboundHandlerDependencies, type EventTarget } from '../handlers.js';
 import { buildWorktreeSession } from '../../../__tests__/utils/build-test-data.js';
-
-// Mock triggerRefresh at the module level since it's a standalone function import
+import * as gitDiffServiceModule from '../../git-diff-service.js';
+
+// `triggerRefresh` is a standalone function import in handlers.ts (no DI
+// seam via InboundHandlerDependencies exists for it yet -- see the PR
+// description for this conversion). spyOn() on the real module keeps this
+// test file-scoped and restorable, instead of process-globally poisoning
+// every other importer of git-diff-service.js the way mock.module() would
+// (`.claude/rules/testing.md` Anti-Pattern #2).
 const mockTriggerRefresh = mock(() => {});
-mock.module('../../git-diff-service.js', () => ({
-  triggerRefresh: mockTriggerRefresh,
-}));
+let triggerRefreshSpy: ReturnType<typeof spyOn>;
+
+beforeEach(() => {
+  triggerRefreshSpy = spyOn(gitDiffServiceModule, 'triggerRefresh').mockImplementation(mockTriggerRefresh);
+});
+
+afterEach(() => {
+  triggerRefreshSpy.mockRestore();
+});
 
 function createEvent(type: 'ci:completed' | 'pr:merged' = 'ci:completed'): InboundSystemEvent {
   return {

scripts/__tests__/check-mock-module-poisoners.test.mjs

@@ -319,8 +319,11 @@ describe('findDefaultFiles — scan glob', () => {
 });
 
 describe('KNOWN_VIOLATIONS / SANCTIONED_LOCATIONS — baseline integrity', () => {
-  it('every KNOWN_VIOLATIONS entry has a file, specifier, and one-line reason', () => {
-    expect(KNOWN_VIOLATIONS.length).toBeGreaterThan(0);
+  it('every KNOWN_VIOLATIONS entry (if any) has a file, specifier, and one-line reason', () => {
+    // The original 8-entry baseline was fully converted in Issue #1238;
+    // the array is expected to be empty until a new justified exception is
+    // added (see the file-exclusive exception in testing.md Anti-Pattern #2).
+    expect(KNOWN_VIOLATIONS.length).toBeGreaterThanOrEqual(0);
     for (const entry of KNOWN_VIOLATIONS) {
       expect(typeof entry.file).toBe('string');
       expect(typeof entry.specifier).toBe('string');

scripts/check-mock-module-poisoners.mjs

@@ -340,56 +340,15 @@ export async function runCheck({ cwd = process.cwd(), files, allowlist = KNOWN_V
 
 // ---------------------------------------------------------------------------
 // KNOWN_VIOLATIONS — see the file header for the allowlist strategy. Keys
-// are `file + specifier` pairs. Enumerated by running this detector
-// against `main` at the time this script landed (Issue #1226); the
-// follow-up cleanup Issue (see .claude/rules/testing.md Anti-Pattern #2)
-// converts these to DI seam / spyOn / central registry migration per the
-// #977 playbook. Remove an entry here in the same PR that converts it —
-// a stale entry left behind fails CI by design.
+// are `file + specifier` pairs. The original 8-entry baseline (enumerated
+// against `main` at the time this script landed, Issue #1226) was fully
+// converted to DI seam / spyOn / central-registry migration per the #977
+// playbook in Issue #1238. Empty now; a new justified entry is added the
+// same way -- via the permitted file-exclusive exception in
+// `.claude/rules/testing.md` Anti-Pattern #2 -- and removed in the same PR
+// that converts it. A stale entry left behind fails CI by design.
 // ---------------------------------------------------------------------------
-export const KNOWN_VIOLATIONS = [
-  {
-    file: 'packages/integration/src/paste-focus-isolation.test.tsx',
-    specifier: '@agent-console/client/src/lib/api',
-    reason:
-      'Live #1225-class poisoner: mocked here but imported for real by sibling integration tests in the same bun:test process; priority conversion target.',
-  },
-  {
-    file: 'packages/integration/src/paste-focus-isolation.test.tsx',
-    specifier: '@agent-console/client/src/lib/worker-websocket',
-    reason: 'Same file/priority as the api.ts entry above.',
-  },
-  {
-    file: 'packages/server/src/__tests__/api.test.ts',
-    specifier: '../lib/pty-provider.js',
-    reason: 'Ad-hoc baseline; large integration-style test file, not yet converted to DI.',
-  },
-  {
-    file: 'packages/server/src/__tests__/api.test.ts',
-    specifier: 'open',
-    reason: 'Ad-hoc baseline, duplicate of the system.test.ts open mock below.',
-  },
-  {
-    file: 'packages/server/src/routes/__tests__/system.test.ts',
-    specifier: 'open',
-    reason: 'Ad-hoc baseline, duplicate of the api.test.ts open mock above.',
-  },
-  {
-    file: 'packages/server/src/services/__tests__/worktree-creation-service.test.ts',
-    specifier: '../../lib/logger.js',
-    reason: 'Ad-hoc baseline; logger mock, candidate for DI conversion.',
-  },
-  {
-    file: 'packages/server/src/services/__tests__/worktree-deletion-service.test.ts',
-    specifier: '../../lib/logger.js',
-    reason: 'Ad-hoc baseline; logger mock, candidate for DI conversion.',
-  },
-  {
-    file: 'packages/server/src/services/inbound/__tests__/diff-worker-handler.test.ts',
-    specifier: '../../git-diff-service.js',
-    reason: 'Ad-hoc baseline; standalone function mock, candidate for DI via InboundHandlerDependencies.',
-  },
-];
+export const KNOWN_VIOLATIONS = [];
 
 // ---------------------------------------------------------------------------
 // CLI wrapper