Skip to content

Commit fe4af01

Browse files
committed
fix: apply CodeRabbit recommendations for hooks support
- Honor GROQ_NO_LOCAL_HOOKS environment variable in getHooksConfig() - Anchor local config at project root instead of current working directory - Fix getHooksConfig() call to use getActiveHooks() in hooks command - Fix getMergedConfig() to properly merge hooks using special merge rules
1 parent 8f7f086 commit fe4af01

File tree

2 files changed

+36
-4
lines changed

2 files changed

+36
-4
lines changed

src/commands/definitions/hooks.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { ConfigManager } from '../../utils/local-settings.js';
55
function listHooks(showSource: boolean = true): string {
66
const globalHooks = hooksManager.getGlobalHooks();
77
const localHooks = hooksManager.getLocalHooks();
8-
const mergedHooks = hooksManager.getHooksConfig();
8+
const mergedHooks = hooksManager.getActiveHooks();
99

1010
if (!mergedHooks || Object.keys(mergedHooks).length === 0) {
1111
return 'No hooks configured\nRun "/hooks init" for global hooks or "/hooks init --local" for project-specific hooks';

src/utils/local-settings.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,29 @@ export class ConfigManager {
1818
constructor() {
1919
const homeDir = os.homedir();
2020
this.globalConfigPath = path.join(homeDir, CONFIG_DIR, CONFIG_FILE);
21-
this.localConfigPath = path.join(process.cwd(), CONFIG_DIR, CONFIG_FILE);
21+
const projectRoot = this.getProjectRoot();
22+
this.localConfigPath = path.join(projectRoot, CONFIG_DIR, CONFIG_FILE);
23+
}
24+
25+
private getProjectRoot(): string {
26+
let dir = process.cwd();
27+
const root = path.parse(dir).root;
28+
29+
while (true) {
30+
if (
31+
fs.existsSync(path.join(dir, '.git')) ||
32+
fs.existsSync(path.join(dir, 'package.json')) ||
33+
fs.existsSync(path.join(dir, CONFIG_DIR))
34+
) {
35+
return dir;
36+
}
37+
if (dir === root) {
38+
break;
39+
}
40+
dir = path.dirname(dir);
41+
}
42+
43+
return process.cwd();
2244
}
2345

2446
private ensureConfigDir(configPath: string): void {
@@ -75,7 +97,15 @@ export class ConfigManager {
7597
const localConfig = this.readConfig(this.localConfigPath) || {};
7698

7799
// Local project settings take precedence over global user settings
78-
return { ...globalConfig, ...localConfig };
100+
const merged: Config = { ...globalConfig, ...localConfig };
101+
// Ensure hooks follow special merge rules (arrays concatenated, objects overridden)
102+
if (globalConfig.hooks || localConfig.hooks) {
103+
merged.hooks = this.mergeHooks(
104+
globalConfig.hooks || {},
105+
localConfig.hooks || {}
106+
);
107+
}
108+
return merged;
79109
}
80110

81111
public getApiKey(): string | null {
@@ -146,8 +176,10 @@ export class ConfigManager {
146176
const globalHooks = globalConfig?.hooks || {};
147177
const localHooks = localConfig?.hooks || {};
148178

179+
// Honor GROQ_NO_LOCAL_HOOKS: when set to "true", drop all local hooks
180+
const disableLocal = process.env.GROQ_NO_LOCAL_HOOKS === 'true';
149181
// Merge hooks - arrays are concatenated (local after global); objects are overridden by local
150-
const mergedHooks = this.mergeHooks(globalHooks, localHooks);
182+
const mergedHooks = this.mergeHooks(globalHooks, disableLocal ? {} : localHooks);
151183

152184
return {
153185
global: globalHooks,

0 commit comments

Comments
 (0)