feat: enhance Hermes Agent support by rudironsoni · Pull Request #2026 · dyoshikawa/rulesync

rudironsoni · 2026-06-23T16:19:17Z

Summary

Add Hermes Agent support across rules, skills, MCP, commands, hooks, permissions, and subagents.
Generate native Hermes subagent plugin files and JSON specs using delegate_task dispatch.
Preserve shared .hermes/config.yaml content when multiple Hermes features contribute config.
Update supported-tool docs and feature matrix tests.

Verification

pnpm cicheck:code
pnpm cicheck:content
Hermes Agent all-feature E2E generation into temp HOME and HERMES_HOME
Verified generated .hermes/config.yaml retains hooks and rulesync-subagents
Verified generated plugin uses delegate_task dispatch

dyoshikawa

Thanks for the broad push to bring Hermes up to feature parity across rules, skills, commands, hooks, permissions, subagents and MCP. The MCP networkTimeout round-trip and its tests look solid. That said, I think a few things need to be sorted out before this can merge.

The biggest one is the permissions allowlist (commented inline) — as written it never produces a usable command_allowlist, and there are no unit tests for the new permissions/hooks/command/skill classes, so this and similar issues slip through. Could you add unit tests for the new Hermes classes? The rules-processor meta change also seems to contradict what HermesagentRule actually emits (inline).

A couple of cross-cutting points worth addressing too:

The new Hermes features (commands/skills/subagents/hooks/permissions) aren't added to the e2e Tool×Feature happy-path specs (src/e2e/e2e-*.spec.ts). The project guidelines ask us to always keep those matrix cases in place, so please add hermesagent cases there.
src/types/feature-processor.ts hardcodes .hermes/config.yaml in the generic base class and matches it with endsWith(".hermes/config.yaml"). Since getFilePath() builds paths with path.join, this won't match on Windows (.hermes\config.yaml), and tool-specific strings in the base class break the abstraction. Better to model "is this file mergeable" as a hook on the tool class.
HermesagentSubagent extends AiFile directly rather than ToolSubagent, which forces the double as unknown as cast in the factory registration. Worth aligning with the other subagent classes or widening the factory type properly.
.github/copilot-instructions.md lost its TOON rules-reference header (inline) — looks like an unintended side effect of local regeneration.

Smaller stuff: hermesagent-command.ts hardcodes the SKILL.md description instead of using the rulesync command's description, and its toRulesyncCommand() embeds the generated frontmatter into the command body which breaks the import round-trip; and the import { join } at the end of hermesagent-paths.ts should move to the top. Happy to dig into any of these further.

dyoshikawa

Thanks for the follow-up round — the core blockers from the last review are resolved. The permissions allowlist now extracts correctly and has unit tests, the rules-processor meta matches what HermesagentRule emits, and there are new tests for permissions/hooks/mcp/subagents. CI is fully green too.

A couple of things still need attention before this can merge, though.

The big one is the e2e Tool×Feature happy-path matrix. The new Hermes commands/skills/hooks/permissions/subagents features aren't represented in src/e2e/e2e-*.spec.ts (only rules and mcp have hermesagent cases). The project guidelines require always keeping those matrix cases in place, so please add hermesagent cases for the new features.

The Windows path check in feature-processor.ts is a real cross-platform bug (inline). Beyond that there are a few mid-level items on the command import round-trip and the subagent inheritance that I left inline, plus the generic merge stage added to generate.ts for what is really a Hermes-specific concern. Happy to talk through any of these.

dyoshikawa · 2026-06-24T11:58:38Z

+  return [
+    "---",
+    `name: ${slug}`,
+    `description: ${slug} command`,


The description is hardcoded to ${slug} command instead of using the rulesync command's description frontmatter. Also rulesyncCommand.getFileContent() below includes the rulesync frontmatter in the body — you likely want getBody() so the generated SKILL.md doesn't embed the source frontmatter.

Fixed in 01026e4. Hermes command generation now uses the Rulesync command description and body, so the generated SKILL.md does not embed the source frontmatter. I added focused coverage for this path.

dyoshikawa

Nice turnaround — most of the last round is sorted. The feature-processor path check is now a polymorphic shouldMergeExistingFileContent() hook (no more Windows-breaking endsWith or hardcoded tool string), the command class uses the real description frontmatter and getBody() with a proper round-trip, HermesagentSubagent extends ToolSubagent so the double casts are gone, and the generic merge stage in generate.ts was removed. The shared hermes-config.ts parser also got the prototype-pollution hardening (isPlainObject + recursive __proto__/constructor stripping), which clears the security note from before.

One blocker still stands: the e2e Tool×Feature happy-path matrix. The five new features (commands, skills, hooks, permissions, subagents) still have no hermesagent case — only e2e-rules.spec.ts and e2e-mcp.spec.ts cover it. The project guidelines treat keeping those matrix cases in place as mandatory, so please add hermesagent rows to src/e2e/e2e-commands.spec.ts, e2e-skills.spec.ts, e2e-hooks.spec.ts, e2e-permissions.spec.ts, and e2e-subagents.spec.ts (each is a one-line addition to the existing it.each table). That's the one thing holding up the merge.

The rest are small cleanups I left inline — the MCP/shared-config DRY divergence is the most worth doing, the others are minor. Thanks for sticking with this.

dyoshikawa · 2026-06-25T01:06:36Z

+    );
+
+    this.config = merged;
+    super.setFileContent(dump(merged));


This serializes with raw dump(merged), while the shared hermes-config.ts (used by hooks/permissions/subagents) goes through stringifyHermesConfig with { sortKeys: false } + .trimEnd() + "\n". Since all of these features write to the same ~/.hermes/config.yaml, the differing formatting can cause non-idempotent diff churn. The MCP adapter also keeps its own private parseHermesConfig. Worth consolidating onto hermes-config.ts so the serialization matches and the parser logic isn't duplicated.

