-
Notifications
You must be signed in to change notification settings - Fork 0
feat: migrate config from JSON to YAML (GIT-89) #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,42 @@ | ||
| /** | ||
| * Hook configuration reader. | ||
| * | ||
| * Reads .git-mem.json from the working directory and returns | ||
| * Reads .git-mem/.git-mem.yaml from the working directory and returns | ||
| * typed hook configuration with sensible defaults. | ||
| * Never throws — returns defaults on missing file or parse errors. | ||
| */ | ||
|
|
||
| import { existsSync, readFileSync } from 'fs'; | ||
| import { join } from 'path'; | ||
| import { parse as parseYaml } from 'yaml'; | ||
| import type { IHookConfig, IHooksConfig } from '../../domain/interfaces/IHookConfig'; | ||
|
|
||
| /** Directory containing git-mem configuration */ | ||
| export const CONFIG_DIR = '.git-mem'; | ||
|
|
||
| /** Config file name */ | ||
| export const CONFIG_FILE = '.git-mem.yaml'; | ||
|
|
||
| /** | ||
| * Get the full path to the config file. | ||
| * @param cwd - Working directory (defaults to process.cwd()) | ||
| * @returns Absolute path to .git-mem/.git-mem.yaml | ||
| */ | ||
| export function getConfigPath(cwd?: string): string { | ||
| const dir = cwd ?? process.cwd(); | ||
| return join(dir, CONFIG_DIR, CONFIG_FILE); | ||
| } | ||
|
|
||
| /** | ||
| * Get the path to the config directory. | ||
| * @param cwd - Working directory (defaults to process.cwd()) | ||
| * @returns Absolute path to .git-mem/ | ||
| */ | ||
| export function getConfigDir(cwd?: string): string { | ||
| const dir = cwd ?? process.cwd(); | ||
| return join(dir, CONFIG_DIR); | ||
| } | ||
|
|
||
| const DEFAULTS: IHookConfig = { | ||
| hooks: { | ||
| enabled: true, | ||
|
|
@@ -30,15 +57,14 @@ const DEFAULTS: IHookConfig = { | |
| }; | ||
|
|
||
| export function loadHookConfig(cwd?: string): IHookConfig { | ||
| const dir = cwd ?? process.cwd(); | ||
| const configPath = join(dir, '.git-mem.json'); | ||
| const configPath = getConfigPath(cwd); | ||
|
|
||
| if (!existsSync(configPath)) { | ||
| return DEFAULTS; | ||
| } | ||
|
|
||
| try { | ||
| const raw = JSON.parse(readFileSync(configPath, 'utf8')) as Record<string, unknown>; | ||
| const raw = parseYaml(readFileSync(configPath, 'utf8')) as Record<string, unknown>; | ||
| const rawHooks = (raw.hooks ?? {}) as Partial<IHooksConfig>; | ||
|
Comment on lines
59
to
68
|
||
|
|
||
| const rawStop = (rawHooks.sessionStop ?? {}) as Record<string, unknown>; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,12 +4,19 @@ | |
|
|
||
| import { describe, it, before, after } from 'node:test'; | ||
| import assert from 'node:assert/strict'; | ||
| import { mkdtempSync, writeFileSync, rmSync } from 'fs'; | ||
| import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; | ||
| import { join } from 'path'; | ||
| import { tmpdir } from 'os'; | ||
| import { loadHookConfig } from '../../../../src/hooks/utils/config'; | ||
|
|
||
| describe('loadHookConfig', () => { | ||
| import { stringify as stringifyYaml } from 'yaml'; | ||
| import { | ||
| loadHookConfig, | ||
| getConfigPath, | ||
| getConfigDir, | ||
| CONFIG_DIR, | ||
| CONFIG_FILE, | ||
| } from '../../../../src/hooks/utils/config'; | ||
|
|
||
| describe('config', () => { | ||
| let tempDir: string; | ||
|
|
||
| before(() => { | ||
|
|
@@ -24,74 +31,132 @@ describe('loadHookConfig', () => { | |
| return mkdtempSync(join(tempDir, 'test-')); | ||
| } | ||
|
|
||
| it('should return defaults when no config file exists', () => { | ||
| const testDir = createTestDir(); | ||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.memoryLimit, 20); | ||
| assert.equal(config.hooks.sessionStop.enabled, true); | ||
| assert.equal(config.hooks.sessionStop.autoExtract, true); | ||
| assert.equal(config.hooks.sessionStop.threshold, 3); | ||
| assert.equal(config.hooks.promptSubmit.enabled, false); | ||
| assert.equal(config.hooks.promptSubmit.recordPrompts, false); | ||
| assert.equal(config.hooks.promptSubmit.surfaceContext, true); | ||
| }); | ||
|
|
||
| it('should read and merge config from .git-mem.json', () => { | ||
| const testDir = createTestDir(); | ||
| writeFileSync(join(testDir, '.git-mem.json'), JSON.stringify({ | ||
| hooks: { | ||
| enabled: true, | ||
| sessionStart: { enabled: true, memoryLimit: 50 }, | ||
| promptSubmit: { enabled: true }, | ||
| }, | ||
| })); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.sessionStart.memoryLimit, 50); | ||
| assert.equal(config.hooks.promptSubmit.enabled, true); | ||
| // Defaults preserved for unset fields | ||
| assert.equal(config.hooks.promptSubmit.recordPrompts, false); | ||
| assert.equal(config.hooks.sessionStop.enabled, true); | ||
| }); | ||
|
|
||
| it('should return defaults for invalid JSON', () => { | ||
| const testDir = createTestDir(); | ||
| writeFileSync(join(testDir, '.git-mem.json'), 'not-json{{{'); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.memoryLimit, 20); | ||
| }); | ||
|
|
||
| it('should return defaults when hooks key is missing', () => { | ||
| const testDir = createTestDir(); | ||
| writeFileSync(join(testDir, '.git-mem.json'), JSON.stringify({ other: 'stuff' })); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
| function writeConfig(testDir: string, config: object): void { | ||
| const configDir = join(testDir, CONFIG_DIR); | ||
| mkdirSync(configDir, { recursive: true }); | ||
| writeFileSync(join(configDir, CONFIG_FILE), stringifyYaml(config)); | ||
| } | ||
|
|
||
| assert.equal(config.hooks.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.enabled, true); | ||
| describe('getConfigPath', () => { | ||
| it('should return path to .git-mem/.git-mem.yaml', () => { | ||
| const testDir = '/some/dir'; | ||
| const result = getConfigPath(testDir); | ||
| assert.equal(result, '/some/dir/.git-mem/.git-mem.yaml'); | ||
| }); | ||
|
|
||
| it('should use process.cwd() when no cwd provided', () => { | ||
| const result = getConfigPath(); | ||
| assert.ok(result.endsWith('.git-mem/.git-mem.yaml')); | ||
| }); | ||
|
Comment on lines
+41
to
+50
|
||
| }); | ||
|
|
||
| it('should allow disabling all hooks', () => { | ||
| const testDir = createTestDir(); | ||
| writeFileSync(join(testDir, '.git-mem.json'), JSON.stringify({ | ||
| hooks: { enabled: false }, | ||
| })); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, false); | ||
| describe('getConfigDir', () => { | ||
| it('should return path to .git-mem directory', () => { | ||
| const testDir = '/some/dir'; | ||
| const result = getConfigDir(testDir); | ||
| assert.equal(result, '/some/dir/.git-mem'); | ||
| }); | ||
|
|
||
| it('should use process.cwd() when no cwd provided', () => { | ||
| const result = getConfigDir(); | ||
| assert.ok(result.endsWith('.git-mem')); | ||
| }); | ||
| }); | ||
|
|
||
| it('should use process.cwd() when no cwd provided', () => { | ||
| // This should not throw — just returns defaults if no .git-mem.json | ||
| const config = loadHookConfig(); | ||
| assert.ok(config.hooks); | ||
| describe('loadHookConfig', () => { | ||
| it('should return defaults when no config file exists', () => { | ||
| const testDir = createTestDir(); | ||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.memoryLimit, 20); | ||
| assert.equal(config.hooks.sessionStop.enabled, true); | ||
| assert.equal(config.hooks.sessionStop.autoExtract, true); | ||
| assert.equal(config.hooks.sessionStop.threshold, 3); | ||
| assert.equal(config.hooks.promptSubmit.enabled, false); | ||
| assert.equal(config.hooks.promptSubmit.recordPrompts, false); | ||
| assert.equal(config.hooks.promptSubmit.surfaceContext, true); | ||
| }); | ||
|
|
||
| it('should read and merge config from .git-mem/.git-mem.yaml', () => { | ||
| const testDir = createTestDir(); | ||
| writeConfig(testDir, { | ||
| hooks: { | ||
| enabled: true, | ||
| sessionStart: { enabled: true, memoryLimit: 50 }, | ||
| promptSubmit: { enabled: true }, | ||
| }, | ||
| }); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.sessionStart.memoryLimit, 50); | ||
| assert.equal(config.hooks.promptSubmit.enabled, true); | ||
| // Defaults preserved for unset fields | ||
| assert.equal(config.hooks.promptSubmit.recordPrompts, false); | ||
| assert.equal(config.hooks.sessionStop.enabled, true); | ||
| }); | ||
|
|
||
| it('should return defaults for invalid YAML', () => { | ||
| const testDir = createTestDir(); | ||
| const configDir = join(testDir, CONFIG_DIR); | ||
| mkdirSync(configDir, { recursive: true }); | ||
| writeFileSync(join(configDir, CONFIG_FILE), 'not: valid: yaml: {{{}}}'); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.memoryLimit, 20); | ||
| }); | ||
|
|
||
| it('should return defaults when hooks key is missing', () => { | ||
| const testDir = createTestDir(); | ||
| writeConfig(testDir, { other: 'stuff' }); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, true); | ||
| assert.equal(config.hooks.sessionStart.enabled, true); | ||
| }); | ||
|
|
||
| it('should allow disabling all hooks', () => { | ||
| const testDir = createTestDir(); | ||
| writeConfig(testDir, { | ||
| hooks: { enabled: false }, | ||
| }); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.enabled, false); | ||
| }); | ||
|
|
||
| it('should use process.cwd() when no cwd provided', () => { | ||
| // This should not throw — just returns defaults if no config | ||
| const config = loadHookConfig(); | ||
| assert.ok(config.hooks); | ||
| }); | ||
|
|
||
| it('should handle commitMsg configuration', () => { | ||
| const testDir = createTestDir(); | ||
| writeConfig(testDir, { | ||
| hooks: { | ||
| commitMsg: { | ||
| enabled: true, | ||
| enrich: false, | ||
| enrichTimeout: 10000, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const config = loadHookConfig(testDir); | ||
|
|
||
| assert.equal(config.hooks.commitMsg.enabled, true); | ||
| assert.equal(config.hooks.commitMsg.enrich, false); | ||
| assert.equal(config.hooks.commitMsg.enrichTimeout, 10000); | ||
| // Defaults preserved | ||
| assert.equal(config.hooks.commitMsg.autoAnalyze, true); | ||
| assert.equal(config.hooks.commitMsg.inferTags, true); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSDoc for
getConfigPath()/getConfigDir()says the returned path is absolute, butjoin(cwd, ...)will return a relative path if the caller passes a relativecwd. Consider either resolvingcwd(e.g., viaresolve()) or adjusting the docs to say the path is relative whencwdis relative.