Skip to content

fix: critical MCP server bugs for Hermes Agent compatibility#2

Closed
htafolla wants to merge 4 commits into
masterfrom
fix/mcp-server-bugs-hermes-compatibility
Closed

fix: critical MCP server bugs for Hermes Agent compatibility#2
htafolla wants to merge 4 commits into
masterfrom
fix/mcp-server-bugs-hermes-compatibility

Conversation

@htafolla

Copy link
Copy Markdown
Owner

Summary

Comprehensive bug fixes across all 10 MCP servers identified through deep testing with Hermes Agent (54 functional tests). These fixes ensure proper function when tools are invoked by AI agents.

Testing

  • 54 functional tests across all 10 servers — all passing
  • Deep scenario testing with real-world inputs (CRUD cycles, code analysis, security scanning, orchestration workflows)
  • Source code analysis of ~5400 lines of TypeScript across 12 files

Critical Fixes

Server Bug Impact
auto-format, lint, security-scan Command injection via execSync string interpolation Security vulnerability — malicious filenames could execute arbitrary commands
architect-tools Division by zero in calculateStructureMetrics and mapDependencies Returns NaN on empty projects, breaking agent analysis
architect-tools Unhandled fs.statSync crash on race condition Tool throws if file deleted between readdir/stat
7 servers Missing top-level try/catch in CallToolRequest handlers Unhandled exceptions kill the MCP connection, requiring restart

High Priority Fixes

Server Bug Impact
enforcer-tools Infinite recursion — method name collision with import Stack overflow on every rule_validation call
auto-format stdio: "inherit" corrupts MCP protocol stream Prettier output corrupts JSON-RPC transport
security-scan npm audit data loss — does not parse vulnerability JSON CVE data silently discarded
enforcer-tools Hardcoded overallScore: 85 Agent receives misleading quality scores
architect-tools, enforcer, researcher Duplicate SIGINT/SIGTERM handlers Double cleanup execution per signal

Medium Priority Fixes

  • execFileSync timeouts (30s) on all shell commands
  • ESLint 9 flat config support (eslint.config.js/mjs)
  • Researcher now searches all project directories (not just src/)
  • Security-scan package manager auto-detection (pnpm/yarn/npm)
  • Security pattern false positive reduction (exclude tests/comments)
  • State manager error propagation on write failure
  • Framework-help data consistency (agent count, codex terms)
  • Orchestrator version alignment (1.10.0 to 1.14.1)
  • Orchestrator stop() now actually closes the server
  • Estimation null-safety for args parameter

Files Changed

src/mcps/architect-tools.server.ts   | 43 +++---
src/mcps/auto-format.server.ts       | 65 +++++---
src/mcps/enforcer-tools.server.ts    | 24 +--
src/mcps/estimation.server.ts        | 49 +++---
src/mcps/framework-help.server.ts    | 49 +++---
src/mcps/lint.server.ts              | 131 ++++++++----
src/mcps/orchestrator/server.ts      | 90 +++++---
src/mcps/researcher.server.ts        | 105 +++++---
src/mcps/security-scan.server.ts     | 291 ++++++++++++++++++++++---
src/mcps/state-manager.server.ts     | 57 +++---
10 files changed, 614 insertions(+), 290 deletions(-)

Comprehensive bug fixes across all 10 MCP servers identified through
deep testing with Hermes Agent. Fixes ensure proper function when
tools are invoked by AI agents.

## Critical Fixes
- **Command injection**: Replaced execSync string interpolation with
  execFileSync args arrays in auto-format, lint, and security-scan
- **Division by zero**: Guarded all division operations in
  architect-tools (file counts, dependency averages)
- **Unhandled sync stat crashes**: Wrapped fs.statSync in try/catch
  to handle race conditions (file deleted between readdir and stat)
- **MCP connection crashes**: Added top-level try/catch in
  CallToolRequest handlers across 7 servers to prevent unhandled
  exceptions from killing the MCP connection

