Skip to content

fix: improve config validation error handling#174

Open
Zebastjan wants to merge 1 commit intosynthetic-lab:mainfrom
Zebastjan:fix/config-validation-clean
Open

fix: improve config validation error handling#174
Zebastjan wants to merge 1 commit intosynthetic-lab:mainfrom
Zebastjan:fix/config-validation-clean

Conversation

@Zebastjan
Copy link
Copy Markdown
Contributor

Supersedes #171 - this is a clean version without the unintended batch tool files.

Problem

When OctoFriend encounters configuration errors (e.g., invalid MCP server config or JSON5 syntax errors), it crashes with an unhelpful error message:

TypeError: [object Object] failed the following checks:
[archon]: undefined
at Err.toError (/usr/lib/node_modules/octofriend/node_modules/structural/build/lib/result.js:16:16)
...

Solution

This PR adds comprehensive error handling that:

  • Catches and explains JSON5 parsing errors
  • Detects invalid MCP server configurations and provides specific guidance
  • Exits gracefully with helpful error messages and examples
  • Suggests remediation steps (e.g., octofriend init to start fresh)

Changes

  • source/config.ts: Added try-catch blocks for JSON5 parsing and schema validation with detailed error messages
  • source/cli.tsx: Added graceful error handling in runMain() to exit cleanly on config errors

Test plan

  • Tested with invalid MCP server configuration (missing command field)
  • Tested with JSON5 syntax errors (missing commas)
  • Verified error messages are clear and helpful
  • Build passes successfully
  • Pre-commit hooks pass

When configuration validation fails (e.g., invalid MCP server config or
JSON5 syntax errors), OctoFriend now provides clear, actionable error
messages instead of crashing with a cryptic stack trace.

Changes:
- Add comprehensive error handling in readConfig() for JSON5 parsing errors
- Detect and provide specific guidance for MCP server configuration issues
- Add graceful error handling in runMain() to exit cleanly on config errors
- Include helpful examples and suggestions in error messages

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@sdjidjev sdjidjev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having more clear error messaging, especially as we add more configurability!
Just a note about how to we can clean the MCP servers validation logic

mcpServers && typeof mcpServers === "object" && Object.keys(mcpServers).length > 0;

if (
hasMcpServers &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this'll be a lot cleaner if we just do:

    const mcpServers = raw["mcpServers"];
    if (mcpServers && typeof mcpServers === "object") {
      const invalidServers: string[] = [];
      for (const [name, serverConfig] of Object.entries(mcpServers)) {
        try {
          McpServerConfigSchema.slice(serverConfig);
        } catch {
          invalidServers.push(name);
        }
      }
      if (invalidServers.length > 0) {
        throw new Error(
          `Invalid MCP server configuration in ${filePath}: ${invalidServers.join(", ")}\n\n` +
            `Each MCP server must have:\n` +
            `  - command: string\n` +
            `  - args: string[] (optional)\n` +
            `  - env: object (optional)`,
        );
      }
    }

    throw new Error(
      `Configuration file validation failed: ${filePath}\n\n` +
        `Error: ${errorMessage}\n\n` +
        `You can reset your config by deleting: ${filePath}`,
    );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants