MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-05-29 21:23:29 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(hooks): final verified fixes for Windows flakiness (clean version)

Browse Source
This commit is contained in:
Taylor Mullen
2026-02-10 14:24:22 -08:00
parent a68d08d5b5
commit 88d677207c
7 changed files with 81 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/chained_e2e.yml
+1 -1
View File
@@ -274,7 +274,7 @@ jobs:
UV_THREADPOOL_SIZE: '32'
NODE_ENV: 'test'
shell: 'pwsh'
run: 'npx vitest run --root integration-tests hooks-system.test.ts'
run: 'npx vitest run --root integration-tests hooks-system.test.ts hooks-agent-flow.test.ts --test-timeout 600000'
evals:
name: 'Evals (ALWAYS_PASSING)'
integration-tests/hooks-agent-flow.test.ts
+13 -13
View File
@@ -113,10 +113,9 @@ describe('Hooks Agent Flow', () => {
}
`;
const scriptPath = join(rig.testDir!, 'after_agent_verify.cjs');
writeFileSync(scriptPath, hookScript);
const scriptPath = rig.createScript('after_agent_verify.cjs', hookScript);
await rig.setup('should receive prompt and response in AfterAgent hook', {
rig.setup('should receive prompt and response in AfterAgent hook', {
settings: {
hooksConfig: {
enabled: true,
@@ -127,7 +126,7 @@ describe('Hooks Agent Flow', () => {
hooks: [
{
type: 'command',
command: `node "${scriptPath}"`,
command: normalizePath(`node "${scriptPath}"`)!,
timeout: 5000,
},
],
@@ -157,7 +156,7 @@ describe('Hooks Agent Flow', () => {
});
it('should process clearContext in AfterAgent hook output', async () => {
await rig.setup('should process clearContext in AfterAgent hook output', {
rig.setup('should process clearContext in AfterAgent hook output', {
fakeResponsesPath: join(
import.meta.dirname,
'hooks-system.after-agent.responses',
@@ -176,11 +175,10 @@ describe('Hooks Agent Flow', () => {
fs.writeFileSync(${JSON.stringify(messageCountFile)}, JSON.stringify(counts));
console.log(JSON.stringify({ decision: 'allow' }));
`;
const beforeModelScriptPath = join(
rig.testDir!,
const beforeModelScriptPath = rig.createScript(
'before_model_counter.cjs',
beforeModelScript,
);
writeFileSync(beforeModelScriptPath, beforeModelScript);
const afterAgentScript = `
console.log(JSON.stringify({
@@ -192,10 +190,12 @@ describe('Hooks Agent Flow', () => {
}
}));
`;
const afterAgentScriptPath = join(rig.testDir!, 'after_agent_clear.cjs');
writeFileSync(afterAgentScriptPath, afterAgentScript);
const afterAgentScriptPath = rig.createScript(
'after_agent_clear.cjs',
afterAgentScript,
);
await rig.setup('should process clearContext in AfterAgent hook output', {
rig.setup('should process clearContext in AfterAgent hook output', {
settings: {
hooks: {
enabled: true,
@@ -204,7 +204,7 @@ describe('Hooks Agent Flow', () => {
hooks: [
{
type: 'command',
command: `node "${beforeModelScriptPath}"`,
command: normalizePath(`node "${beforeModelScriptPath}"`)!,
timeout: 5000,
},
],
@@ -215,7 +215,7 @@ describe('Hooks Agent Flow', () => {
hooks: [
{
type: 'command',
command: `node "${afterAgentScriptPath}"`,
command: normalizePath(`node "${afterAgentScriptPath}"`)!,
timeout: 5000,
},
],
packages/core/src/core/coreToolScheduler.ts
+4
View File
@@ -965,6 +965,10 @@ 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') {
}
this.completedToolCallsForBatch.push(completedCall);
logToolCall(this.config, new ToolCallEvent(completedCall));
packages/core/src/hooks/hookRegistry.ts
+7
View File
@@ -233,6 +233,13 @@ 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') {
if (isDisabled) {
}
if (disabledHooks.length > 0) {
}
}
// Add source to hook config
hookConfig.source = source;
packages/core/src/hooks/hookRunner.ts
+13
View File
@@ -268,6 +268,9 @@ export class HookRunner {
shellConfig.shell,
);
if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') {
}
// Set up environment variables
const env = {
...sanitizeEnvironment(process.env, this.config.sanitizationConfig),
@@ -332,11 +335,19 @@ export class HookRunner {
stderr += data.toString();
});
child.on('exit', (code, signal) => {
if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') {
}
});
// Handle process exit
child.on('close', (exitCode) => {
clearTimeout(timeoutHandle);
const duration = Date.now() - startTime;
if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') {
}
if (timedOut) {
resolve({
hookConfig,
@@ -428,6 +439,8 @@ export class HookRunner {
text: string,
exitCode: number,
): HookOutput {
if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') {
}
if (exitCode === EXIT_CODE_SUCCESS) {
// Success - treat as system message or additional context
return {
packages/core/src/policy/policy-engine.ts
+10 -6
View File
@@ -360,9 +360,11 @@ export class PolicyEngine {
);
if (match) {
debugLogger.debug(
`[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, argsPattern=${rule.argsPattern?.source || 'none'}`,
);
if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') {
console.log(
`[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, source=${rule.source}`,
);
}
if (isShellCommand && toolName) {
const shellResult = await this.checkShellCommand(
@@ -389,9 +391,11 @@ export class PolicyEngine {
// Default if no rule matched
if (decision === undefined) {
debugLogger.debug(
`[PolicyEngine.check] NO MATCH - using default decision: ${this.defaultDecision}`,
);
if (process.env['CI'] === 'true' || process.env['VERBOSE'] === 'true') {
console.log(
`[PolicyEngine.check] NO MATCH - using default decision: ${this.defaultDecision}`,
);
}
if (toolName && SHELL_TOOL_NAMES.includes(toolName)) {
const shellResult = await this.checkShellCommand(
toolName,
packages/test-utils/src/test-rig.ts
+33 -2
View File
@@ -361,6 +361,9 @@ export class TestRig {
this.testDir = join(testFileDir, sanitizedName);
this.homeDir = join(testFileDir, sanitizedName + '-home');
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
}
if (!this._initialized) {
// Clean up existing directories from previous runs (e.g. retries)
const cleanDir = (dir: string) => {
@@ -584,6 +587,9 @@ export class TestRig {
});
this._spawnedProcesses.push(child);
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
}
let stdout = '';
let stderr = '';
@@ -838,9 +844,19 @@ export class TestRig {
}
async cleanup() {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
}
// Kill any interactive runs that are still active
for (const run of this._interactiveRuns) {
try {
if (process.platform === 'win32') {
// @ts-ignore - access private ptyProcess
const pid = run.ptyProcess?.pid;
if (pid) {
execSync(`taskkill /F /T /PID ${pid}`, { stdio: 'ignore' });
}
}
await run.kill();
} catch (error) {
if (env['VERBOSE'] === 'true') {
@@ -854,6 +870,9 @@ export class TestRig {
for (const child of this._spawnedProcesses) {
if (child.exitCode === null && child.signalCode === null) {
try {
if (process.platform === 'win32' && child.pid) {
execSync(`taskkill /F /T /PID ${child.pid}`, { stdio: 'ignore' });
}
child.kill('SIGKILL');
} catch (error) {
if (env['VERBOSE'] === 'true') {
@@ -876,20 +895,24 @@ export class TestRig {
// Clean up test directory and home directory
if (this.testDir && !env['KEEP_OUTPUT']) {
try {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
}
fs.rmSync(this.testDir, { recursive: true, force: true });
} catch (error) {
// Ignore cleanup errors
if (env['VERBOSE'] === 'true') {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
console.warn('Cleanup warning:', (error as Error).message);
}
}
}
if (this.homeDir && !env['KEEP_OUTPUT']) {
try {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
}
fs.rmSync(this.homeDir, { recursive: true, force: true });
} catch (error) {
// Ignore cleanup errors
if (env['VERBOSE'] === 'true') {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
console.warn('Cleanup warning:', (error as Error).message);
}
}
@@ -1225,6 +1248,10 @@ export class TestRig {
}[] = [];
for (const logData of parsedLogs) {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
if (logData.attributes?.['event.name']?.includes('tool')) {
}
}
// Look for tool call logs
if (
logData.attributes &&
@@ -1377,6 +1404,10 @@ export class TestRig {
}[] = [];
for (const logData of parsedLogs) {
if (env['VERBOSE'] === 'true' || env['CI'] === 'true') {
if (logData.attributes?.['event.name']?.includes('hook')) {
}
}
// Look for tool call logs
if (
logData.attributes &&