## High Priority Fixes
- **Infinite recursion**: Renamed enforcer-tools import to avoid
  method name collision causing stack overflow
- **stdio:inherit corrupting MCP protocol**: Changed auto-format
  Prettier to always use stdio:'pipe'
- **npm audit data loss**: Parse error.stdout for JSON vulnerability
  data when npm audit exits non-zero
- **Hardcoded quality scores**: Compute enforcer overallScore from
  actual sub-check results instead of constant 85
- **Duplicate signal handlers**: Removed duplicate SIGINT/SIGTERM
  registrations in architect-tools, enforcer, researcher

## Medium Priority Fixes
- **execSync timeouts**: Added 30s timeout to all execFileSync calls
- **ESLint 9 flat config**: Added eslint.config.js/mjs detection
- **Researcher search scope**: Now searches all project dirs, not
  just src/
- **Package manager auto-detection**: security-scan now detects
  pnpm/yarn/npm based on lockfile presence
- **Security scan false positives**: Exclude test files and comments
  from vulnerability pattern matching
- **State manager error propagation**: saveState now throws on write
  failure instead of silently succeeding
- **Framework-help inconsistencies**: Fixed agent count (8->26),
  codex term count, added default case for unknown command types
- **Orchestrator version**: Updated 1.10.0->1.14.1, fixed stop()
  to actually close the server
- **Estimation null safety**: Added null guard for args parameter

## Low Priority Fixes
- Removed void keyword from frameworkLogger.log calls
- Double JSON.stringify in state-manager validation
- findDependentKeys now matches quoted keys only
- Unbounded timer cleanup in researcher checkParent
- File scan depth limit (10) in security-scan

Tested: 54 functional tests across all 10 servers, all passing.
10 files changed, 614 insertions(+), 290 deletions(-)
Use optional chaining (?.[1]) instead of truthy check on match array
before indexing. TypeScript strict mode does not narrow RegExpMatchArray
index access even after null check.
htafolla added a commit that referenced this pull request Mar 27, 2026
- Add Hermes Agent skill (src/skills/hermes-agent/SKILL.md)
  - 257-line skill covering all CLI commands
  - MCP server integration guide with config.yaml
  - Quick decision matrix and pitfalls

- Add standalone mode (--standalone flag)
  - Install MCP servers without OpenCode dependency
  - For use with Hermes Agent without OpenCode

- MCP server security fixes (from PR #2)
  - execSync → execFileSync (command injection prevention)
  - Add try/catch around tool handlers
  - Add timeouts (30s) to exec calls
  - Remove SIGINT/SIGTERM handlers (Hermes manages)
  - fs.statSync error handling

- Update codex term counts (43→55, 46→60)
- Update agent counts (8→26)

- Add processor pipeline MCP server test
- Remove invalid routing pipeline test
@htafolla

Copy link
Copy Markdown
Owner Author

Closing in favor of a fresh PR with the same fix

@htafolla htafolla closed this Mar 27, 2026
htafolla added a commit that referenced this pull request Mar 28, 2026
Bug #1: Test logs not being deleted
- Added afterEach cleanup to remove test-activity-*.log files
- Added afterEach cleanup to remove test-calibration-*.log files
- These test artifacts were created by unit tests but never cleaned up
- Located in src/__tests__/setup.ts

Bug #2: activity.log not being archived
- Removed 'logs/framework/activity.log' from cleanupLogFiles excludePatterns
- This was preventing proper log rotation and archival
- Located in src/postprocessor/triggers/GitHookTrigger.ts line 474

Impact:
- Test files will now be properly cleaned after tests run
- activity.log will now be properly rotated and archived instead of truncated
- Prevents future data loss from log rotation bugs
@htafolla htafolla deleted the fix/mcp-server-bugs-hermes-compatibility branch March 31, 2026 15:40
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.

1 participant