Skip to content

Conversation

barisatalay
Copy link
Contributor

@barisatalay barisatalay commented Sep 20, 2025

This pull request adds support for the Firebender agent to the project, allowing Ruler to generate and manage configuration for Firebender via a new adapter. It updates documentation, the agent registry, and includes comprehensive unit tests to ensure correct behavior for rule processing, MCP server configuration, and file operations.

Firebender: https://firebender.com/
Docs: https://docs.firebender.com/context/configurations

Firebender agent support:

  • Introduced a new FirebenderAgent class in src/agents/FirebenderAgent.ts that implements the IAgent interface. This agent generates and updates firebender.json with rules and MCP server configuration, supports merging or overwriting MCP servers, and handles both plain text and file-based rules.
  • Registered the new agent in the agent index (src/agents/index.ts) by importing and adding FirebenderAgent to the allAgents array. [1] [2]

Documentation updates:

  • Updated README.md to document Firebender as a supported agent, including its output file, usage in --agents, and example commands for applying rules to Firebender. [1] [2] [3] [4]

Testing and robustness:

  • Added a comprehensive test suite for FirebenderAgent in tests/unit/agents/FirebenderAgent.test.ts, covering rule creation, merging, deduplication, MCP configuration strategies, file operations, and error handling.

Type safety improvements:

  • Refactored MCP propagation code for type safety by introducing an OpenHandsConfig interface in src/mcp/propagateOpenHandsMcp.ts and using it for config parsing. [1] [2]

Implementation

Identifier: firebender
Output file: firebender.json (in project root)
MCP support: Enabled (in firebender.json)

Usage

Users can now target the Firebender agent specifically:

# Apply rules only to Warp
ruler apply --agents firebender

# Use with other agents
ruler apply --agents firebender,claude

The agent can also be configured in ruler.toml:

default_agents = ["firebender"]

# Firebender agent configuration
[agents.firebender]
enabled = true

[agents.firebender]
enabled = true

[agents.firebender.mcp]
enabled = true
merge_strategy = "overwrite"  # or "merge"

Introduces the FirebenderAgent implementation, updates agent registry, and documents Firebender usage in the README. Includes unit tests for FirebenderAgent and minor type fix in propagateOpenHandsMcp.
Introduces handling of MCP server configuration in FirebenderAgent, allowing merging or overwriting of MCP servers in firebender.json based on strategy. Updates support flags for MCP, adds related methods, and provides comprehensive unit tests for MCP configuration scenarios.
Introduced explicit TypeScript interfaces for Firebender and OpenHands configuration structures to improve type safety and code clarity. Updated method signatures and internal logic in FirebenderAgent and propagateOpenHandsMcp to use these new types.
@intellectronica
Copy link
Owner

@barisatalay why the change to propagateOpenHandsMcp.ts? Doesn't look relevant to this feature.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds Firebender agent support, enabling Ruler to generate and manage firebender.json, including rules and MCP server configuration. Key changes:

  • New FirebenderAgent with rule processing, MCP merge/overwrite strategies, and file operations (backups, custom paths).
  • Registration of FirebenderAgent and documentation updates to include it in CLI options and usage.
  • Type-safety refactor in propagateOpenHandsMcp via an OpenHandsConfig interface.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/agents/FirebenderAgent.ts Implements FirebenderAgent: rules parsing, deduplication, MCP server merge/overwrite, backups, output path resolution.
tests/unit/agents/FirebenderAgent.test.ts Unit tests covering rules (plain and file-based), deduplication, MCP strategies, backups, and malformed JSON handling.
src/agents/index.ts Registers FirebenderAgent in the agent registry.
src/mcp/propagateOpenHandsMcp.ts Introduces OpenHandsConfig and uses it for config parsing.
README.md Documents Firebender agent, CLI usage, and availability in --agents and revert lists.

@intellectronica
Copy link
Owner

@barisatalay also why so much new code in FirebenderAgent.ts‎? Surely you could inherit a lot of that from AbstractAgent, no? Or am I missing something?

@intellectronica
Copy link
Owner

@barisatalay also see the comments from Copilot, they are all valid (except for one that I resolved).

refactor: rollback propagateOpenHandsMcp development
@barisatalay
Copy link
Contributor Author

@barisatalay why the change to propagateOpenHandsMcp.ts? Doesn't look relevant to this feature.

You’re right, that was my mistake — I rolled it back.

@barisatalay also see the comments from Copilot, they are all valid (except for one that I resolved).

Thanks for highlighting these, I’ll go ahead and fix them 👍

@barisatalay also why so much new code in FirebenderAgent.ts‎? Surely you could inherit a lot of that from AbstractAgent, no? Or am I missing something?

Actually "AbstractAgent" is really good but in "Firebender" rule definition and mcp definition system is a little more complicated than other tools.

For example: All definitions in one

{
  "rules": [
    "string rule",
    {
      "filePathMatches": "glob/pattern",
      "rules": ["rule1", "rule2"]
    }
  ],
  "ignore": [
    "glob/pattern1",
    "glob/pattern2"
  ],
  "mcpServers": {
    "serverName": {
      "command": "executable",
      "args": ["arg1", "arg2"],
      "env": {"KEY": "VALUE"}
    }
  },
  "mcpEnvFile": "path/to/env/file"
}

Thanks for pointing this out — there might be things I’ve overlooked, and your feedback is really valuable 🙏

Updated the Firebender configuration file path from `.firebender.json` to `firebender.json` in the documentation for accuracy.
Previously, object rules were deduplicated only by rulesPaths, causing
valid distinct rules with different filePathMatches to be incorrectly
removed. Now uses `filePathMatches::rulesPaths` composite key.

Includes test case for the fix.
…adFile

Use async I/O with ENOENT handling instead of existsSync/readFileSync
for better consistency and non-blocking operations.
@barisatalay barisatalay requested a review from Copilot September 20, 2025 20:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Prevent malicious HTML comments from accessing files outside project directory via path traversal attacks.
Replace unsafe type assertion with type guard function
Add isFirebenderRule() for safe FirebenderRule validation
Addresses PR review feedback on type checking
@barisatalay barisatalay requested a review from Copilot September 20, 2025 20:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

@barisatalay barisatalay requested a review from Copilot September 20, 2025 20:36
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

@intellectronica intellectronica merged commit f7eb4b1 into intellectronica:main Sep 20, 2025
1 check passed
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