refactor: move suppressWarnings to session-aware default

codeman9 · codeman9 · commit 713a8215936e · 2025-12-16T08:53:48.000-08:00
Per PR feedback, suppressWarnings is now a session-level setting instead
of a per-tool parameter. This reduces tool schema surface area and aligns
with the session-aware architecture.

Changes:
- Add suppressWarnings to SessionDefaults type and session_set_defaults
- Remove suppressWarnings from all individual build/test tool schemas
- Update build-utils.ts to read from sessionStore
- Update tests to reflect the new session-based approach
diff --git a/src/mcp/tools/device/__tests__/build_device.test.ts b/src/mcp/tools/device/__tests__/build_device.test.ts
@@ -37,12 +37,7 @@ describe('build_device plugin', () => {
       expect(schema.safeParse({ projectPath: '/path/to/MyProject.xcodeproj' }).success).toBe(false);
 
       const schemaKeys = Object.keys(buildDevice.schema).sort();
-      expect(schemaKeys).toEqual([
-        'derivedDataPath',
-        'extraArgs',
-        'preferXcodebuild',
-        'suppressWarnings',
-      ]);
+      expect(schemaKeys).toEqual(['derivedDataPath', 'extraArgs', 'preferXcodebuild']);
     });
   });
 
diff --git a/src/mcp/tools/device/__tests__/test_device.test.ts b/src/mcp/tools/device/__tests__/test_device.test.ts
@@ -52,7 +52,6 @@ describe('test_device plugin', () => {
         'extraArgs',
         'platform',
         'preferXcodebuild',
-        'suppressWarnings',
         'testRunnerEnv',
       ]);
     });
diff --git a/src/mcp/tools/device/build_device.ts b/src/mcp/tools/device/build_device.ts
@@ -25,10 +25,6 @@ const baseSchemaObject = z.object({
   derivedDataPath: z.string().optional().describe('Path to derived data directory'),
   extraArgs: z.array(z.string()).optional().describe('Additional arguments to pass to xcodebuild'),
   preferXcodebuild: z.boolean().optional().describe('Prefer xcodebuild over faster alternatives'),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 });
 
 const baseSchema = z.preprocess(nullifyEmptyStrings, baseSchemaObject);
diff --git a/src/mcp/tools/device/test_device.ts b/src/mcp/tools/device/test_device.ts
@@ -47,10 +47,6 @@ const baseSchemaObject = z.object({
     .describe(
       'Environment variables to pass to the test runner (TEST_RUNNER_ prefix added automatically)',
     ),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 });
 
 const baseSchema = z.preprocess(nullifyEmptyStrings, baseSchemaObject);
@@ -225,7 +221,6 @@ export async function testDeviceLogic(
         configuration: params.configuration ?? 'Debug',
         derivedDataPath: params.derivedDataPath,
         extraArgs,
-        suppressWarnings: params.suppressWarnings,
       },
       {
         platform: (params.platform as XcodePlatform) || XcodePlatform.iOS,
diff --git a/src/mcp/tools/macos/__tests__/build_macos.test.ts b/src/mcp/tools/macos/__tests__/build_macos.test.ts
@@ -46,9 +46,7 @@ describe('build_macos plugin', () => {
       expect(schema.safeParse({ preferXcodebuild: 'yes' }).success).toBe(false);
 
       const schemaKeys = Object.keys(buildMacOS.schema).sort();
-      expect(schemaKeys).toEqual(
-        ['derivedDataPath', 'extraArgs', 'preferXcodebuild', 'suppressWarnings'].sort(),
-      );
+      expect(schemaKeys).toEqual(['derivedDataPath', 'extraArgs', 'preferXcodebuild'].sort());
     });
   });
 
diff --git a/src/mcp/tools/macos/__tests__/build_run_macos.test.ts b/src/mcp/tools/macos/__tests__/build_run_macos.test.ts
@@ -39,9 +39,7 @@ describe('build_run_macos', () => {
       expect(schema.safeParse({ preferXcodebuild: 'yes' }).success).toBe(false);
 
       const schemaKeys = Object.keys(tool.schema).sort();
-      expect(schemaKeys).toEqual(
-        ['derivedDataPath', 'extraArgs', 'preferXcodebuild', 'suppressWarnings'].sort(),
-      );
+      expect(schemaKeys).toEqual(['derivedDataPath', 'extraArgs', 'preferXcodebuild'].sort());
     });
   });
 
diff --git a/src/mcp/tools/macos/__tests__/test_macos.test.ts b/src/mcp/tools/macos/__tests__/test_macos.test.ts
@@ -47,13 +47,7 @@ describe('test_macos plugin (unified)', () => {
 
       const schemaKeys = Object.keys(testMacos.schema).sort();
       expect(schemaKeys).toEqual(
-        [
-          'derivedDataPath',
-          'extraArgs',
-          'preferXcodebuild',
-          'suppressWarnings',
-          'testRunnerEnv',
-        ].sort(),
+        ['derivedDataPath', 'extraArgs', 'preferXcodebuild', 'testRunnerEnv'].sort(),
       );
     });
   });