dyoshikawa · 2026-06-25T01:06:36Z

    return processEmptyFeatureGeneration({ config, processor, skipFilePaths });
  }
-  const toolFiles = await processor.convertRulesyncFilesToToolFiles(rulesyncFiles);
+  let toolFiles = await processor.convertRulesyncFilesToToolFiles(rulesyncFiles);


toolFiles is no longer reassigned now that the merge stage is gone — this can go back to const.

dyoshikawa · 2026-06-25T01:06:36Z

+
+      if (
+        existingFileContent !== null &&
+        "shouldMergeExistingFileContent" in aiFile &&


shouldMergeExistingFileContent is declared on the AiFile base class now, so the "..." in aiFile && typeof ... === "function" guard is redundant — aiFile.shouldMergeExistingFileContent() alone is enough and keeps the type contract explicit rather than duck-typed.

dyoshikawa · 2026-06-25T01:06:36Z

 /** MCP servers and other settings live in `config.yaml` under `~/.hermes/`. */
 export const HERMESAGENT_MCP_FILE_NAME = "config.yaml";
+export const HERMESAGENT_SOUL_FILE_NAME = "SOUL.md";
+export const HERMESAGENT_CONFIG_FILE_NAME = "config.yaml";


HERMESAGENT_CONFIG_FILE_NAME and HERMESAGENT_MCP_FILE_NAME are both "config.yaml". Two constants for the same value invites drift — consider collapsing to one.

dyoshikawa · 2026-06-25T01:06:36Z

+
+  constructor({ slug, ...params }: HermesagentCommandParams) {
+    const resolvedSlug =
+      slug ?? basename(dirname(params.relativeDirPath)) ?? commandSlug(params.relativeFilePath);


basename(dirname(...)) never returns nullish, so the ?? commandSlug(...) fallback is dead code. Minor, but worth trimming for clarity.

dyoshikawa · 2026-06-25T01:34:08Z

@rudironsoni Thank you!

…eanups Follow-up from PR dyoshikawa#2026: - Add `hermesagent` happy-path rows to the five e2e Tool x Feature specs (commands/skills/hooks/permissions/subagents). Hermes is global-only, so the cases live in each spec's global-mode block; hooks/permissions assert on the shared ~/.hermes/config.yaml. - Consolidate `hermesagent-mcp.ts` onto the shared `hermes-config.ts`: drop the private `parseHermesConfig` and `dump(...)` calls in favor of the shared `parseHermesConfig` / `stringifyHermesConfig`, so all features that write ~/.hermes/config.yaml serialize identically (sortKeys:false + trailing newline) and avoid idempotency churn. - Collapse the duplicate `HERMESAGENT_MCP_FILE_NAME` / `HERMESAGENT_CONFIG_FILE_NAME` constants into a single `HERMESAGENT_CONFIG_FILE_NAME` and update references. - Add a unit test for `HermesagentSkill`. - Remove the dead `?? commandSlug(...)` fallback in `hermesagent-command.ts` (`basename` never returns nullish). - Revert `let toolFiles` back to `const` in `generate.ts` (never reassigned). - Drop the redundant `"shouldMergeExistingFileContent" in aiFile && typeof ...` guard in `feature-processor.ts`; the method is declared on the `AiFile` base class. Update the test mock accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rudironsoni mentioned this pull request Jun 23, 2026

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

Closed

dyoshikawa reviewed Jun 24, 2026

View reviewed changes

Comment thread .github/copilot-instructions.md

Comment thread src/features/permissions/hermesagent-permissions.ts Outdated

Comment thread src/features/rules/rules-processor.ts Outdated

rudironsoni force-pushed the feature/enhance-hermesagent-support branch from d7479af to 359cb6d Compare June 24, 2026 10:28

rudironsoni requested a review from dyoshikawa June 24, 2026 10:41

dyoshikawa reviewed Jun 24, 2026

View reviewed changes

rudironsoni added 6 commits June 24, 2026 22:17

feat: enhance hermes agent support

9c2c5d9

fix: address hermes e2e failures

1333034

chore: update generated ignore rules

208db67

chore: format hermes support fixes

e3f43af

fix: preserve hermes mcp network timeout

24fb5d9

fix: address hermes review feedback

0fecd2e

rudironsoni force-pushed the feature/enhance-hermesagent-support branch from 359cb6d to 0fecd2e Compare June 24, 2026 20:17

fix: address hermes review suggestions

01026e4

rudironsoni requested a review from dyoshikawa June 24, 2026 22:18

dyoshikawa reviewed Jun 25, 2026

View reviewed changes

dyoshikawa mentioned this pull request Jun 25, 2026

Follow-up from PR #2026: Hermes Agent e2e matrix coverage, MCP config DRY, and cleanups #2044

Closed

dyoshikawa merged commit f6de0d4 into dyoshikawa:main Jun 25, 2026
5 checks passed

dyoshikawa mentioned this pull request Jun 25, 2026

chore(hermesagent): e2e matrix coverage, MCP config DRY, and cleanups #2047

Merged

Uh oh!

Conversation

rudironsoni commented Jun 23, 2026

Summary

Verification

Uh oh!

dyoshikawa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dyoshikawa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dyoshikawa Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

rudironsoni Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dyoshikawa left a comment

Choose a reason for hiding this comment

Uh oh!

dyoshikawa Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

dyoshikawa Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

dyoshikawa Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

dyoshikawa Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

dyoshikawa Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dyoshikawa commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants