hermes: register_skill fix + CLI parity + canonical per-profile hooks + pack deploy

[grimmjoww]

why this follow-up exists

#40 got the Hermes Tier A adapter onto main. This PR fixes the install and update paths around it.

The adapter worked once it was loaded, but sage init --platform hermes could not reliably reach it. hermes was missing from several platform switches in bin/sage, the community generator resolved its repo root incorrectly on Windows, and the old install path guessed one Hermes home even though plugins and hooks live per profile.

There were two quieter bugs too. The plugin passed a string to ctx.register_skill() even though Hermes expects a Path, so those registrations failed. sage update also left normal managed plugin copies stale after the framework itself had updated.

what changed

• bin/sage now treats hermes as a real platform during init, update, --platform all, interactive selection, worktree setup, and the get-started output.
• generate-hermes.sh builds SOUL.md from Sage's shared emitters, discovers Hermes profiles, lets an interactive user choose which profiles get Sage, and installs to every discovered profile in non-interactive mode.
• Each selected profile gets its own plugin copy, gate scripts, and idempotent config.yaml hook registrations.
• The Windows handoffs normalize paths before they cross the MSYS Bash/native Python boundary.
• sage update now refreshes normal managed plugin copies. It refuses to overwrite a Git-managed checkout when its contents differ, because an update should not destroy somebody's branch.
• skill_manager.py can deploy and undeploy packs on Hermes's native skill surface.
• register_skill() now receives a Path.

The profile chooser is intentionally a numbered prompt rather than a TUI widget. In automation or any other non-TTY run, the installer provisions all discovered profiles.

proof I reran

I reran the current branch from a clean worktree at 8c2e696:

• Hermes CLI/init/update tests: 14 passed
• plugin build and audit tests: 21 passed
• Windows Bash/path/config probe: 11 passed
• Hermes gate probe: 16 passed
• Bash syntax check: clean

I also ran the profile chooser through a real PTY with two fake profiles, selected only alpha, and asserted that beta stayed untouched:

PASS selected=alpha version=1.3.17 siblings_untouched=true hook_installed=true

GitHub's required checks are green. The community conformance matrix entry is skipped because Hermes is a community platform; it is not a failed check.

relation to #40

#40 is already merged at 9fa8aa0. This branch's merge base is that exact commit, so #41 contains #40 and adds nine follow-up commits. It is zero commits behind main, GitHub reports the merge state as CLEAN, and there is no second reconciliation branch to merge.

note from the #40 review

The review on #40 pointed out that the Hermes allowlist and SKILLS_NOT_IN_PLUGIN could drift. This branch carries a regression check for that relationship. sage-autoresearch remains a pip package rather than a shipped plugin skill, so the intended invariant is _HERMES_SKILLS ⊆ SKILLS_NOT_IN_PLUGIN, not strict equality.

[grimmjoww]

Two commits landed since the last update (1bf0cd5 → 97cf216). Both fix Windows failure modes found by live-firing the installed hooks, not by reading the code.

5838838 — shell-hook exit=127: hooks were decorative on Windows

Hermes spawns hook commands via shlex.split + shell=False. On Windows, CreateProcess searches System32 before PATH, so a bare bash argv[0] resolves to WSL's System32 bash.exe. That bash cannot read G:/-style script paths, so every registered hook exited 127 and failed open. The gates looked installed. They never ran.

Two fixes:

1. SAGE_BASH_EXE: the generator resolves the bash actually running it to an absolute Windows path at install time (cygpath -w of command -v bash, forward-slashed) and uses it as argv[0]. Nothing machine-specific is written into the config; hosts without cygpath fall back to bare bash.
2. In-place rewrite of broken entries. Dedup now searches the whole config text, because in folded configs the adapter+script pattern spans two lines and a per-line search can never match (that was the root cause of a silent updated=0 on the first fix attempt). The rewriter walks back from the match to the command: line, and if that line uses the bare-bash form it rewrites the whole folded block. The block terminator derives from the command line's own indent, so timeout: 30, which sits at the same indent, survives. A hardcoded shallow threshold would have eaten it.

New probe: develop/conformance/probes/test_hermes_bash_fix_probe.py (199 lines). It asserts 11 live entries, not 3, rewrites the broken blocks on run 1 (added=8 updated=3), no-ops on run 2 (added=0 updated=0), keeps all 11 timeouts, and asserts the merged config still parses with yaml.safe_load. Wire-spawn check: the broken form returns 127, the fixed absolute-bash form returns 0.

97cf216 — second failure, found only after the first was fixed: gates still failed open

