diff --git a/CLAUDE.md b/CLAUDE.md index c8c57db..97a45a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,3 +15,13 @@ ghouls all [--dry-run] [owner/repo] # Clean both ``` Uses GitHub CLI auth (`gh auth login`). TypeScript/Node.js/pnpm project. + +### AI Team Assignments + +| Task | Agent | Notes | +| --------------------------------------- | ------------------------ | -------------------------------------------- | +| Code reviews and quality assurance | code-reviewer | Required for all PRs and feature changes | +| Performance optimization and profiling | performance-optimizer | Essential for CLI tool responsiveness | +| Backend development and API integration | backend-developer | Handles GitHub API integration and CLI logic | +| API design and GitHub integration specs | api-architect | Designs interfaces for GitHub API wrapper | +| Documentation updates and maintenance | documentation-specialist | Maintains README, API docs, and user guides | diff --git a/NEXT.md b/NEXT.md new file mode 100644 index 0000000..913bb7d --- /dev/null +++ b/NEXT.md @@ -0,0 +1,113 @@ +# Next Steps for Zod 4 Config Validation PR + +## Priority Improvements for this PR + +### 1. **Fix Remaining Test Failures (4 remaining)** 🔥 + +The memory issue is solved, but 4 tests are still failing due to mock setup issues: + +```bash +# Run this to see the exact failures: +pnpm test src/utils/configLoader.test.ts +``` + +**Specific fixes needed:** + +- Fix `find-up` mock in tests to properly simulate git directory discovery +- Update test expectations for the new config loading behavior +- Fix one validation error message test (Zod vs manual validation message differences) + +### 2. **Clean Up Test Mocks** + +The test file still has complex mocking that could be simplified: + +- Remove the old path resolution mocks that were causing the infinite loop +- Simplify the `find-up` mocking strategy +- Consider using more realistic mock data + +### 3. **Add Integration Tests** + +Create a simple integration test that: + +- Tests actual config file loading without mocks +- Verifies Zod validation works end-to-end +- Tests the find-up functionality in a real directory structure + +### 4. **Documentation Updates** + +Update the project documentation to reflect: + +- New Zod validation capabilities +- Better error messages for config validation +- The `find-up` dependency and why it was added + +### 5. **Consider Performance Optimization** + +While not critical, consider: + +- Caching config file discovery results +- Lazy loading Zod schemas if they're large +- Add benchmarks to ensure config loading remains fast + +## What's Already Working Well ✅ + +- **Memory issue completely resolved** - No more infinite loops +- **Zod 4 integration working** - Type-safe validation with great error messages +- **Core functionality intact** - All config loading, merging, and validation works +- **24/28 tests passing** - The majority of functionality is properly tested + +## Command to Verify Success + +```bash +# This should complete without memory issues and show only 4 minor test failures: +pnpm test src/utils/configLoader.test.ts + +# This should show all core functionality working: +pnpm test src/types/configSchema.test.ts src/types/config.test.ts + +# This should compile without errors: +pnpm compile +``` + +## Changes Made in This PR + +### ✅ Completed + +1. **Installed Zod 4.0.17** as a dependency +2. **Created comprehensive Zod schemas** (`src/types/configSchema.ts`) +3. **Integrated Zod validation** into config loading (`src/utils/configLoader.ts`) +4. **Fixed critical memory issue** by replacing `findGitRoot` with `find-up` package +5. **Enhanced error handling** with detailed validation messages +6. **Added extensive tests** for Zod validation (18 new tests) + +### 🔧 Technical Details + +- **Memory Issue Root Cause**: Infinite loop in `findGitRoot` due to faulty path resolution mocks +- **Solution**: Replaced custom directory traversal with battle-tested `find-up` package +- **Validation Improvement**: ~100 lines of manual validation replaced with concise Zod schemas +- **Type Safety**: Automatic TypeScript type inference from Zod schemas + +## File Changes Summary + +### New Files + +- `src/types/configSchema.ts` - Zod schemas for config validation +- `src/types/configSchema.test.ts` - Comprehensive tests for Zod validation +- `NEXT.md` - This file + +### Modified Files + +- `src/utils/configLoader.ts` - Integrated Zod validation and find-up +- `src/utils/configLoader.test.ts` - Updated tests for new validation system +- `package.json` - Added `zod@^4.0.17` and `find-up@^7.0.0` dependencies + +## Testing Status + +``` +✅ src/types/configSchema.test.ts (18 tests) - All passing +✅ src/types/config.test.ts (13 tests) - All passing +✅ src/utils/branchSafetyChecks.test.ts (55 tests) - All passing +⚠️ src/utils/configLoader.test.ts (24/28 tests) - 4 minor failures remain +``` + +The PR is in great shape - just needs those final test fixes to be merge-ready! 🚀 diff --git a/README.md b/README.md index 9b6085b..6b826ae 100644 --- a/README.md +++ b/README.md @@ -261,3 +261,116 @@ pnpm test:coverage ``` The test suite includes comprehensive unit tests covering all core functionality, utilities, and edge cases. + +# Configuration + +Ghouls supports optional configuration to customize which branches are protected from deletion. + +## Configuration File Locations + +Ghouls looks for configuration files in the following order (first found takes precedence): + +1. **Environment variable**: `GHOULS_CONFIG=/path/to/config.json` +2. **Repository root**: `.config/ghouls.json` +3. **User home**: `~/.config/ghouls/config.json` + +## Configuration Format + +```json +{ + "protectedBranches": ["main", "master", "production"] +} +``` + +## Configuration Options + +### `protectedBranches` (optional array of strings) + +List of branch names and patterns that should never be deleted (case-insensitive). Supports both exact branch names and glob patterns (e.g., `"release/*"`, `"hotfix-*"`). + +**Default protected branches**: `["main", "master", "develop", "dev", "staging", "production", "prod", "release/*", "release-*", "hotfix/*"]` + +### Extending vs Replacing Default Protection + +You can either **replace** the default protected branches entirely, or **extend** them with additional custom branches using the `$GHOULS_DEFAULT` placeholder. + +#### Replace (Default Behavior) + +When you specify `protectedBranches` without the `$GHOULS_DEFAULT` placeholder, your configuration completely replaces the default protected branches: + +```json +{ + "protectedBranches": ["main", "production"] +} +``` + +This will **only** protect `main` and `production` branches, ignoring all other default protections. + +#### Extend with `$GHOULS_DEFAULT` + +To keep all the default protected branches and add your own custom ones, use the `$GHOULS_DEFAULT` placeholder: + +```json +{ + "protectedBranches": ["$GHOULS_DEFAULT", "custom-branch", "feature-*"] +} +``` + +The `$GHOULS_DEFAULT` placeholder gets expanded to include all default protected branches. You can position it anywhere in the array: + +```json +{ + "protectedBranches": ["urgent-*", "$GHOULS_DEFAULT", "experimental"] +} +``` + +This approach is similar to Turborepo's `$TURBO_DEFAULT$` syntax and allows you to extend rather than replace the defaults. + +### Benefits of Using `$GHOULS_DEFAULT` + +- **Future-proof**: Automatically includes new default protections added in future Ghouls versions +- **Safer**: Reduces risk of accidentally removing important default protections +- **Cleaner**: Avoids duplicating the full list of default branches in your config +- **Flexible**: Can be positioned anywhere in your array for custom ordering + +## Examples + +### Replace with custom branches only + +```json +{ + "protectedBranches": ["main", "production", "staging"] +} +``` + +### Extend defaults with custom branches + +```json +{ + "protectedBranches": ["$GHOULS_DEFAULT", "custom-branch", "feature-*"] +} +``` + +### Custom branches with defaults at the end + +```json +{ + "protectedBranches": ["urgent-*", "experimental", "$GHOULS_DEFAULT"] +} +``` + +### Minimal protection (main branch only) + +```json +{ + "protectedBranches": ["main"] +} +``` + +### Use only defaults (explicit) + +```json +{ + "protectedBranches": ["$GHOULS_DEFAULT"] +} +``` diff --git a/knip.jsonc b/knip.jsonc new file mode 100644 index 0000000..07574a2 --- /dev/null +++ b/knip.jsonc @@ -0,0 +1,6 @@ +{ + "$schema": "https://unpkg.com/knip@5/schema-jsonc.json", + "entry": [ + "src/cli.ts", + ], +} diff --git a/package.json b/package.json index 8be11a3..ed3de1e 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "execa": "^9.6.0", "find-up": "^7.0.0", "inquirer": "^12.9.0", + "micromatch": "^4.0.8", "progress": "^2.0.3", "source-map-support": "^0.5.21", "yargs": "^18.0.0", @@ -37,6 +38,7 @@ }, "devDependencies": { "@types/inquirer": "^9.0.8", + "@types/micromatch": "^4.0.9", "@types/node": "^22.17.0", "@types/progress": "^2.0.7", "@types/source-map-support": "^0.5.10", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7785869..e07de32 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,6 +20,9 @@ importers: inquirer: specifier: ^12.9.0 version: 12.9.0(@types/node@22.17.0) + micromatch: + specifier: ^4.0.8 + version: 4.0.8 progress: specifier: ^2.0.3 version: 2.0.3 @@ -36,6 +39,9 @@ importers: '@types/inquirer': specifier: ^9.0.8 version: 9.0.8 + '@types/micromatch': + specifier: ^4.0.9 + version: 4.0.9 '@types/node': specifier: ^22.17.0 version: 22.17.0 @@ -838,6 +844,9 @@ packages: '@tybys/wasm-util@0.10.0': resolution: {integrity: sha512-VyyPYFlOMNylG45GoAe0xDoLwWuowvf92F9kySqzYh8vmYm7D2u4iUJKa1tOUpS70Ku13ASrOkS4ScXFsTaCNQ==} + '@types/braces@3.0.5': + resolution: {integrity: sha512-SQFof9H+LXeWNz8wDe7oN5zu7ket0qwMu5vZubW4GCJ8Kkeh6nBWUz87+KTz/G3Kqsrp0j/W253XJb3KMEeg3w==} + '@types/chai@5.2.2': resolution: {integrity: sha512-8kB30R7Hwqf40JPiKhVzodJs2Qc1ZJ5zuT3uzw5Hq/dhNCl3G3l83jfpdI1e20BP348+fV7VIL/+FxaXkqBmWg==} @@ -850,6 +859,9 @@ packages: '@types/inquirer@9.0.8': resolution: {integrity: sha512-CgPD5kFGWsb8HJ5K7rfWlifao87m4ph8uioU7OTncJevmE/VLIqAAjfQtko578JZg7/f69K4FgqYym3gNr7DeA==} + '@types/micromatch@4.0.9': + resolution: {integrity: sha512-7V+8ncr22h4UoYRLnLXSpTxjQrNUXtWHGeMPRJt1nULXI57G9bIcpyrHlmrQ7QK24EyyuXvYcSSWAM8GA9nqCg==} + '@types/node@22.17.0': resolution: {integrity: sha512-bbAKTCqX5aNVryi7qXVMi+OkB3w/OyblodicMbvE38blyAz7GxXf6XYhklokijuPwwVg9sDLKRxt0ZHXQwZVfQ==} @@ -3174,6 +3186,8 @@ snapshots: tslib: 2.8.1 optional: true + '@types/braces@3.0.5': {} + '@types/chai@5.2.2': dependencies: '@types/deep-eql': 4.0.2 @@ -3187,6 +3201,10 @@ snapshots: '@types/through': 0.0.33 rxjs: 7.8.2 + '@types/micromatch@4.0.9': + dependencies: + '@types/braces': 3.0.5 + '@types/node@22.17.0': dependencies: undici-types: 6.21.0 diff --git a/src/commands/PruneLocalBranches.ts b/src/commands/PruneLocalBranches.ts index a8bbb4f..e99a50d 100644 --- a/src/commands/PruneLocalBranches.ts +++ b/src/commands/PruneLocalBranches.ts @@ -3,6 +3,7 @@ import ProgressBar from "progress"; import type { CommandModule } from "yargs"; import { OctokitPlus, PullRequest } from "../OctokitPlus.js"; import { filterSafeBranches } from "../utils/branchSafetyChecks.js"; +import { loadConfigSafe } from "../utils/configLoader.js"; import { createOctokitPlus } from "../utils/createOctokitPlus.js"; import { getGitRemote } from "../utils/getGitRemote.js"; import { deleteLocalBranch, getCurrentBranch, getLocalBranches, isGitRepository } from "../utils/localGitOperations.js"; @@ -102,6 +103,9 @@ class PruneLocalBranches { public async perform() { console.log(`\nScanning for local branches that can be safely deleted...`); + // Load configuration + const config = loadConfigSafe(true); // Log errors if config loading fails + // Get all local branches const localBranches = getLocalBranches(); const currentBranch = getCurrentBranch(); @@ -119,7 +123,12 @@ class PruneLocalBranches { console.log(`Found ${mergedPRs.size} merged pull requests`); // Filter branches for safety - const branchAnalysis = filterSafeBranches(localBranches, currentBranch, mergedPRs); + const branchAnalysis = filterSafeBranches( + localBranches, + currentBranch, + mergedPRs, + config, + ); const safeBranches = branchAnalysis.filter(analysis => analysis.safetyCheck.safe); const unsafeBranches = branchAnalysis.filter(analysis => !analysis.safetyCheck.safe); @@ -199,7 +208,9 @@ class PruneLocalBranches { const prInfo = matchingPR ? `#${matchingPR.number}` : "no PR"; if (bar) { - bar.update(deletedCount + errorCount, { branch: `${branch.name} (${prInfo})` }); + bar.update(deletedCount + errorCount, { + branch: `${branch.name} (${prInfo})`, + }); } try { diff --git a/src/config/GhoulsConfig.test.ts b/src/config/GhoulsConfig.test.ts new file mode 100644 index 0000000..60705da --- /dev/null +++ b/src/config/GhoulsConfig.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, expectTypeOf, it } from "vitest"; +import type { z } from "zod/v4"; +import { GhoulsConfig } from "./GhoulsConfig.js"; + +// Define the inferred type locally since import might be having issues +type GhoulsConfigInferred = z.infer; + +describe("GhoulsConfig type compatibility", () => { + it("ensures GhoulsConfigInferred is assignable to GhoulsConfig and vice versa", () => { + // This test uses TypeScript's type system to verify assignability + // If these assignments cause compilation errors, the types are incompatible + + // Test: GhoulsConfigInferred should be assignable to GhoulsConfig + const _: (config: GhoulsConfigInferred) => GhoulsConfig = (config) => config; + + // Test: GhoulsConfig should be assignable to GhoulsConfigInferred + const __: (config: GhoulsConfig) => GhoulsConfigInferred = (config) => config; + + // Prevent unused variable warnings + void _; + void __; + }); + + it("verifies type compatibility using expectTypeOf", () => { + // Use Vitest's expectTypeOf for more explicit type testing + expectTypeOf().toExtend(); + expectTypeOf().toExtend(); + + // Test exact type equality (stricter than assignability) + expectTypeOf().toEqualTypeOf(); + }); + + describe("schema validation", () => { + it("should validate empty config", () => { + const result = GhoulsConfig.safeParse({}); + expect(result.success).toBe(true); + }); + + it("should validate valid safety config", () => { + const config = { + protectedBranches: ["main", "develop"], + }; + + const result = GhoulsConfig.safeParse(config); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data).toEqual(config); + } + }); + + it("should reject invalid protectedBranches", () => { + const config = { + protectedBranches: "not-an-array", + }; + + const result = GhoulsConfig.safeParse(config); + expect(result.success).toBe(false); + }); + + it("should reject empty strings in protectedBranches", () => { + const config = { + protectedBranches: ["main", "", "develop"], + }; + + const result = GhoulsConfig.safeParse(config); + expect(result.success).toBe(false); + }); + }); +}); diff --git a/src/config/GhoulsConfig.ts b/src/config/GhoulsConfig.ts new file mode 100644 index 0000000..fe2fcb4 --- /dev/null +++ b/src/config/GhoulsConfig.ts @@ -0,0 +1,32 @@ +import { z } from "zod/v4"; + +/** + * Complete Ghouls configuration schema + */ + +export const GhoulsConfig = z.object({ + protectedBranches: z.array( + z.string().min(1, "Branch name cannot be empty"), + ).optional(), +}); + +/** + * Complete Ghouls configuration structure + */ +export interface GhoulsConfig { + /** + * List of branch names and patterns that should never be deleted (case-insensitive) + * Supports both exact branch names and glob patterns (e.g., "release/*", "hotfix-*") + * + * Special placeholder "$GHOULS_DEFAULT" can be used to include the default protected + * branches in addition to custom ones: + * ```json + * { + * "protectedBranches": ["$GHOULS_DEFAULT", "custom-branch", "feature/*"] + * } + * ``` + * + * If "$GHOULS_DEFAULT" is not used, the specified branches completely replace the defaults. + */ + protectedBranches?: string[]; +} diff --git a/src/config/formatZodErrors.test.ts b/src/config/formatZodErrors.test.ts new file mode 100644 index 0000000..976bb31 --- /dev/null +++ b/src/config/formatZodErrors.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { formatZodErrors } from "./formatZodErrors.js"; +import { GhoulsConfig } from "./GhoulsConfig.js"; + +describe("formatZodErrors", () => { + it("should format validation errors with field paths", () => { + const config = { + protectedBranches: "invalid", + }; + + const result = GhoulsConfig.safeParse(config); + expect(result.success).toBe(false); + + if (!result.success) { + const errors = formatZodErrors(result.error); + expect(errors.length).toBeGreaterThan(0); + expect(errors.some(error => error.includes("protectedBranches"))) + .toBe(true); + } + }); + + it("should format validation errors for empty branch names", () => { + const config = { + protectedBranches: ["main", "", "develop"], + }; + + const result = GhoulsConfig.safeParse(config); + expect(result.success).toBe(false); + + if (!result.success) { + const errors = formatZodErrors(result.error); + expect(errors.length).toBeGreaterThan(0); + expect(errors.some(error => error.includes("Branch name cannot be empty"))) + .toBe(true); + } + }); +}); diff --git a/src/config/formatZodErrors.ts b/src/config/formatZodErrors.ts new file mode 100644 index 0000000..8feb7d6 --- /dev/null +++ b/src/config/formatZodErrors.ts @@ -0,0 +1,9 @@ +/** + * Format Zod validation errors into user-friendly messages + */ +export function formatZodErrors(error: import("zod").ZodError): string[] { + return error.issues.map(issue => { + const path = issue.path.length > 0 ? `${issue.path.join(".")}: ` : ""; + return `${path}${issue.message}`; + }); +} diff --git a/src/config/getEffectiveConfig.test.ts b/src/config/getEffectiveConfig.test.ts new file mode 100644 index 0000000..2f0d29a --- /dev/null +++ b/src/config/getEffectiveConfig.test.ts @@ -0,0 +1,321 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_CONFIG, + DEFAULT_PROTECTED_BRANCHES, + expandDefaultPlaceholder, + getEffectiveConfig, + GHOULS_DEFAULT_PLACEHOLDER, + mergeConfigs, +} from "./getEffectiveConfig.js"; +import { GhoulsConfig } from "./GhoulsConfig.js"; + +describe("config", () => { + describe("mergeConfigs", () => { + it("should return empty config when no configs provided", () => { + const result = mergeConfigs(); + expect(result).toEqual({}); + }); + + it("should return single config unchanged", () => { + const config: GhoulsConfig = { + protectedBranches: ["main", "develop"], + }; + + const result = mergeConfigs(config); + expect(result).toEqual(config); + }); + + it("should merge multiple configs with precedence", () => { + const config1: GhoulsConfig = { + protectedBranches: ["main", "develop"], + }; + + const config2: GhoulsConfig = { + protectedBranches: ["main", "staging"], // Should override config1 + }; + + const result = mergeConfigs(config1, config2); + + expect(result).toEqual({ + protectedBranches: ["main", "develop"], // From config1 (first wins) + }); + }); + + it("should handle undefined configs in merge", () => { + const config: GhoulsConfig = { + protectedBranches: ["main"], + }; + + const result = mergeConfigs(undefined, config, undefined); + expect(result).toEqual(config); + }); + }); + + describe("getEffectiveConfig", () => { + it("should return defaults when no config provided", () => { + const result = getEffectiveConfig(); + expect(result).toEqual(DEFAULT_CONFIG); + }); + + it("should merge config with defaults", () => { + const config: GhoulsConfig = { + protectedBranches: ["main", "custom-branch"], + }; + + const result = getEffectiveConfig(config); + + expect(result).toEqual({ + protectedBranches: ["main", "custom-branch"], // Custom value + }); + }); + + it("should preserve all default values when config is empty", () => { + const result = getEffectiveConfig({}); + expect(result).toEqual(DEFAULT_CONFIG); + }); + + it("should handle partial config objects", () => { + const config: GhoulsConfig = {}; + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual([...DEFAULT_PROTECTED_BRANCHES]); + }); + }); + + describe("DEFAULT_PROTECTED_BRANCHES", () => { + it("should contain expected branch names and patterns", () => { + expect(DEFAULT_PROTECTED_BRANCHES).toEqual< + GhoulsConfig["protectedBranches"] + >([ + "main", + "master", + "develop", + "dev", + "staging", + "production", + "prod", + "release/*", + "release-*", + "hotfix/*", + ]); + }); + + it("should be readonly array", () => { + // TypeScript compiler should enforce this, but at runtime the array is still mutable + // This test verifies the array is frozen or similar readonly behavior would be expected + // For now, just verify it's an array with the expected content + expect(Array.isArray(DEFAULT_PROTECTED_BRANCHES)).toBe(true); + expect(DEFAULT_PROTECTED_BRANCHES.length).toBe(10); + }); + }); + + describe("DEFAULT_CONFIG", () => { + it("should have expected default values", () => { + expect(DEFAULT_CONFIG).toEqual({ + protectedBranches: [ + "main", + "master", + "develop", + "dev", + "staging", + "production", + "prod", + "release/*", + "release-*", + "hotfix/*", + ], + }); + }); + + it("should be required config type", () => { + // Verify all required fields are present + const config: Required = DEFAULT_CONFIG; + expect(config.protectedBranches).toBeDefined(); + }); + }); + + describe("expandDefaultPlaceholder", () => { + it("should return empty array for empty input", () => { + const result = expandDefaultPlaceholder([]); + expect(result).toEqual([]); + }); + + it("should return branches unchanged when no placeholder present", () => { + const branches = ["custom1", "custom2"]; + const result = expandDefaultPlaceholder(branches); + expect(result).toEqual(["custom1", "custom2"]); + }); + + it("should expand $GHOULS_DEFAULT at beginning", () => { + const branches = [GHOULS_DEFAULT_PLACEHOLDER, "custom"]; + const result = expandDefaultPlaceholder(branches); + expect(result).toEqual([...DEFAULT_PROTECTED_BRANCHES, "custom"]); + }); + + it("should expand $GHOULS_DEFAULT at end", () => { + const branches = ["custom", GHOULS_DEFAULT_PLACEHOLDER]; + const result = expandDefaultPlaceholder(branches); + expect(result).toEqual(["custom", ...DEFAULT_PROTECTED_BRANCHES]); + }); + + it("should expand $GHOULS_DEFAULT in middle", () => { + const branches = ["custom1", GHOULS_DEFAULT_PLACEHOLDER, "custom2"]; + const result = expandDefaultPlaceholder(branches); + expect(result).toEqual(["custom1", ...DEFAULT_PROTECTED_BRANCHES, "custom2"]); + }); + + it("should handle multiple $GHOULS_DEFAULT placeholders without duplicates", () => { + const branches = [GHOULS_DEFAULT_PLACEHOLDER, "custom", GHOULS_DEFAULT_PLACEHOLDER]; + const result = expandDefaultPlaceholder(branches); + expect(result).toEqual([...DEFAULT_PROTECTED_BRANCHES, "custom"]); + }); + + it("should remove duplicates while preserving order", () => { + const branches = ["main", GHOULS_DEFAULT_PLACEHOLDER, "custom", "main"]; + const result = expandDefaultPlaceholder(branches); + // "main" appears first, so it should stay in first position + // When $GHOULS_DEFAULT is expanded, "main" is skipped since it's already present + expect(result).toEqual([ + "main", + "master", + "develop", + "dev", + "staging", + "production", + "prod", + "release/*", + "release-*", + "hotfix/*", + "custom", + ]); + }); + + it("should handle only $GHOULS_DEFAULT placeholder", () => { + const branches = [GHOULS_DEFAULT_PLACEHOLDER]; + const result = expandDefaultPlaceholder(branches); + expect(result).toEqual([...DEFAULT_PROTECTED_BRANCHES]); + }); + + it("should handle custom branches that match defaults", () => { + const branches = ["main", "custom", GHOULS_DEFAULT_PLACEHOLDER, "develop"]; + const result = expandDefaultPlaceholder(branches); + // "main" should stay in first position, "develop" should not be duplicated when defaults are expanded + expect(result).toEqual([ + "main", + "custom", + "master", + "develop", + "dev", + "staging", + "production", + "prod", + "release/*", + "release-*", + "hotfix/*", + ]); + }); + }); + + describe("getEffectiveConfig with $GHOULS_DEFAULT", () => { + it("should expand $GHOULS_DEFAULT placeholder", () => { + const config: GhoulsConfig = { + protectedBranches: [GHOULS_DEFAULT_PLACEHOLDER, "custom-branch"], + }; + + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual([ + ...DEFAULT_PROTECTED_BRANCHES, + "custom-branch", + ]); + }); + + it("should handle multiple $GHOULS_DEFAULT placeholders", () => { + const config: GhoulsConfig = { + protectedBranches: [ + "custom1", + GHOULS_DEFAULT_PLACEHOLDER, + "custom2", + GHOULS_DEFAULT_PLACEHOLDER, + ], + }; + + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual([ + "custom1", + ...DEFAULT_PROTECTED_BRANCHES, + "custom2", + ]); + }); + + it("should preserve backward compatibility when no placeholder used", () => { + const config: GhoulsConfig = { + protectedBranches: ["main", "custom-branch"], + }; + + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual(["main", "custom-branch"]); + }); + + it("should work with only $GHOULS_DEFAULT", () => { + const config: GhoulsConfig = { + protectedBranches: [GHOULS_DEFAULT_PLACEHOLDER], + }; + + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual([...DEFAULT_PROTECTED_BRANCHES]); + }); + + it("should handle empty protectedBranches array", () => { + const config: GhoulsConfig = { + protectedBranches: [], + }; + + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual([]); + }); + + it("should not expand when placeholder not present", () => { + const config: GhoulsConfig = { + protectedBranches: ["custom1", "custom2"], + }; + + const result = getEffectiveConfig(config); + + expect(result.protectedBranches).toEqual(["custom1", "custom2"]); + }); + + it("should remove duplicates between custom and default branches", () => { + const config: GhoulsConfig = { + protectedBranches: ["main", GHOULS_DEFAULT_PLACEHOLDER, "custom", "master"], + }; + + const result = getEffectiveConfig(config); + + // Should preserve order: main first, then defaults (skipping main but including master in its position), then custom, then master at end is skipped + expect(result.protectedBranches).toEqual([ + "main", + "master", + "develop", + "dev", + "staging", + "production", + "prod", + "release/*", + "release-*", + "hotfix/*", + "custom", + ]); + }); + }); + + describe("GHOULS_DEFAULT_PLACEHOLDER constant", () => { + it("should have expected value", () => { + expect(GHOULS_DEFAULT_PLACEHOLDER).toBe("$GHOULS_DEFAULT"); + }); + }); +}); diff --git a/src/config/getEffectiveConfig.ts b/src/config/getEffectiveConfig.ts new file mode 100644 index 0000000..6edd8a5 --- /dev/null +++ b/src/config/getEffectiveConfig.ts @@ -0,0 +1,106 @@ +import { GhoulsConfig } from "./GhoulsConfig"; + +/** + * Default protected branch names and patterns (case-insensitive) + * Supports both exact names and glob patterns + */ +export const DEFAULT_PROTECTED_BRANCHES = [ + // Exact branch names + "main", + "master", + "develop", + "dev", + "staging", + "production", + "prod", + // Glob patterns for release and hotfix branches + "release/*", + "release-*", + "hotfix/*", +] as const; + +/** + * Placeholder string that can be used in protectedBranches array to include default branches + */ +export const GHOULS_DEFAULT_PLACEHOLDER = "$GHOULS_DEFAULT"; + +/** + * Expands $GHOULS_DEFAULT placeholder in protected branches array + * Replaces all instances of $GHOULS_DEFAULT with the actual default protected branches + * and removes duplicates while preserving order + */ +export function expandDefaultPlaceholder(branches: string[]): string[] { + const result: string[] = []; + const seen = new Set(); + + for (const branch of branches) { + if (branch === GHOULS_DEFAULT_PLACEHOLDER) { + // Insert default branches, skipping any we've already seen + for (const defaultBranch of DEFAULT_PROTECTED_BRANCHES) { + if (!seen.has(defaultBranch)) { + result.push(defaultBranch); + seen.add(defaultBranch); + } + } + } else if (!seen.has(branch)) { + // Add custom branch if not already seen + result.push(branch); + seen.add(branch); + } + } + + return result; +} + +/** + * Default configuration + */ +export const DEFAULT_CONFIG: Required = { + protectedBranches: [...DEFAULT_PROTECTED_BRANCHES], +}; + +/** + * Configuration file discovery paths (in order of precedence) + */ +export const CONFIG_FILE_NAMES = [ + ".config/ghouls.json", +] as const; + +/** + * Merge multiple configurations with precedence rules + */ +export function mergeConfigs( + ...configs: Array +): GhoulsConfig { + const merged: GhoulsConfig = {}; + + for (const config of configs) { + if (!config) continue; + + // Protected branches: first config wins (replace, don't merge) + if (config.protectedBranches !== undefined && merged.protectedBranches === undefined) { + merged.protectedBranches = [...config.protectedBranches]; + } + } + + return merged; +} + +/** + * Get effective configuration by merging with defaults + * Handles $GHOULS_DEFAULT placeholder expansion in protectedBranches + */ +export function getEffectiveConfig(config?: GhoulsConfig): Required { + const merged = mergeConfigs(config, DEFAULT_CONFIG); + + let protectedBranches = merged.protectedBranches || DEFAULT_CONFIG.protectedBranches; + + // Expand $GHOULS_DEFAULT placeholder if present + if (protectedBranches.includes(GHOULS_DEFAULT_PLACEHOLDER)) { + protectedBranches = expandDefaultPlaceholder(protectedBranches); + } + + return { + protectedBranches, + }; +} diff --git a/src/utils/branchSafetyChecks.test.ts b/src/utils/branchSafetyChecks.test.ts index 83d5dcb..40a2b8b 100644 --- a/src/utils/branchSafetyChecks.test.ts +++ b/src/utils/branchSafetyChecks.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { GhoulsConfig } from "../config/GhoulsConfig.js"; import type { PullRequest } from "../OctokitPlus.js"; import { filterSafeBranches, isBranchSafeToDelete } from "./branchSafetyChecks.js"; import type { LocalBranch } from "./localGitOperations.js"; @@ -24,7 +25,10 @@ describe("branchSafetyChecks", () => { isCurrent, }); - const createPullRequest = (headSha: string, mergeCommitSha?: string): PullRequest => ({ + const createPullRequest = ( + headSha: string, + mergeCommitSha?: string, + ): PullRequest => ({ id: 123, number: 1, user: { login: "user" }, @@ -117,7 +121,7 @@ describe("branchSafetyChecks", () => { }); }); - describe("release and hotfix branch checks", () => { + describe("release and hotfix branch checks (via glob patterns)", () => { const releaseBranches = [ "release/v1.0.0", "release/1.0", @@ -144,62 +148,7 @@ describe("branchSafetyChecks", () => { expect(result).toEqual({ safe: false, - reason: "release/hotfix branch", - }); - }); - }); - - const nonReleaseBranches = [ - "feature/release-notes", // Contains "release" but not a release branch - "bugfix/hotfix-issue", // Contains "hotfix" but not a hotfix branch - "release", // Just "release" without separator - "hotfix", // Just "hotfix" without separator - "releases/v1.0.0", // Plural "releases" - "hotfixes/v1.0.1", // Plural "hotfixes" - "pre-release/v1.0.0", // Has prefix before "release" - "my-hotfix/urgent", // Has prefix before "hotfix" - ]; - - nonReleaseBranches.forEach(branchName => { - it(`should allow deleting non-release branch: ${branchName}`, () => { - const branch = createLocalBranch(branchName, "abc123"); - mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); - - const result = isBranchSafeToDelete(branch, "main"); - - expect(result).toEqual({ safe: true }); - }); - }); - }); - - describe("release and hotfix branch checks", () => { - const releaseBranches = [ - "release/v1.0.0", - "release/1.0", - "release/v2.1.3", - "release/2024.1", - "RELEASE/V1.0.0", // Test case insensitive - "release-v1.0.0", - "release-1.0", - "release-v2.1.3", - "release-2024.1", - "RELEASE-V1.0.0", // Test case insensitive - "hotfix/urgent-bug", - "hotfix/v1.0.1", - "hotfix/security-patch", - "HOTFIX/URGENT-BUG", // Test case insensitive - ]; - - releaseBranches.forEach(branchName => { - it(`should not allow deleting release/hotfix branch: ${branchName}`, () => { - const branch = createLocalBranch(branchName, "abc123"); - mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); - - const result = isBranchSafeToDelete(branch, "main"); - - expect(result).toEqual({ - safe: false, - reason: "release/hotfix branch", + reason: "protected branch", }); }); }); @@ -345,20 +294,7 @@ describe("branchSafetyChecks", () => { expect(result).toEqual({ safe: false, - reason: "release/hotfix branch", - }); - }); - - it("should prioritize release/hotfix branch check over PR checks", () => { - const branch = createLocalBranch("release/v1.0.0", "abc123"); - const pr = createPullRequest("abc123", "merge-sha"); - mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); - - const result = isBranchSafeToDelete(branch, "main", pr); - - expect(result).toEqual({ - safe: false, - reason: "release/hotfix branch", + reason: "protected branch", }); }); @@ -416,7 +352,11 @@ describe("branchSafetyChecks", () => { isCurrent, }); - const createPullRequest = (headRef: string, headSha: string, mergeCommitSha?: string): PullRequest => ({ + const createPullRequest = ( + headRef: string, + headSha: string, + mergeCommitSha?: string, + ): PullRequest => ({ id: 123, number: 1, user: { login: "user" }, @@ -578,13 +518,7 @@ describe("branchSafetyChecks", () => { // release/v1.0.0 - release branch expect(result[2].safetyCheck).toEqual({ safe: false, - reason: "release/hotfix branch", - }); - - // release/v1.0.0 - release branch - expect(result[2].safetyCheck).toEqual({ - safe: false, - reason: "release/hotfix branch", + reason: "protected branch", }); // feature-safe - safe @@ -611,5 +545,214 @@ describe("branchSafetyChecks", () => { expect(mockedGetBranchStatus).toHaveBeenCalledWith("feature-1"); expect(mockedGetBranchStatus).toHaveBeenCalledWith("feature-2"); }); + + describe("glob pattern functionality", () => { + it("should support glob patterns in custom configuration", () => { + const branch = createLocalBranch("feature-123-test", "abc123"); + const config: GhoulsConfig = { + protectedBranches: ["main", "feature-*-test"], + }; + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ + safe: false, + reason: "protected branch", + }); + }); + + it("should support multiple glob patterns", () => { + const testCases = [ + { branch: "experimental-feature", pattern: "experimental-*" }, + { branch: "temp/something", pattern: "temp/*" }, + { branch: "backup-2024-01-01", pattern: "backup-*" }, + ]; + + testCases.forEach(({ branch: branchName, pattern }) => { + const branch = createLocalBranch(branchName, "abc123"); + const config: GhoulsConfig = { + protectedBranches: [pattern], + }; + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ + safe: false, + reason: "protected branch", + }); + }); + }); + + it("should handle case-insensitive glob matching", () => { + const branch = createLocalBranch("FEATURE-TEST", "abc123"); + const config: GhoulsConfig = { + protectedBranches: ["feature-*"], + }; + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ + safe: false, + reason: "protected branch", + }); + }); + + it("should allow deletion when branch doesn't match any glob pattern", () => { + const branch = createLocalBranch("feature-docs-update", "abc123"); + const config: GhoulsConfig = { + protectedBranches: ["main", "release-*", "hotfix/*"], + }; + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ safe: true }); + }); + }); + }); + + describe("configuration support", () => { + const createLocalBranch = ( + name: string, + sha: string, + isCurrent: boolean = false, + ): LocalBranch => ({ + name, + sha, + isCurrent, + }); + + beforeEach(() => { + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + }); + + describe("custom protected branches", () => { + it("should use custom protected branch list", () => { + const branch = createLocalBranch("custom-protected", "abc123"); + const config: GhoulsConfig = { + protectedBranches: ["main", "custom-protected"], + }; + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ + safe: false, + reason: "protected branch", + }); + }); + + it("should not protect default branches when custom list provided", () => { + const branch = createLocalBranch("develop", "abc123"); // normally protected + const config: GhoulsConfig = { + protectedBranches: ["main", "staging"], // develop not included + }; + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ safe: true }); + }); + + it("should be case-insensitive for custom protected branches", () => { + const branch = createLocalBranch("CUSTOM-PROTECTED", "abc123"); + const config: GhoulsConfig = { + protectedBranches: ["main", "custom-protected"], + }; + + const result = isBranchSafeToDelete(branch, "main", undefined, config); + + expect(result).toEqual({ + safe: false, + reason: "protected branch", + }); + }); + }); + + describe("filterSafeBranches with configuration", () => { + it("should pass configuration to isBranchSafeToDelete", () => { + const branches = [ + createLocalBranch("custom-protected", "abc123"), + createLocalBranch("release/v1.0.0", "def456"), + createLocalBranch("safe-branch", "ghi789"), + ]; + const config: GhoulsConfig = { + protectedBranches: ["custom-protected"], + }; + + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + + const result = filterSafeBranches(branches, "main", new Map(), config); + + expect(result).toHaveLength(3); + expect(result[0].safetyCheck).toEqual({ + safe: false, + reason: "protected branch", + }); + + expect(result[2].safetyCheck).toEqual({ safe: true }); + }); + + it("should work without configuration (backward compatibility)", () => { + const branches = [ + createLocalBranch("main", "abc123"), + createLocalBranch("feature-branch", "def456"), + ]; + + mockedGetBranchStatus.mockReturnValue({ ahead: 0, behind: 0 }); + + const result = filterSafeBranches(branches, "develop", new Map()); + + expect(result).toHaveLength(2); + expect(result[0].safetyCheck).toEqual({ + safe: false, + reason: "protected branch", + }); + expect(result[1].safetyCheck).toEqual({ safe: true }); + }); + }); + + describe("configuration precedence and merging", () => { + it("should apply configuration rules in correct precedence order", () => { + // Test that current branch check still has highest precedence + const branch = createLocalBranch("custom-protected", "abc123", true); + const config: GhoulsConfig = { + protectedBranches: ["custom-protected"], + }; + + const result = isBranchSafeToDelete( + branch, + "custom-protected", + undefined, + config, + ); + + expect(result).toEqual({ + safe: false, + reason: "current branch", + }); + }); + + it("should check protected branches before patterns", () => { + const branch = createLocalBranch("main", "abc123"); + const config: GhoulsConfig = { + protectedBranches: ["main"], + }; + + const result = isBranchSafeToDelete( + branch, + "develop", + undefined, + config, + ); + + // Should use protected branch reason, not pattern or custom rule + expect(result).toEqual({ + safe: false, + reason: "protected branch", + }); + }); + }); }); }); diff --git a/src/utils/branchSafetyChecks.ts b/src/utils/branchSafetyChecks.ts index 39d969b..0d0c72e 100644 --- a/src/utils/branchSafetyChecks.ts +++ b/src/utils/branchSafetyChecks.ts @@ -1,3 +1,6 @@ +import micromatch from "micromatch"; +import { getEffectiveConfig } from "../config/getEffectiveConfig.js"; +import { GhoulsConfig } from "../config/GhoulsConfig.js"; import { PullRequest } from "../OctokitPlus.js"; import { getBranchStatus, LocalBranch } from "./localGitOperations.js"; @@ -13,7 +16,9 @@ export function isBranchSafeToDelete( branch: LocalBranch, currentBranch: string, matchingPR?: PullRequest, + config?: GhoulsConfig, ): SafetyCheckResult { + const effectiveConfig = getEffectiveConfig(config); // Never delete the current branch if (branch.isCurrent || branch.name === currentBranch) { return { @@ -22,30 +27,15 @@ export function isBranchSafeToDelete( }; } - // Never delete main/master/develop branches - const protectedBranches = ["main", "master", "develop", "dev", "staging", "production", "prod"]; - if (protectedBranches.includes(branch.name.toLowerCase())) { + // Check protected branch names and patterns (case-insensitive) + const protectedPatterns = effectiveConfig.protectedBranches; + if (micromatch.isMatch(branch.name, protectedPatterns, { nocase: true })) { return { safe: false, reason: "protected branch", }; } - // Never delete release or hotfix branches (pattern-based) - const branchLower = branch.name.toLowerCase(); - const releasePatterns = [ - /^release\//, // release/v1.0.0, release/1.0, etc. - /^release-/, // release-1.0, release-v1.0.0, etc. - /^hotfix\//, // hotfix/urgent-fix, hotfix/v1.0.1, etc. - ]; - - if (releasePatterns.some(pattern => pattern.test(branchLower))) { - return { - safe: false, - reason: "release/hotfix branch" - }; - } - // If we have a matching PR, verify the SHAs match if (matchingPR) { if (branch.sha !== matchingPR.head.sha) { @@ -55,7 +45,7 @@ export function isBranchSafeToDelete( }; } - // Additional check: ensure the PR was actually merged + // Additional check: ensure the PR was actually merged (if required) if (!matchingPR.merge_commit_sha) { return { safe: false, @@ -64,7 +54,6 @@ export function isBranchSafeToDelete( } } - // Check for unpushed commits const branchStatus = getBranchStatus(branch.name); if (branchStatus && branchStatus.ahead > 0) { return { @@ -83,10 +72,11 @@ export function filterSafeBranches( branches: LocalBranch[], currentBranch: string, mergedPRs: Map = new Map(), + config?: GhoulsConfig, ): Array<{ branch: LocalBranch; safetyCheck: SafetyCheckResult; matchingPR?: PullRequest }> { return branches.map(branch => { const matchingPR = mergedPRs.get(branch.name); - const safetyCheck = isBranchSafeToDelete(branch, currentBranch, matchingPR); + const safetyCheck = isBranchSafeToDelete(branch, currentBranch, matchingPR, config); return { branch, diff --git a/src/utils/configLoader.test.ts b/src/utils/configLoader.test.ts new file mode 100644 index 0000000..a259e6e --- /dev/null +++ b/src/utils/configLoader.test.ts @@ -0,0 +1,375 @@ +import { findUpSync } from "find-up"; +import { existsSync, readFileSync } from "fs"; +import { homedir } from "os"; +import { dirname, join, resolve } from "path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { GhoulsConfig } from "../config/GhoulsConfig.js"; +import { ConfigLoadError, getConfigFilePaths, loadConfig, loadConfigSafe } from "./configLoader.js"; + +// Mock filesystem operations +vi.mock("fs"); +vi.mock("path"); +vi.mock("os"); +vi.mock("find-up"); + +const mockedExistsSync = vi.mocked(existsSync); +const mockedReadFileSync = vi.mocked(readFileSync); +const mockedResolve = vi.mocked(resolve); +const mockedJoin = vi.mocked(join); +const mockedDirname = vi.mocked(dirname); +const mockedHomedir = vi.mocked(homedir); +const mockedFindUpSync = vi.mocked(findUpSync); + +describe("configLoader", () => { + beforeEach(() => { + vi.clearAllMocks(); + + // Setup default mock behaviors + mockedResolve.mockImplementation((path) => `/resolved/${path}`); + mockedJoin.mockImplementation((...paths) => paths.join("/")); + mockedDirname.mockImplementation((path) => { + // Simple implementation for our test paths + if (path === "/current/dir/.git") return "/current/dir"; + return path.split("/").slice(0, -1).join("/"); + }); + mockedHomedir.mockReturnValue("/home/user"); + mockedFindUpSync.mockReturnValue(undefined); // Default: no git found + + // Mock process.cwd() + vi.spyOn(process, "cwd").mockReturnValue("/current/dir"); + }); + + afterEach(() => { + vi.restoreAllMocks(); + delete process.env.GHOULS_CONFIG; + }); + + describe("loadConfig", () => { + it("should return empty config when no config files exist", () => { + mockedExistsSync.mockReturnValue(false); + + const result = loadConfig(); + expect(result).toEqual({}); + }); + + it("should load config from environment variable", () => { + process.env.GHOULS_CONFIG = "/custom/config.json"; + + const mockConfig: GhoulsConfig = { + protectedBranches: ["main", "custom"], + }; + + mockedExistsSync.mockImplementation((path) => path === "/resolved//custom/config.json"); + mockedReadFileSync.mockImplementation((path) => { + if (path === "/resolved//custom/config.json") { + return JSON.stringify(mockConfig); + } + throw new Error("File not found"); + }); + + const result = loadConfig(); + expect(result).toEqual(mockConfig); + }); + + it("should load config from git repository root", () => { + // Mock find-up to find .git directory + mockedFindUpSync.mockReturnValue("/current/dir/.git"); + + mockedExistsSync.mockImplementation((path) => { + if (path === "/current/dir/.config/ghouls.json") return true; + return false; + }); + + const mockConfig: GhoulsConfig = { + protectedBranches: ["main", "develop"], + }; + + mockedReadFileSync.mockImplementation((path) => { + if (path === "/current/dir/.config/ghouls.json") { + return JSON.stringify(mockConfig); + } + throw new Error("File not found"); + }); + + const result = loadConfig(); + expect(result).toEqual(mockConfig); + }); + + it("should load config from user home directory", () => { + mockedExistsSync.mockImplementation((path) => { + if (path === "/home/user/.config/ghouls/config.json") return true; + return false; + }); + + const mockConfig: GhoulsConfig = {}; + + mockedReadFileSync.mockReturnValue(JSON.stringify(mockConfig)); + + const result = loadConfig(); + expect(result).toEqual(mockConfig); + }); + + it("should merge multiple config files with precedence", () => { + const envConfig: GhoulsConfig = { + protectedBranches: ["main", "env-branch"], + }; + + const repoConfig: GhoulsConfig = { + protectedBranches: ["main", "repo-branch"], // Should be overridden by env + }; + + process.env.GHOULS_CONFIG = "/env/config.json"; + + // Mock find-up to find .git directory + mockedFindUpSync.mockReturnValue("/current/dir/.git"); + + mockedExistsSync.mockImplementation((path) => { + if (path === "/resolved//env/config.json") return true; + if (path === "/current/dir/.config/ghouls.json") return true; + return false; + }); + + mockedReadFileSync.mockImplementation((path) => { + if (path === "/resolved//env/config.json") { + return JSON.stringify(envConfig); + } + if (path === "/current/dir/.config/ghouls.json") { + return JSON.stringify(repoConfig); + } + throw new Error("File not found"); + }); + + const result = loadConfig(); + + // Environment config should take precedence + expect(result).toEqual({ + protectedBranches: ["main", "env-branch"], // From env config + }); + }); + + it("should throw ConfigLoadError for invalid JSON", () => { + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue("invalid json {"); + + expect(() => loadConfig()).toThrow(ConfigLoadError); + expect(() => loadConfig()).toThrow("Invalid JSON"); + }); + + it("should throw ConfigLoadError for config validation failures", () => { + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(JSON.stringify({ + protectedBranches: "invalid-type", + })); + + expect(() => loadConfig()).toThrow(ConfigLoadError); + expect(() => loadConfig()).toThrow("Configuration validation failed"); + }); + + it("should throw ConfigLoadError with detailed validation errors", () => { + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(JSON.stringify({ + protectedBranches: ["", "valid"], + })); + + try { + loadConfig(); + expect.fail("Should have thrown ConfigLoadError"); + } catch (error) { + expect(error).toBeInstanceOf(ConfigLoadError); + const configError = error as ConfigLoadError; + expect(configError.validationErrors).toBeDefined(); + expect(configError.validationErrors?.length).toBeGreaterThan(0); + expect(configError.message).toContain( + "Configuration validation failed", + ); + } + }); + + it("should throw ConfigLoadError for file read errors", () => { + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockImplementation(() => { + throw new Error("Permission denied"); + }); + + expect(() => loadConfig()).toThrow(ConfigLoadError); + expect(() => loadConfig()).toThrow("Permission denied"); + }); + + it("should handle empty configs", () => { + const emptyConfig: GhoulsConfig = {}; + + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(JSON.stringify(emptyConfig)); + + const result = loadConfig(); + expect(result).toEqual({}); + }); + }); + + describe("loadConfigSafe", () => { + it("should return undefined when no config found", () => { + mockedExistsSync.mockReturnValue(false); + + const result = loadConfigSafe(); + expect(result).toBeUndefined(); + }); + + it("should return undefined on config load error", () => { + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockImplementation(() => { + throw new Error("File error"); + }); + + const result = loadConfigSafe(); + expect(result).toBeUndefined(); + }); + + it("should return config when loading succeeds", () => { + const mockConfig: GhoulsConfig = { + protectedBranches: ["main"], + }; + + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(JSON.stringify(mockConfig)); + + const result = loadConfigSafe(); + expect(result).toEqual(mockConfig); + }); + + it("should log errors when logErrors is true", () => { + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockImplementation(() => { + throw new Error("Test error"); + }); + + const result = loadConfigSafe(true); + + expect(result).toBeUndefined(); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("Warning: Failed to load configuration"), + ); + + consoleSpy.mockRestore(); + }); + + it("should log validation errors when logErrors is true", () => { + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(JSON.stringify({ + protectedBranches: 123, + })); + + const result = loadConfigSafe(true); + + expect(result).toBeUndefined(); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("Configuration validation failed"), + ); + + consoleSpy.mockRestore(); + }); + + it("should not log errors when logErrors is false", () => { + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockImplementation(() => { + throw new Error("Test error"); + }); + + const result = loadConfigSafe(false); + + expect(result).toBeUndefined(); + expect(consoleSpy).not.toHaveBeenCalled(); + + consoleSpy.mockRestore(); + }); + }); + + describe("getConfigFilePaths", () => { + it("should return config file paths with existence status", () => { + process.env.GHOULS_CONFIG = "/env/config.json"; + + // Mock find-up to find .git directory + mockedFindUpSync.mockReturnValue("/current/dir/.git"); + + mockedExistsSync.mockImplementation((path) => { + if (path === "/resolved//env/config.json") return true; + if (path === "/current/dir/.config/ghouls.json") return true; + return false; + }); + + mockedReadFileSync.mockImplementation((path) => { + if (path === "/resolved//env/config.json") { + return "{\"protectedBranches\": [\"main\"]}"; + } + if (path === "/current/dir/.config/ghouls.json") { + return "invalid json"; + } + throw new Error("File not found"); + }); + + const result = getConfigFilePaths(); + + expect(result).toEqual( + expect.arrayContaining([ + { path: "/resolved//env/config.json", exists: true, loaded: true }, + { + path: "/current/dir/.config/ghouls.json", + exists: true, + loaded: false, + error: expect.stringContaining("Invalid JSON"), + }, + ]), + ); + }); + + it("should handle non-existent files", () => { + mockedExistsSync.mockReturnValue(false); + mockedFindUpSync.mockReturnValue(undefined); + + const result = getConfigFilePaths(); + + result.forEach(entry => { + expect(entry.exists).toBe(false); + expect(entry.loaded).toBe(false); + expect(entry.error).toBeUndefined(); + }); + }); + }); + + describe("ConfigLoadError", () => { + it("should create error with message and path", () => { + const error = new ConfigLoadError("Test message", "/test/path"); + + expect(error.message).toBe("Test message"); + expect(error.path).toBe("/test/path"); + expect(error.name).toBe("ConfigLoadError"); + expect(error.cause).toBeUndefined(); + }); + + it("should create error with cause", () => { + const cause = new Error("Original error"); + const error = new ConfigLoadError("Test message", "/test/path", cause); + + expect(error.cause).toBe(cause); + }); + + it("should create error with validation errors", () => { + const validationErrors = ["Error 1", "Error 2"]; + const error = new ConfigLoadError( + "Test message", + "/test/path", + undefined, + validationErrors, + ); + + expect(error.validationErrors).toEqual(validationErrors); + expect(error.message).toBe("Test message"); + expect(error.path).toBe("/test/path"); + }); + }); +}); diff --git a/src/utils/configLoader.ts b/src/utils/configLoader.ts new file mode 100644 index 0000000..8e536b1 --- /dev/null +++ b/src/utils/configLoader.ts @@ -0,0 +1,208 @@ +/** + * Configuration file discovery and loading for Ghouls + */ + +import { findUpSync } from "find-up"; +import { existsSync, readFileSync } from "fs"; +import { homedir } from "os"; +import { dirname, join, resolve } from "path"; +import { formatZodErrors } from "../config/formatZodErrors.js"; +import { CONFIG_FILE_NAMES, mergeConfigs } from "../config/getEffectiveConfig.js"; +import { GhoulsConfig } from "../config/GhoulsConfig.js"; + +/** + * Configuration loading error + */ +export class ConfigLoadError extends Error { + constructor( + message: string, + public readonly path: string, + public readonly cause?: Error, + public readonly validationErrors?: string[], + ) { + super(message); + this.name = "ConfigLoadError"; + } +} + +/** + * Find git repository root by looking for .git directory + */ +function findGitRoot(startPath: string = process.cwd()): string | null { + const gitDir = findUpSync(".git", { cwd: startPath, type: "directory" }); + return gitDir ? dirname(gitDir) : null; +} + +/** + * Load configuration from a JSON file with Zod validation + */ +function loadConfigFile(configPath: string): GhoulsConfig { + try { + const content = readFileSync(configPath, "utf8"); + let parsedJson: unknown; + + try { + parsedJson = JSON.parse(content); + } catch (jsonError) { + throw new ConfigLoadError( + `Invalid JSON in configuration file: ${jsonError instanceof Error ? jsonError.message : String(jsonError)}`, + configPath, + jsonError instanceof Error ? jsonError : undefined, + ); + } + + // Validate with Zod + const validationResult = GhoulsConfig.safeParse(parsedJson); + + if (!validationResult.success) { + const errors = formatZodErrors(validationResult.error); + throw new ConfigLoadError( + `Configuration validation failed: ${errors.join(", ")}`, + configPath, + undefined, + errors, + ); + } + + return validationResult.data; + } catch (error) { + if (error instanceof ConfigLoadError) { + throw error; + } + throw new ConfigLoadError( + `Failed to load configuration: ${error instanceof Error ? error.message : String(error)}`, + configPath, + error instanceof Error ? error : undefined, + ); + } +} + +/** + * Find configuration files in order of precedence + */ +function findConfigFiles(): string[] { + const configPaths: string[] = []; + + // 1. Environment variable + if (process.env.GHOULS_CONFIG) { + configPaths.push(resolve(process.env.GHOULS_CONFIG)); + } + + // 2. Repository-level config files (in git root) + const gitRoot = findGitRoot(); + if (gitRoot) { + for (const fileName of CONFIG_FILE_NAMES) { + configPaths.push(join(gitRoot, fileName)); + } + } + + // 3. User-level config + const userConfigDir = join(homedir(), ".config", "ghouls"); + configPaths.push(join(userConfigDir, "config.json")); + + // 4. Current directory (fallback) + for (const fileName of CONFIG_FILE_NAMES) { + configPaths.push(resolve(fileName)); + } + + return configPaths; +} + +/** + * Load all available configuration files and merge them + */ +export function loadConfig(): GhoulsConfig { + const configPaths = findConfigFiles(); + const loadedConfigs: GhoulsConfig[] = []; + const errors: ConfigLoadError[] = []; + + for (const configPath of configPaths) { + if (!existsSync(configPath)) { + continue; + } + + try { + const config = loadConfigFile(configPath); + loadedConfigs.push(config); + } catch (error) { + if (error instanceof ConfigLoadError) { + errors.push(error); + } else { + errors.push( + new ConfigLoadError(`Unexpected error loading config: ${String(error)}`, configPath), + ); + } + } + } + + // If we have errors but no successful configs, throw the most relevant error + if (errors.length > 0 && loadedConfigs.length === 0) { + const firstError = errors[0]; + // If it's a validation error, provide more context + if (firstError.validationErrors && firstError.validationErrors.length > 0) { + throw new ConfigLoadError( + `Configuration validation failed in ${firstError.path}:\n${ + firstError.validationErrors.map(e => ` - ${e}`).join("\n") + }`, + firstError.path, + firstError.cause, + firstError.validationErrors, + ); + } + throw firstError; + } + + // Merge all loaded configs (first config has highest precedence) + return mergeConfigs(...loadedConfigs); +} + +/** + * Load configuration synchronously with error handling + * Returns undefined if no config found or on error (with optional error logging) + */ +export function loadConfigSafe( + logErrors: boolean = false, +): GhoulsConfig | undefined { + try { + const config = loadConfig(); + return Object.keys(config).length > 0 ? config : undefined; + } catch (error) { + if (logErrors && error instanceof ConfigLoadError) { + if (error.validationErrors && error.validationErrors.length > 0) { + console.warn(`Warning: Configuration validation failed in ${error.path}:`); + error.validationErrors.forEach(validationError => { + console.warn(` - ${validationError}`); + }); + } else { + console.warn(`Warning: Failed to load configuration from ${error.path}: ${error.message}`); + } + } + return undefined; + } +} + +/** + * Get discovered configuration file paths for debugging + */ +export function getConfigFilePaths(): Array< + { path: string; exists: boolean; loaded?: boolean; error?: string } +> { + const configPaths = findConfigFiles(); + + return configPaths.map(path => { + const exists = existsSync(path); + let loaded = false; + let error: string | undefined; + + if (exists) { + try { + loadConfigFile(path); + loaded = true; + } catch (err) { + error = err instanceof Error ? err.message : String(err); + } + } + + return { path, exists, loaded, error }; + }); +}