PR #2019 follow-ups: Hermes Agent MCP/Rules minor improvements

[dyoshikawa-claw]

Background

PR #2019 ("feat(hermesagent): add Hermes Agent support for rules and MCP") was reviewed and merged. The implementation is solid and follows existing patterns (Goose for MCP, GrokCLI for rules). Several low-severity findings were identified during review that could be addressed as follow-up improvements.

Details

Code Quality

1. YAML round-trip in fromRulesyncMcp (src/features/mcp/hermesagent-mcp.ts): The method builds a merged JS object, calls dump() to serialize to YAML, then the constructor calls load() to parse it back. Consider a direct constructor path to avoid the serialize-then-deserialize cycle. (Same pattern exists in Goose.)

2. Missing inline comment for _relativeFilePath (src/features/rules/hermesagent-rule.ts): The fromFile method ignores relativeFilePath (prefixed with _) but lacks an inline comment explaining why, unlike the class-level JSDoc.

3. Missing path constant for ~/.hermes/SOUL.md (src/constants/hermesagent-paths.ts): The file header mentions this path, but no constant is defined for it. Adding it now would make the file a complete source of truth.

4. Non-alphabetical e2e matrix insertion (src/e2e/e2e-rules.spec.ts, src/e2e/e2e-mcp.spec.ts): Hermes Agent entries are inserted at inconsistent alphabetical positions. Consider sorting the existing entries while adding the new ones.

5. Hermes-specific timeout passthrough: No timeout/networkTimeout passthrough unlike Goose. If Hermes adds timeout support, this would be a gap.

Security Hardening

6. isRecord vs isPlainObject in parseHermesConfig (src/features/mcp/hermesagent-mcp.ts): isRecord() accepts class instances while isPlainObject() is preferred for prototype-pollution hardening per the codebase guidelines. (Same pattern exists in Goose — consider a cross-cutting fix.)

7. env/headers prototype pollution keys not filtered: While server names are filtered via PROTOTYPE_POLLUTION_KEYS, nested env/headers object keys are not. Low practical risk since js-yaml v4 load() is safe and spread syntax handles __proto__ as a normal property. (Same pattern exists in Goose.)

Solution / Next Steps

• Items 1–5 are low-priority polish. Address at leisure.
• Items 6–7 could be addressed as a cross-cutting improvement across all MCP tool implementations (Goose, Hermes, etc.).
• No urgency — none of these affect correctness or security in practice.

[rudironsoni]

Opened PR #2026 to build on the partial Hermes Agent support and cover the remaining Rulesync features: #2026

[dyoshikawa]

Note: item 5 (Hermes-specific timeout/networkTimeout passthrough) was already implemented in PR #2023 — convertServerToHermes/convertFromHermesFormat now carry timeout through both directions (with the networkTimeout alias resolved on export), mirroring Goose. The remaining items (1–4 polish, 6–7 cross-cutting prototype-pollution/isPlainObject hardening shared with Goose) are low-priority as noted; items 6–7 in particular are best done as a single cross-cutting MCP-adapter pass rather than Hermes-only, so leaving this open for that.

[rudironsoni]

Done in PR #2026.

We also handled the follow-up from #2020 (comment). Hermes MCP conversion now works both ways: Rulesync networkTimeout becomes Hermes timeout, and Hermes timeout converts back to Rulesync networkTimeout when reading the config.

I added focused coverage for that round trip. The latest PR head is green: local pnpm cicheck, Code Quality & Tests, Build binaries, and E2E on Ubuntu, macOS, and Windows.