With the 127 gone, the hooks were reachable but still allowed everything. Every gate script writes its decision logic to an mktemp file and runs python3 "$TMPFILE". Two Windows problems stacked: native python3 cannot open /tmp/... (that is the MSYS2 tree, not a Windows path), and MSYS argument conversion rewrote the path to G:\tmp\... on the way through. Result: internal error in every gate, and the fail-open design did its job, silently.

Fixed at the adapter boundary so the gate scripts stay the single canonical copy of decision logic: before running a gate, the adapter exports TMPDIR pointed at a Windows-native temp dir and sets MSYS_NO_PATHCONV=1 so paths arrive untouched. No-op on hosts without cygpath. Proof is a live-fire of the exact registered command shape: secrets-gate now returns a block JSON for an sk-ant payload and clean output for a benign edit.

Portability check (per review request)

No hardcoded paths, homes, or usernames in any shipped file. HERMES_HOME env override with $HOME/.hermes fallback, profile enumeration with default mapped to the home root, and the generated command shape was verified to survive shlex.split with spaces in the path (C:/Program Files/... parses to exactly 3 argv parts). Gates remain opt-in per project via .sage/config.yaml; installing into one project never enforces anything outside it.

[grimmjoww]

quick follow-up after the last push: I found two more install/update problems and fixed both.

72fa482 finishes the CLI wiring. hermes is now reachable from init, update, --platform all, the interactive menu, worktree setup, and the saved platform flow.

8c2e696 fixes stale managed installs. sage update refreshes a normal managed plugin copy, but refuses to overwrite a divergent Git checkout.

I reran the branch from a clean worktree:

• CLI/init/update: 14 passed
• plugin build/audit: 21 passed
• Windows path/config probe: 11 passed
• Hermes gates: 16 passed
• interactive PTY profile test: selected alpha; beta stayed untouched

I also checked the relationship to #40 rather than assuming it. #40 is merged at 9fa8aa0, and that is the exact merge base for this branch. #41 is 0 behind and 9 ahead, with a CLEAN merge state. Nothing needs to be reopened or double-merged.

I rewrote the PR description so it matches the current branch and the checks above. ready for review.

[grimmjoww]

@xoai, when you have a minute, #41 is ready for review. #40 is already merged, and this branch starts from that merge commit, so there isn't another branch to reconcile. This one covers the remaining CLI and per-profile install/update fixes. CI is green, and the description has the latest Windows and PTY test receipts. thanks.

[xoai]

Review — approve after rebase; strong follow-up, one merge conflict, four non-blocking findings

First: thank you — this is careful work, and two things in it deserve explicit credit. The backtick escaping in the config.yaml heredoc is a real pre-existing bug fix, not cosmetics: that heredoc is unquoted, so the bare backticks were live command substitutions silently mangling every generated config (I verified the heredoc is now fully escaped — zero unescaped backticks remain in its 73 lines). And the gate adapter's copy-our-hooks-per-profile design means the claude-code gate hardening that landed on main this week flows to Hermes automatically, with no forked decision logic. That's the right architecture.

The rebase, and the one conflict you'll hit. This branch is now 10 commits behind main — v1.3.18 (the parallel-lanes pack) landed after you opened this, so the "zero commits behind, CLEAN" note in the description is stale. There's exactly one conflict: develop/validators/tools/test_build_plugin.py. Main's 10b89f2 independently fixed the same #40 collision bug your test_build_omits_excluded_skills rewrite fixes. Your version is tighter (it asserts the canonical replacement ships and encodes overlay-before-system priority; main's only asserts absence-or-byte-match), so the resolution is: keep your test body, and also keep main's OVERLAY_SKILL_NAMES class attribute + test_collision_names_ship_the_canonical_source_not_the_mirror companion — they compose. I ran exactly that composition on a trial merge: all 23 tests pass, and the full battery on the merged tree is green (fastcheck 26/26, hook suite 166/166, generation-smoke 8/8 including your hermes case, coverage registry clean at 129 surfaces).

Findings (all reproduced; none blocking, descending severity):

1. develop/conformance/probes/test_hermes_bash_fix_probe.py exits 1 on POSIX hosts. It derives a Windows bash path (C:/Program Files/Git/bin/bash.exe) and crashes with FileNotFoundError on Linux — no skip guard anywhere in the file. It isn't wired into CI, but anyone running the probes directory on a POSIX box gets a red test that looks like a real failure. A unittest.skipUnless(shutil.which("cygpath"), "Windows/MSYS-only probe") at the class would fix it. Would appreciate this in the same push as the rebase.
2. The git-checkout refusal exit 1s mid-loop (generate-hermes.sh, the [ -e "$DEST_PLUGIN/.git" ] branch): one git-managed profile aborts the whole multi-profile install, leaving later profiles unprovisioned and that profile half-done (its hooks are already installed by step 1). Suggest warn-and-continue with a nonzero exit at the end summarizing which profiles were skipped.
3. PR description vs. code on the refusal condition: the body says it refuses "when its contents differ"; the code refuses on any .git presence, unconditionally. The code is the safer behavior — I'd just amend the description to match it rather than weaken the code.
4. Plugin refresh merge-copies without pruning: cp -r "$PLUGIN_STAGE"/. "$DEST_PLUGIN"/ never removes files deleted upstream, so a skill or gate removed from Sage lingers in every profile across updates — the stale-artifact class the repo's own Gate-4 history is about. An rm -rf of the destination before copy (you already stage, so the window is small), or a post-copy sweep of files absent from the stage, would close it. Fine as a follow-up if you'd rather keep this PR's scope.