diff --git a/src/mcp/tools/macos/build_macos.ts b/src/mcp/tools/macos/build_macos.ts
@@ -46,10 +46,6 @@ const baseSchemaObject = z.object({
     .boolean()
     .optional()
     .describe('If true, prefers xcodebuild over the experimental incremental build system'),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 });
 
 const baseSchema = z.preprocess(nullifyEmptyStrings, baseSchemaObject);
diff --git a/src/mcp/tools/macos/build_run_macos.ts b/src/mcp/tools/macos/build_run_macos.ts
@@ -37,10 +37,6 @@ const baseSchemaObject = z.object({
     .boolean()
     .optional()
     .describe('If true, prefers xcodebuild over the experimental incremental build system'),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 });
 
 const baseSchema = z.preprocess(nullifyEmptyStrings, baseSchemaObject);
diff --git a/src/mcp/tools/macos/test_macos.ts b/src/mcp/tools/macos/test_macos.ts
@@ -48,10 +48,6 @@ const baseSchemaObject = z.object({
     .describe(
       'Environment variables to pass to the test runner (TEST_RUNNER_ prefix added automatically)',
     ),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 });
 
 const baseSchema = z.preprocess(nullifyEmptyStrings, baseSchemaObject);
@@ -268,7 +264,6 @@ export async function testMacosLogic(
         configuration: params.configuration ?? 'Debug',
         derivedDataPath: params.derivedDataPath,
         extraArgs,
-        suppressWarnings: params.suppressWarnings,
       },
       {
         platform: XcodePlatform.macOS,
diff --git a/src/mcp/tools/session-management/session_set_defaults.ts b/src/mcp/tools/session-management/session_set_defaults.ts
@@ -14,6 +14,10 @@ const baseSchema = z.object({
   deviceId: z.string().optional(),
   useLatestOS: z.boolean().optional(),
   arch: z.enum(['arm64', 'x86_64']).optional(),
+  suppressWarnings: z
+    .boolean()
+    .optional()
+    .describe('When true, warning messages are filtered from build output to conserve context'),
 });
 
 const schemaObj = baseSchema
diff --git a/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts b/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts
@@ -45,9 +45,7 @@ describe('build_run_sim tool', () => {
       expect(schema.safeParse({ preferXcodebuild: 'yes' }).success).toBe(false);
 
       const schemaKeys = Object.keys(buildRunSim.schema).sort();
-      expect(schemaKeys).toEqual(
-        ['derivedDataPath', 'extraArgs', 'preferXcodebuild', 'suppressWarnings'].sort(),
-      );
+      expect(schemaKeys).toEqual(['derivedDataPath', 'extraArgs', 'preferXcodebuild'].sort());
       expect(schemaKeys).not.toContain('scheme');
       expect(schemaKeys).not.toContain('simulatorName');
       expect(schemaKeys).not.toContain('projectPath');
diff --git a/src/mcp/tools/simulator/__tests__/test_sim.test.ts b/src/mcp/tools/simulator/__tests__/test_sim.test.ts
@@ -46,13 +46,7 @@ describe('test_sim tool', () => {
 
       const schemaKeys = Object.keys(testSim.schema).sort();
       expect(schemaKeys).toEqual(
-        [
-          'derivedDataPath',
-          'extraArgs',
-          'preferXcodebuild',
-          'suppressWarnings',
-          'testRunnerEnv',
-        ].sort(),
+        ['derivedDataPath', 'extraArgs', 'preferXcodebuild', 'testRunnerEnv'].sort(),
       );
     });
   });
diff --git a/src/mcp/tools/simulator/build_run_sim.ts b/src/mcp/tools/simulator/build_run_sim.ts
@@ -51,10 +51,6 @@ const baseOptions = {
     .describe(
       'If true, prefers xcodebuild over the experimental incremental build system, useful for when incremental build system fails.',
     ),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 };
 
 const baseSchemaObject = z.object({
diff --git a/src/mcp/tools/simulator/build_sim.ts b/src/mcp/tools/simulator/build_sim.ts
@@ -49,10 +49,6 @@ const baseOptions = {
     .describe(
       'If true, prefers xcodebuild over the experimental incremental build system, useful for when incremental build system fails.',
     ),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 };
 
 const baseSchemaObject = z.object({
diff --git a/src/mcp/tools/simulator/test_sim.ts b/src/mcp/tools/simulator/test_sim.ts
@@ -64,10 +64,6 @@ const baseSchemaObject = z.object({
     .describe(
       'Environment variables to pass to the test runner (TEST_RUNNER_ prefix added automatically)',
     ),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 });
 
 // Apply preprocessor to handle empty strings
@@ -117,7 +113,6 @@ export async function test_simLogic(
       preferXcodebuild: params.preferXcodebuild ?? false,
       platform: XcodePlatform.iOSSimulator,
       testRunnerEnv: params.testRunnerEnv,
-      suppressWarnings: params.suppressWarnings,
     },
     executor,
   );
diff --git a/src/mcp/tools/utilities/__tests__/clean.test.ts b/src/mcp/tools/utilities/__tests__/clean.test.ts
@@ -28,7 +28,7 @@ describe('clean (unified) tool', () => {
 
     const schemaKeys = Object.keys(tool.schema).sort();
     expect(schemaKeys).toEqual(
-      ['derivedDataPath', 'extraArgs', 'platform', 'preferXcodebuild', 'suppressWarnings'].sort(),
+      ['derivedDataPath', 'extraArgs', 'platform', 'preferXcodebuild'].sort(),
     );
   });
 
diff --git a/src/mcp/tools/utilities/clean.ts b/src/mcp/tools/utilities/clean.ts
@@ -51,10 +51,6 @@ const baseOptions = {
     .describe(
       'Optional: Platform to clean for (defaults to iOS). Choose from macOS, iOS, iOS Simulator, watchOS, watchOS Simulator, tvOS, tvOS Simulator, visionOS, visionOS Simulator',
     ),
-  suppressWarnings: z
-    .boolean()
-    .optional()
-    .describe('If true, suppresses warning messages from build output to reduce context usage'),
 };
 
 const baseSchemaObject = z.object({
@@ -127,7 +123,6 @@ export async function cleanLogic(
     configuration: params.configuration ?? 'Debug',
     derivedDataPath: params.derivedDataPath,
     extraArgs: params.extraArgs,
-    suppressWarnings: params.suppressWarnings,
   };
 
   // For clean operations, simulator platforms should be mapped to their device equivalents
diff --git a/src/types/common.ts b/src/types/common.ts
@@ -94,7 +94,6 @@ export interface SharedBuildParams {
   configuration: string;
   derivedDataPath?: string;
   extraArgs?: string[];
-  suppressWarnings?: boolean;
 }
 
 /**
diff --git a/src/utils/__tests__/build-utils-suppress-warnings.test.ts b/src/utils/__tests__/build-utils-suppress-warnings.test.ts
@@ -1,9 +1,16 @@
-import { describe, it, expect, vi } from 'vitest';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { executeXcodeBuildCommand } from '../build-utils.ts';
 import { XcodePlatform } from '../../types/common.ts';
+import { sessionStore } from '../session-store.ts';
 
 describe('executeXcodeBuildCommand - suppressWarnings', () => {
+  beforeEach(() => {
+    sessionStore.clear();
+  });
+
   it('should include warnings when suppressWarnings is false', async () => {
+    sessionStore.setDefaults({ suppressWarnings: false });
+
     const mockExecutor = vi.fn().mockResolvedValue({
       success: true,
       output: 'warning: Some warning\nerror: Some error',
@@ -16,7 +23,6 @@ describe('executeXcodeBuildCommand - suppressWarnings', () => {
         projectPath: '/test/project.xcodeproj',
         scheme: 'TestScheme',
         configuration: 'Debug',
-        suppressWarnings: false,
       },
       {
         platform: XcodePlatform.macOS,
@@ -36,6 +42,8 @@ describe('executeXcodeBuildCommand - suppressWarnings', () => {
   });
 
   it('should suppress warnings when suppressWarnings is true', async () => {
+    sessionStore.setDefaults({ suppressWarnings: true });
+
     const mockExecutor = vi.fn().mockResolvedValue({
       success: true,
       output: 'warning: Some warning\nerror: Some error',
@@ -48,7 +56,6 @@ describe('executeXcodeBuildCommand - suppressWarnings', () => {
         projectPath: '/test/project.xcodeproj',
         scheme: 'TestScheme',
         configuration: 'Debug',
-        suppressWarnings: true,
       },
       {
         platform: XcodePlatform.macOS,
diff --git a/src/utils/build-utils.ts b/src/utils/build-utils.ts
@@ -30,6 +30,7 @@ import {
   doesMakefileExist,
   doesMakeLogFileExist,
 } from './xcodemake.ts';
+import { sessionStore } from './session-store.ts';
 import path from 'path';
 
 /**
@@ -231,8 +232,9 @@ export async function executeXcodeBuildCommand(
 
     // Grep warnings and errors from stdout (build output)
     const warningOrErrorLines = grepWarningsAndErrors(result.output);
+    const suppressWarnings = sessionStore.get('suppressWarnings');
     warningOrErrorLines.forEach(({ type, content }) => {
-      if (type === 'warning' && params.suppressWarnings) {
+      if (type === 'warning' && suppressWarnings) {
         return;
       }
       buildMessages.push({
diff --git a/src/utils/session-store.ts b/src/utils/session-store.ts
@@ -10,6 +10,7 @@ export type SessionDefaults = {
   deviceId?: string;
   useLatestOS?: boolean;
   arch?: 'arm64' | 'x86_64';
+  suppressWarnings?: boolean;
 };
 
 class SessionStore {
diff --git a/src/utils/test-common.ts b/src/utils/test-common.ts
@@ -159,7 +159,6 @@ export async function handleTestLogic(
     preferXcodebuild?: boolean;
     platform: XcodePlatform;
     testRunnerEnv?: Record<string, string>;
-    suppressWarnings?: boolean;
   },
   executor?: CommandExecutor,
 ): Promise<ToolResponse> {
