From efab27e67be8790d84229351fdae9f2095ea0f8f Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Tue, 10 Feb 2026 15:29:22 -0800 Subject: [PATCH] repro: final clean verified fixes for Windows flakiness --- integration-tests/hooks-agent-flow.test.ts | 12 +- integration-tests/hooks-system.test.ts | 134 ++++++-------------- packages/core/src/core/coreToolScheduler.ts | 4 - packages/core/src/hooks/hookRegistry.ts | 10 -- packages/core/src/hooks/hookRunner.ts | 18 --- packages/test-utils/src/test-rig.ts | 24 ---- 6 files changed, 43 insertions(+), 159 deletions(-) diff --git a/integration-tests/hooks-agent-flow.test.ts b/integration-tests/hooks-agent-flow.test.ts index 5529c49cd2..8cc819a5a9 100644 --- a/integration-tests/hooks-agent-flow.test.ts +++ b/integration-tests/hooks-agent-flow.test.ts @@ -52,9 +52,7 @@ describe('Hooks Agent Flow', () => { await rig.setup('should inject additional context via BeforeAgent hook', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeAgent: [ { @@ -116,9 +114,7 @@ describe('Hooks Agent Flow', () => { rig.setup('should receive prompt and response in AfterAgent hook', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { AfterAgent: [ { @@ -273,9 +269,7 @@ describe('Hooks Agent Flow', () => { 'should fire BeforeAgent and AfterAgent exactly once per turn despite tool calls', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeAgent: [ { diff --git a/integration-tests/hooks-system.test.ts b/integration-tests/hooks-system.test.ts index 3911840d09..88c7f954b2 100644 --- a/integration-tests/hooks-system.test.ts +++ b/integration-tests/hooks-system.test.ts @@ -43,10 +43,8 @@ describe('Hooks System Integration', () => { 'should block tool execution when hook returns block decision', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { BeforeTool: [ { matcher: 'write_file', @@ -87,7 +85,7 @@ describe('Hooks System Integration', () => { expect(hookTelemetryFound).toBeTruthy(); }); - it.only('should block tool execution and use stderr as reason when hook exits with code 2', async () => { + it('should block tool execution and use stderr as reason when hook exits with code 2', async () => { rig.setup( 'should block tool execution and use stderr as reason when hook exits with code 2', { @@ -115,10 +113,8 @@ process.exit(0); 'should block tool execution and use stderr as reason when hook exits with code 2', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { BeforeTool: [ { matcher: 'write_file', @@ -188,10 +184,8 @@ process.exit(0); 'should allow tool execution when hook returns allow decision', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { BeforeTool: [ { matcher: 'write_file', @@ -245,9 +239,7 @@ process.exit(0); const command = `node "${scriptPath}"`; rig.setup('should add additional context from AfterTool hooks', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { AfterTool: [ { @@ -322,9 +314,7 @@ console.log(JSON.stringify({ rig.setup('should modify LLM requests with BeforeModel hooks', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeModel: [ { @@ -386,10 +376,8 @@ console.log(JSON.stringify({ 'should block model execution when BeforeModel hook returns deny decision', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { BeforeModel: [ { sequential: true, @@ -434,10 +422,8 @@ console.log(JSON.stringify({ 'should block model execution when BeforeModel hook returns block decision', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { BeforeModel: [ { sequential: true, @@ -501,10 +487,8 @@ console.log(JSON.stringify({ rig.setup('should modify LLM responses with AfterModel hooks', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { AfterModel: [ { hooks: [ @@ -562,9 +546,7 @@ console.log(JSON.stringify({ rig.setup('should modify tool selection with BeforeToolSelection hooks', { settings: { debugMode: true, - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeToolSelection: [ { @@ -629,9 +611,7 @@ console.log(JSON.stringify({ rig.setup('should augment prompts with BeforeAgent hooks', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeAgent: [ { @@ -683,9 +663,7 @@ console.log(JSON.stringify({ approval: 'ASK', // Disable YOLO mode to show permission prompts confirmationRequired: ['run_shell_command'], }, - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { Notification: [ { @@ -787,9 +765,7 @@ console.log(JSON.stringify({ rig.setup('should execute hooks sequentially when configured', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeAgent: [ { @@ -871,9 +847,7 @@ try { rig.setup('should provide correct input format to hooks', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeTool: [ { @@ -928,10 +902,8 @@ try { 'should treat mixed stdout (text + JSON) as system message and allow execution when exit code is 0', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { BeforeTool: [ { matcher: 'write_file', @@ -995,9 +967,7 @@ try { rig.setup('should handle hooks for all major event types', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeAgent: [ { @@ -1118,9 +1088,7 @@ try { rig.setup('should handle hook failures gracefully', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeTool: [ { @@ -1179,9 +1147,7 @@ try { rig.setup('should generate telemetry events for hook executions', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeTool: [ { @@ -1229,9 +1195,7 @@ try { rig.setup('should fire SessionStart hook on app startup', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { SessionStart: [ { @@ -1307,9 +1271,7 @@ console.log(JSON.stringify({ rig.setup('should fire SessionStart hook and inject context', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { SessionStart: [ { @@ -1392,10 +1354,8 @@ console.log(JSON.stringify({ 'should fire SessionStart hook and display systemMessage in interactive mode', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { SessionStart: [ { matcher: 'startup', @@ -1476,10 +1436,8 @@ console.log(JSON.stringify({ 'should fire SessionEnd and SessionStart hooks on /clear command', { settings: { - hooksConfig: { - enabled: true, - }, - hooks: { + enableHooks: true, + hooks: { SessionEnd: [ { matcher: '*', @@ -1659,9 +1617,7 @@ console.log(JSON.stringify({ rig.setup('should fire PreCompress hook on automatic compression', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { PreCompress: [ { @@ -1680,7 +1636,7 @@ console.log(JSON.stringify({ // Configure automatic compression with a very low threshold // This will trigger auto-compression after the first response contextCompression: { - enabled: true, + // enabled: true, targetTokenCount: 10, // Very low threshold to trigger compression }, }, @@ -1737,9 +1693,7 @@ console.log(JSON.stringify({ rig.setup('should fire SessionEnd hook on graceful exit', { settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { SessionEnd: [ { @@ -1840,10 +1794,8 @@ console.log(JSON.stringify({decision: "block", systemMessage: "Disabled hook sho rig.setup('should not execute hooks disabled in settings file', { settings: { - hooksConfig: { - enabled: true, - disabled: [normalizePath(`node "${disabledPath}"`)], // Disable the second hook - }, + enableHooks: true, + disabledHooks: [normalizePath(`node "${disabledPath}"`)], // Disable the second hook hooks: { BeforeTool: [ { @@ -1915,10 +1867,8 @@ console.log(JSON.stringify({decision: "block", systemMessage: "Disabled hook sho rig.setup('should respect disabled hooks across multiple operations', { settings: { - hooksConfig: { - enabled: true, - disabled: [normalizedDisabledCmd!], // Disable the second hook, - }, + enableHooks: true, + disabledHooks: [normalizedDisabledCmd!], // Disable the second hook, hooks: { BeforeTool: [ { @@ -2019,9 +1969,7 @@ console.log(JSON.stringify({decision: "block", systemMessage: "Disabled hook sho 'hooks-system.input-modification.responses', ), settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeTool: [ { @@ -2107,9 +2055,7 @@ console.log(JSON.stringify({decision: "block", systemMessage: "Disabled hook sho 'hooks-system.before-tool-stop.responses', ), settings: { - hooksConfig: { - enabled: true, - }, + enableHooks: true, hooks: { BeforeTool: [ { diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 3ea97c8adf..ec566da0fe 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -966,10 +966,6 @@ export class CoreToolScheduler { // The active tool is finished. Move it to the completed batch. const completedCall = activeCall as CompletedToolCall; - if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') { - console.log(`[CoreToolScheduler] Completed tool call: ${completedCall.request.name}, status: ${completedCall.status}`); - } - this.completedToolCallsForBatch.push(completedCall); logToolCall(this.config, new ToolCallEvent(completedCall)); diff --git a/packages/core/src/hooks/hookRegistry.ts b/packages/core/src/hooks/hookRegistry.ts index e90c60f4b5..1e5515a6a0 100644 --- a/packages/core/src/hooks/hookRegistry.ts +++ b/packages/core/src/hooks/hookRegistry.ts @@ -233,16 +233,6 @@ please review the project settings (.gemini/settings.json) and remove them.`; } as HookRegistryEntry); const isDisabled = disabledHooks.includes(hookName); - if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') { - console.log(`[HookRegistry] Processing hook: "${hookName}"`); - if (isDisabled) { - console.log(`[HookRegistry] Hook is DISABLED (found in disabledHooks list)`); - } - if (disabledHooks.length > 0) { - console.log(`[HookRegistry] Current disabledHooks: ${JSON.stringify(disabledHooks)}`); - } - } - // Add source to hook config hookConfig.source = source; diff --git a/packages/core/src/hooks/hookRunner.ts b/packages/core/src/hooks/hookRunner.ts index d7238d98be..6619bc51df 100644 --- a/packages/core/src/hooks/hookRunner.ts +++ b/packages/core/src/hooks/hookRunner.ts @@ -268,12 +268,6 @@ export class HookRunner { shellConfig.shell, ); - if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') { - console.log(`[HookRunner] shellConfig: ${JSON.stringify(shellConfig)}`); - console.log(`[HookRunner] command: ${command}`); - console.log(`[HookRunner] cwd: ${input.cwd}`); - } - // Set up environment variables const env = { ...sanitizeEnvironment(process.env, this.config.sanitizationConfig), @@ -339,9 +333,6 @@ export class HookRunner { }); child.on('exit', (code, signal) => { - if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') { - console.log(`[HookRunner] Hook exit. code: ${code}, signal: ${signal}`); - } }); // Handle process exit @@ -349,12 +340,6 @@ export class HookRunner { clearTimeout(timeoutHandle); const duration = Date.now() - startTime; - if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') { - console.log(`[HookRunner] Hook closed. exitCode: ${exitCode}, duration: ${duration}ms`); - console.log(`[HookRunner] stdout: ${stdout}`); - console.log(`[HookRunner] stderr: ${stderr}`); - } - if (timedOut) { resolve({ hookConfig, @@ -446,9 +431,6 @@ export class HookRunner { text: string, exitCode: number, ): HookOutput { - if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') { - console.log(`[HookRunner] convertPlainTextToHookOutput: exitCode=${exitCode}, text="${text}"`); - } if (exitCode === EXIT_CODE_SUCCESS) { // Success - treat as system message or additional context return { diff --git a/packages/test-utils/src/test-rig.ts b/packages/test-utils/src/test-rig.ts index 5caeccdcda..87983ba0e3 100644 --- a/packages/test-utils/src/test-rig.ts +++ b/packages/test-utils/src/test-rig.ts @@ -361,19 +361,10 @@ export class TestRig { this.testDir = join(testFileDir, sanitizedName); this.homeDir = join(testFileDir, sanitizedName + '-home'); - if (env['VERBOSE'] === 'true' || env['CI'] === 'true') { - console.log(`[TestRig] Setting up test: ${this.testName}`); - console.log(`[TestRig] testDir: ${this.testDir}`); - console.log(`[TestRig] homeDir: ${this.homeDir}`); - } - if (!this._initialized) { // Clean up existing directories from previous runs (e.g. retries) const cleanDir = (dir: string) => { if (fs.existsSync(dir)) { - if (env['VERBOSE'] === 'true' || env['CI'] === 'true') { - console.log(`[TestRig] Cleaning up existing directory: ${dir}`); - } for (let i = 0; i < 5; i++) { try { fs.rmSync(dir, { recursive: true, force: true }); @@ -771,11 +762,6 @@ export class TestRig { }); this._spawnedProcesses.push(child); - if (env['VERBOSE'] === 'true' || env['CI'] === 'true') { - console.log(`[TestRig] Running: ${command} ${commandArgs.join(' ')}`); - console.log(`[TestRig] CWD: ${this.testDir}`); - } - let stdout = ''; let stderr = ''; @@ -1259,11 +1245,6 @@ export class TestRig { }[] = []; for (const logData of parsedLogs) { - if (env['VERBOSE'] === 'true' || env['CI'] === 'true') { - if (logData.attributes?.['event.name']?.includes('tool')) { - console.log(`[TestRig] Found tool-related log: ${JSON.stringify(logData.attributes)}`); - } - } // Look for tool call logs if ( logData.attributes && @@ -1416,11 +1397,6 @@ export class TestRig { }[] = []; for (const logData of parsedLogs) { - if (env['VERBOSE'] === 'true' || env['CI'] === 'true') { - if (logData.attributes?.['event.name']?.includes('hook')) { - console.log(`[TestRig] Found hook-related log: ${JSON.stringify(logData.attributes)}`); - } - } // Look for tool call logs if ( logData.attributes &&