Two nits, take or leave: the Hermes home-resolution block in skill_manager.py is duplicated verbatim between deploy and undeploy (a small helper would do), and build_plugin.py's comment still cites runtime/platforms/community/hermes/__init__.py for the plugin entry point that deliberately lives at the repo root.

What I verified beyond the diff: your claimed suites reproduce at the PR head (init tests OK, plugin tests OK, hermes gate probe 16/16); bin/sage covers every platform switch (init, update, all, interactive, worktree, help, get-started) with the empty-array bash-3.2 idiom intact and parses clean under bash -n; the root __init__.py placement matches the documented Hermes packaging model; the _HERMES_SKILLS ⊆ SKILLS_NOT_IN_PLUGIN regression check does what the #40 review asked.

Happy to hand you the composed test_build_plugin.py resolution if useful. After the rebase + finding 1, this is an approve from me.

[xoai]

As promised — the composed test_build_plugin.py resolution for the rebase, verified against the actual merge with current main (22 tests OK, no duplicate method names, build_plugin.py --check clean at 1.3.18).

Resolving the conflict, in two steps:

1. For test_build_omits_excluded_skills, take your side (the canonical-first version with the sources tuple) verbatim — it supersedes main's split absence-check. Take main's side for nothing in that method.

2. Add the following immediately before def test_gate_scripts_are_identical_to_their_sources — this is main's 10b89f2 companion tripwire, with one adjustment for your PR (noted below):

    OVERLAY_SKILL_NAMES = frozenset(
        p.name for p in (REPO_ROOT / "runtime" / "plugin-overlay"
                         / "skills").iterdir()
        if (p / "SKILL.md").is_file())

    def test_collision_names_ship_the_canonical_source_not_the_mirror(self):
        """The Gate-4 duplication lesson as a tripwire: for names that
        exist both as hermes mirrors (skills/) and as system skills or
        overlay skills, the plugin must ship the CANONICAL source — if
        an edit lands there and the plugin starts shipping the stale
        mirror, this fails. (Overlay outranks system-skills, matching
        the canonical-resolution order in test_build_omits_excluded_
        skills.)"""
        out = self._build()
        for name in (build_plugin.SKILLS_NOT_IN_PLUGIN
                     & build_plugin.SYSTEM_SKILL_NAMES
                     - self.OVERLAY_SKILL_NAMES):
            shipped = (out / "skills" / name / "SKILL.md").read_bytes()
            system = (REPO_ROOT / "core" / "system-skills" / name
                      / "SKILL.md").read_bytes()
            self.assertEqual(shipped, system,
                             f"{name}: shipped bytes are not the "
                             f"system-skill source")
        for name in (build_plugin.SKILLS_NOT_IN_PLUGIN
                     & self.OVERLAY_SKILL_NAMES):
            shipped = (out / "skills" / name / "SKILL.md").read_bytes()
            overlay = (REPO_ROOT / "runtime" / "plugin-overlay" / "skills"
                       / name / "SKILL.md").read_bytes()
            self.assertEqual(shipped, overlay,
                             f"{name}: shipped bytes are not the "
                             f"overlay source")

The one adjustment vs. main's version: the system-skill loop now subtracts OVERLAY_SKILL_NAMES (& SYSTEM_SKILL_NAMES - self.OVERLAY_SKILL_NAMES), so for a name that ever exists in both overlay and system-skills, the tripwire compares against the overlay — matching the overlay-before-system priority your sources tuple encodes. Main's original would have compared such a name against system-skills and disagreed with your test. The intersection is empty today, so both pass now; this keeps them agreeing if it ever isn't.

Two composition traps I hit so you don't have to: concatenating both conflict sides compiles fine but leaves two methods named test_build_omits_excluded_skills, and Python silently runs only the second — the suite stays green while one version stops executing. Worth an eyeball (grep -c "def test_build_omits_excluded_skills" should say 1) after resolving. And your version + this companion are redundant where they overlap, deliberately — yours asserts the canonical file ships, this one pins which bytes; they fail on different mutations.