From 743525d7e29dc5284a812f7702aa7873e21a698d Mon Sep 17 00:00:00 2001 From: Nicholas Hart Date: Thu, 31 Jul 2025 20:03:17 -0700 Subject: [PATCH] improved the config parsing to handle selectively overriding defaults --- package.json | 2 + pnpm-lock.yaml | 16 ++++ src/core/config-discovery.ts | 69 ++++++++++++-- src/core/workflow-engine.ts | 7 +- src/shared/mermaid-processor.ts | 12 ++- tests/unit/core/config-layering.test.ts | 99 +++++++++++++++++++++ tests/unit/shared/mermaid-processor.test.ts | 5 +- workflows/default-config.yml | 93 +++++++++++++++++++ 8 files changed, 288 insertions(+), 15 deletions(-) create mode 100644 tests/unit/core/config-layering.test.ts create mode 100644 workflows/default-config.yml diff --git a/package.json b/package.json index 350f175..ad3f214 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,7 @@ "@mermaid-js/mermaid-cli": "^11.9.0", "commander": "^14.0.0", "fs-extra": "^11.3.0", + "lodash": "^4.17.21", "mustache": "^4.2.0", "next": "15.4.2", "react": "19.1.0", @@ -54,6 +55,7 @@ "@tailwindcss/postcss": "^4.1.11", "@types/fs-extra": "^11.0.4", "@types/jest": "^30.0.0", + "@types/lodash": "^4.17.20", "@types/mustache": "^4.2.6", "@types/node": "^20.19.9", "@types/react": "^19.1.8", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 804da95..78d5a28 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -17,6 +17,9 @@ importers: fs-extra: specifier: ^11.3.0 version: 11.3.0 + lodash: + specifier: ^4.17.21 + version: 4.17.21 mustache: specifier: ^4.2.0 version: 4.2.0 @@ -48,6 +51,9 @@ importers: '@types/jest': specifier: ^30.0.0 version: 30.0.0 + '@types/lodash': + specifier: ^4.17.20 + version: 4.17.20 '@types/mustache': specifier: ^4.2.6 version: 4.2.6 @@ -1218,6 +1224,9 @@ packages: '@types/katex@0.16.7': resolution: {integrity: sha512-HMwFiRujE5PjrgwHQ25+bsLJgowjGjm5Z8FVSf0N6PwgJrwxH0QxzHYDcKsTfV3wva0vzrpqMTJS2jXPr5BMEQ==} + '@types/lodash@4.17.20': + resolution: {integrity: sha512-H3MHACvFUEiujabxhaI/ImO6gUrd8oOurg7LQtS7mbwIXA/cUqWrvBsaeJ23aZEPk1TAYkurjfMbSELfoCXlGA==} + '@types/ms@2.1.0': resolution: {integrity: sha512-GsCCIZDE/p3i96vtEqx+7dBUGXrc7zeSK3wwPHIaRThS+9OhWIXRqzs4d6k1SVU8g91DrNRWxWUGhp5KXQb2VA==} @@ -3176,6 +3185,9 @@ packages: lodash.merge@4.6.2: resolution: {integrity: sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==} + lodash@4.17.21: + resolution: {integrity: sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==} + loose-envify@1.4.0: resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==} hasBin: true @@ -5602,6 +5614,8 @@ snapshots: '@types/katex@0.16.7': {} + '@types/lodash@4.17.20': {} + '@types/ms@2.1.0': {} '@types/mustache@4.2.6': {} @@ -7975,6 +7989,8 @@ snapshots: lodash.merge@4.6.2: {} + lodash@4.17.21: {} + loose-envify@1.4.0: dependencies: js-tokens: 4.0.0 diff --git a/src/core/config-discovery.ts b/src/core/config-discovery.ts index 7b165d2..9d2981b 100644 --- a/src/core/config-discovery.ts +++ b/src/core/config-discovery.ts @@ -1,5 +1,6 @@ import * as path from 'path'; import * as YAML from 'yaml'; +import _ from 'lodash'; import { ConfigPaths, ResolvedConfig } from './types.js'; import { ProjectConfigSchema, type ProjectConfig } from './schemas.js'; import { SystemInterface, NodeSystemInterface } from './system-interface.js'; @@ -118,20 +119,76 @@ export class ConfigDiscovery { } /** - * Load and parse project configuration with Zod validation - * Reads config.yml from the project and validates its structure + * Load system default configuration + * Reads default-config.yml from the workflows directory */ - async loadProjectConfig(configPath: string): Promise { + private async loadSystemDefaults(systemRoot: string): Promise { try { - if (!this.systemInterface.existsSync(configPath)) { + const defaultConfigPath = path.join(systemRoot, 'workflows', 'default-config.yml'); + + if (!this.systemInterface.existsSync(defaultConfigPath)) { + console.warn('System default config not found, using empty defaults'); return null; } - const configContent = this.systemInterface.readFileSync(configPath); + const configContent = this.systemInterface.readFileSync(defaultConfigPath); const parsedYaml = YAML.parse(configContent); const validationResult = ProjectConfigSchema.safeParse(parsedYaml); + if (!validationResult.success) { + console.warn(`Invalid system default config: ${validationResult.error.message}`); + return null; + } + + return validationResult.data; + } catch (error) { + console.warn( + `Error loading system defaults: ${error instanceof Error ? error.message : String(error)}`, + ); + return null; + } + } + + /** + * Load and parse project configuration with Zod validation + * Reads config.yml from the project, merges with system defaults, and validates structure + */ + async loadProjectConfig(configPath: string, systemRoot?: string): Promise { + try { + // Load system defaults first + let systemDefaults: ProjectConfig | null = null; + if (systemRoot) { + systemDefaults = await this.loadSystemDefaults(systemRoot); + } + + // Load user config if it exists + let userConfig: unknown = null; + if (this.systemInterface.existsSync(configPath)) { + const configContent = this.systemInterface.readFileSync(configPath); + userConfig = YAML.parse(configContent); + } + + // Merge user config over system defaults + let mergedConfig: unknown; + if (systemDefaults && userConfig) { + // User config takes precedence, system defaults fill in missing values + mergedConfig = _.defaultsDeep({}, userConfig, systemDefaults); + console.log('šŸ”§ Config merged: user config + system defaults'); + } else if (userConfig) { + mergedConfig = userConfig; + console.log('šŸ”§ Config loaded: user config only (no system defaults)'); + } else if (systemDefaults) { + mergedConfig = systemDefaults; + console.log('šŸ”§ Config loaded: system defaults only (no user config)'); + } else { + console.log('šŸ”§ No configuration found'); + return null; + } + + // Validate the merged configuration + const validationResult = ProjectConfigSchema.safeParse(mergedConfig); + if (!validationResult.success) { throw new Error(`Invalid configuration format: ${validationResult.error.message}`); } @@ -177,7 +234,7 @@ export class ConfigDiscovery { let projectConfig: ProjectConfig | null = null; if (paths.projectConfig) { - projectConfig = await this.loadProjectConfig(paths.projectConfig); + projectConfig = await this.loadProjectConfig(paths.projectConfig, paths.systemRoot); } return { diff --git a/src/core/workflow-engine.ts b/src/core/workflow-engine.ts index 13472ba..d762e2b 100644 --- a/src/core/workflow-engine.ts +++ b/src/core/workflow-engine.ts @@ -374,11 +374,8 @@ export class WorkflowEngine { console.log(`\nāš ļø NO TEMPLATE TYPE DETECTED - using default pandoc styling\n`); } - // Get Mermaid configuration from project config - let mermaidConfig = undefined; - if (this.projectConfig?.system?.mermaid) { - mermaidConfig = this.projectConfig.system.mermaid; - } + // Get Mermaid configuration from project config (with system defaults already merged) + const mermaidConfig = this.projectConfig?.system?.mermaid; const result = await convertDocument({ inputFile: inputPath, diff --git a/src/shared/mermaid-processor.ts b/src/shared/mermaid-processor.ts index 7be211f..db8ec6b 100644 --- a/src/shared/mermaid-processor.ts +++ b/src/shared/mermaid-processor.ts @@ -54,9 +54,17 @@ export class MermaidProcessor { */ static async detectMermaidCLI(): Promise { try { - execSync('npx @mermaid-js/mermaid-cli --version', { stdio: 'ignore', timeout: 5000 }); + // Try to run the Mermaid CLI version command + execSync('npx @mermaid-js/mermaid-cli --version', { + stdio: 'pipe', + timeout: 10000, + encoding: 'utf8', + }); return true; - } catch { + } catch (error) { + console.warn( + `āš ļø Mermaid CLI detection failed: ${error instanceof Error ? error.message : String(error)}`, + ); return false; } } diff --git a/tests/unit/core/config-layering.test.ts b/tests/unit/core/config-layering.test.ts new file mode 100644 index 0000000..dab43c2 --- /dev/null +++ b/tests/unit/core/config-layering.test.ts @@ -0,0 +1,99 @@ +/** + * Integration tests for configuration layering behavior + * Tests that system defaults are properly merged with user configuration + */ + +import { ConfigDiscovery } from '../../../src/core/config-discovery.js'; +import { MockSystemInterface } from '../mocks/mock-system-interface.js'; +import { describe, it, expect, beforeEach } from '@jest/globals'; +import _ from 'lodash'; + +describe('Configuration Layering Integration', () => { + let configDiscovery: ConfigDiscovery; + let mockSystemInterface: MockSystemInterface; + + beforeEach(() => { + mockSystemInterface = new MockSystemInterface(); + configDiscovery = new ConfigDiscovery(mockSystemInterface); + }); + + describe('loadProjectConfig', () => { + it('should return null when neither config exists', async () => { + // No files exist + const result = await configDiscovery.loadProjectConfig( + '/project/.markdown-workflow/config.yml', + '/system', + ); + + expect(result).toBeNull(); + }); + + it('should load system defaults when available', async () => { + // This test validates that the system defaults loading mechanism works + // without requiring full schema validation + const mockLoadSystemDefaults = jest.spyOn( + configDiscovery as unknown as { loadSystemDefaults: (path: string) => Promise }, + 'loadSystemDefaults', + ); + mockLoadSystemDefaults.mockResolvedValue(null); // Mock return value + + const result = await configDiscovery.loadProjectConfig( + '/project/.markdown-workflow/config.yml', + '/system', + ); + + expect(mockLoadSystemDefaults).toHaveBeenCalledWith('/system'); + expect(result).toBeNull(); // Since both configs are null/missing + }); + + it('should handle missing system root gracefully', async () => { + const result = await configDiscovery.loadProjectConfig( + '/project/.markdown-workflow/config.yml', + // No systemRoot provided + ); + + expect(result).toBeNull(); + }); + }); + + describe('Configuration merging behavior', () => { + it('should demonstrate lodash defaultsDeep behavior', () => { + // This test validates our merging logic works as expected + + const systemDefaults = { + system: { + mermaid: { + output_format: 'png', + theme: 'default', + timeout: 30, + }, + git: { + auto_commit: true, + }, + }, + user: { + name: 'Default User', + }, + }; + + const userConfig = { + system: { + mermaid: { + theme: 'dark', // User override + }, + }, + user: { + name: 'John Doe', // User override + }, + }; + + const merged = _.defaultsDeep({}, userConfig, systemDefaults); + + expect(merged.user.name).toBe('John Doe'); // User override + expect(merged.system.mermaid.theme).toBe('dark'); // User override + expect(merged.system.mermaid.output_format).toBe('png'); // System default + expect(merged.system.mermaid.timeout).toBe(30); // System default + expect(merged.system.git.auto_commit).toBe(true); // System default + }); + }); +}); diff --git a/tests/unit/shared/mermaid-processor.test.ts b/tests/unit/shared/mermaid-processor.test.ts index 5b35fce..97af5f5 100644 --- a/tests/unit/shared/mermaid-processor.test.ts +++ b/tests/unit/shared/mermaid-processor.test.ts @@ -203,8 +203,9 @@ No mermaid diagrams here. expect(mockExecSync).toHaveBeenCalledWith( 'npx @mermaid-js/mermaid-cli --version', expect.objectContaining({ - stdio: 'ignore', - timeout: 5000, + stdio: 'pipe', + timeout: 10000, + encoding: 'utf8', }), ); }); diff --git a/workflows/default-config.yml b/workflows/default-config.yml new file mode 100644 index 0000000..473b69f --- /dev/null +++ b/workflows/default-config.yml @@ -0,0 +1,93 @@ +# System Default Configuration +# This file contains default values for all system configuration options. +# User projects can override any of these values in their .markdown-workflow/config.yml + +# User Information Template (will be overridden by user config) +user: + name: "Your Name" + preferred_name: "Your Name" + email: "your.email@example.com" + phone: "(555) 123-4567" + address: "123 Main Street" + city: "Your City" + state: "ST" + zip: "12345" + linkedin: "linkedin.com/in/yourname" + github: "github.com/yourusername" + website: "yourwebsite.com" + +# System Configuration Defaults +system: + # Web scraping preferences + scraper: "wget" # Options: "wget", "curl", "chrome" + web_download: + timeout: 30 + add_utf8_bom: true + html_cleanup: "scripts" # Options: "none", "scripts", "markdown" + + # Supported output formats + output_formats: + - "docx" + - "html" + - "pdf" + - "pptx" + + # Git integration settings + git: + auto_commit: true + commit_message_template: "Add {{workflow}} collection: {{collection_id}}" + + # Collection ID generation settings + collection_id: + date_format: "YYYYMMDD" + sanitize_spaces: "_" + max_length: 50 + + # Mermaid diagram generation settings + mermaid: + output_format: "png" # Options: "png", "svg" + theme: "default" # Options: "default", "dark", "forest", "neutral" + timeout: 30 + + # Testing overrides (normally disabled) + testing: + # Uncomment to override current date for predictable testing + # override_current_date: "2025-01-21T10:00:00.000Z" + # override_timezone: "UTC" + # deterministic_ids: true + +# Workflow-specific default overrides +workflows: + job: + templates: + resume: + default_template: "default" + available_templates: + - "default" + - "mobile" + - "frontend" + + custom_fields: + - name: "salary_range" + type: "string" + description: "Expected salary range" + - name: "remote_preference" + type: "enum" + options: ["remote", "hybrid", "onsite"] + description: "Work location preference" + + blog: + custom_fields: + - name: "estimated_reading_time" + type: "number" + description: "Estimated reading time in minutes" + + presentation: + custom_fields: + - name: "presentation_duration" + type: "number" + description: "Expected presentation duration in minutes" + - name: "audience_type" + type: "enum" + options: ["technical", "business", "mixed", "academic"] + description: "Target audience type" \ No newline at end of file