diff --git a/.changeset/native-hermes-integration.md b/.changeset/native-hermes-integration.md new file mode 100644 index 0000000..ce4002c --- /dev/null +++ b/.changeset/native-hermes-integration.md @@ -0,0 +1,33 @@ +--- +'tenjin-cli': minor +--- + +Add a working native Hermes Agent integration. `tenjin install --harness hermes` +now installs the Tenjin skills, adds a conservative MCP entry, and enables a +stdlib-only Hermes plugin that checks Tenjin before `web_search`, attaches a hit +to that tool's result, and surfaces unresolved searches through +`transform_llm_output` for publish-back. + +Hermes reuses the same generated, bounded, fail-open retrieval/store/nag scripts +as Claude Code instead of carrying a second payment-facing implementation. The +installer honors an absolute `HERMES_HOME`, embeds absolute executable paths, +preserves unsupported or user-owned YAML byte-for-byte, never overrides +`plugins.disabled`, keeps automatic detection inert until explicit activation, +and adds a warn-level doctor check. It adds no `TENJIN_HARNESS` policy selector +and does not copy or couple wallet state. + +Hook consent and plugin activation are two separate decisions. `--no-hooks` and +`--search-hooks off` withhold the Hermes scripts, plugin, and activation exactly +as they withhold Claude's `settings.json` entries; the `mcp_servers.tenjin` entry +is a server registration, so it is still written. Where the stored +`hooks.searchMode` is what holds the plugin back, install and `tenjin doctor` say +`tenjin config set hooks.searchMode auto` rather than an install command that +cannot move the blocker. Withholding a write is not an uninstall, so install +reports it as `skipped` and names any enabled plugin an earlier run left behind. + +Re-pointing the MCP entry (an nvm switch, a pnpm-vs-npm global) rewrites the +managed block in place, leaving one marker comment and any neighbouring comments +untouched. `tenjin doctor` tolerates a relative `HERMES_HOME` set for some other +tool, reports a baked MCP command that no longer exists as stale rather than +green, and shares the installer's classifier so its fix cannot point into a +conflict it did not predict. diff --git a/README.md b/README.md index 285b75a..d67fd24 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ Tenjin is meant for questions that are public, durable, and annoying to reproduc This repository ships: - `tenjin`, the CLI published as [`tenjin-cli`](https://www.npmjs.com/package/tenjin-cli) -- Agent Skills for Claude Code, Codex, and other Agent-Skills-compatible harnesses +- Agent Skills for Claude Code, Codex, Hermes Agent, and other Agent-Skills-compatible harnesses - A local stdio MCP server backed by the same command core No API key or Tenjin account is required. Your wallet is the credential, and the private key stays on your machine. @@ -257,6 +257,13 @@ Cursor: } ``` +Hermes Agent: `tenjin install --harness hermes` writes the entry into +`~/.hermes/config.yaml` for you, alongside a native plugin that checks Tenjin +before `web_search` and raises unresolved searches at turn end. The plugin runs +the same scripts as Claude Code's hooks, so `--no-hooks` and +`hooks.searchMode off` withhold and disarm it the same way. Auto-detection +installs it inert; naming the harness is what enables it. + There is also a keyless remote MCP server: ```text diff --git a/src/cli.ts b/src/cli.ts index 1816725..8d5529e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -147,11 +147,11 @@ export function buildProgram(io: Io, setExit: (code: number) => void): Command { addGlobalFlags(program.command('install')) .description( - 'Detect installed harnesses (Claude Code, Codex), wire the Tenjin skills, then run the doctor checks last', + 'Detect installed harnesses (Claude Code, Codex, Hermes), wire Tenjin, then run doctor last', ) .option( '--harness ', - 'target a specific harness: claude | codex | shared (repeatable; overrides detection)', + 'target a specific harness: claude | codex | hermes | shared (repeatable; overrides detection)', collect, [], ) diff --git a/src/commands/config.ts b/src/commands/config.ts index 199933c..c43286b 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -12,6 +12,7 @@ import { } from '../lib/harness-permissions'; import { modeGatedPointer } from '../lib/permissions'; import { stopHookIsCurrent } from '../lib/harness-hooks'; +import { resolveHermesHomeLenient } from '../lib/hermes'; import { CONFIG_KEYS, HOOKS_CONFIG_KEYS, @@ -403,11 +404,13 @@ async function claudeInPlay( const requested = await loadRawConfig(ctx.dataDir) .then((c) => c.install?.harness ?? []) .catch(() => [] as HarnessTarget[]); + const hermesHome = resolveHermesHomeLenient(home, env).home; return harnessInPlay( home, - harnessTargetDir(home, 'claude'), - detectHarnesses(home, which), + harnessTargetDir(home, 'claude', hermesHome), + detectHarnesses(home, which, hermesHome), requested, + hermesHome, ); } diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts index 17f9bdb..8194573 100644 --- a/src/commands/doctor.test.ts +++ b/src/commands/doctor.test.ts @@ -19,6 +19,7 @@ import { saveSessionFile } from '../lib/session-key'; import { sessionPath } from '../lib/paths'; import { testSessionKey } from '../lib/read-test-utils'; import type { WalletProvider } from '../lib/wallet'; +import { wireHermesIntegration } from '../lib/hermes'; // doctor loads viem's balance read lazily; the mock keeps every test off-chain. vi.mock('../lib/usdc', () => ({ getUsdcBalance: vi.fn() })); @@ -129,6 +130,98 @@ async function writeWallet(mode: number): Promise { } describe('runDoctor — passing outcomes', () => { + it('reports a working native Hermes integration separately from portable skills', async () => { + await wireHermesIntegration({ + // A path that EXISTS: doctor now stats the baked command, because an + // `npx`/`dlx` cache path can be pruned out from under a green check. + hermesHome: join(skillHome, '.hermes'), + dataDir: dir, + tenjinCommand: process.execPath, + nodeCommand: process.execPath, + dryRun: false, + explicit: true, + hooks: { enabled: true, mode: 'auto' }, + }); + const res = await runDoctor(ctxFor(), { + walletPassphrase: NO_OS_STORE, + homeDir: skillHome, + skillsSourceDir: pkgSrc, + env: {}, + which: () => false, + fetchImpl: healthyFetch, + }); + const checks = (res.data as { checks: CheckResult[] }).checks; + expect(find(checks, 'hermes')).toMatchObject({ status: 'ok', required: false }); + expect(find(checks, 'hermes').detail).toContain('retrieval and publish-back'); + }); + + // Doctor is the command you reach for when something is already broken, so the + // STRICT resolver must never run here: a stray relative HERMES_HOME belonging to + // some other tool would return CONFIG_INVALID and run zero checks on a machine + // with no Hermes at all. + it('a relative HERMES_HOME warns and falls back instead of aborting every check', async () => { + const res = await runDoctor(ctxFor(), { + walletPassphrase: NO_OS_STORE, + homeDir: skillHome, + skillsSourceDir: pkgSrc, + env: { HERMES_HOME: 'relative/hermes' }, + which: () => false, + fetchImpl: healthyFetch, + }); + const checks = (res.data as { checks: CheckResult[] }).checks; + expect(checks.length).toBeGreaterThan(1); + expect(find(checks, 'node').status).toBe('ok'); + }); + + it('a baked MCP command that no longer exists warns instead of reading green', async () => { + const hermesHome = join(skillHome, '.hermes'); + await wireHermesIntegration({ + hermesHome, + dataDir: dir, + tenjinCommand: join(skillHome, 'pruned-npx-cache', 'tenjin'), + nodeCommand: process.execPath, + dryRun: false, + explicit: true, + hooks: { enabled: true, mode: 'auto' }, + }); + const res = await runDoctor(ctxFor(), { + walletPassphrase: NO_OS_STORE, + homeDir: skillHome, + skillsSourceDir: pkgSrc, + env: {}, + which: () => false, + fetchImpl: healthyFetch, + }); + const hermes = find((res.data as { checks: CheckResult[] }).checks, 'hermes'); + expect(hermes.status).toBe('warn'); + expect(hermes.detail).toContain('MCP command missing'); + // One subject per problem: prefixing the activation with `plugin` too reads as + // one subject named twice. + expect(hermes.detail).not.toContain('plugin plugin'); + }); + + // `tenjin install --harness hermes` alone is a dead end with the mode stored off: + // it re-runs, withholds the hook code by design, and prints the same warning + // forever. The `native-harness` fix string in this same PR already names the + // config command; doctor has to as well. + it('names the config command when the stored searchMode is what blocks the plugin', async () => { + await writeFile( + join(dir, 'config.json'), + JSON.stringify({ install: { harness: ['hermes'] }, hooks: { searchMode: 'off' } }), + ); + const res = await runDoctor(ctxFor(), { + walletPassphrase: NO_OS_STORE, + homeDir: skillHome, + skillsSourceDir: pkgSrc, + env: {}, + which: () => false, + fetchImpl: healthyFetch, + }); + const hermes = find((res.data as { checks: CheckResult[] }).checks, 'hermes'); + expect(hermes.status).toBe('warn'); + expect(hermes.fix).toContain('tenjin config set hooks.searchMode auto'); + }); + it('all required checks green, no wallet: status pass with a warn wallet check', async () => { const res = await runDoctor(ctxFor(), { walletPassphrase: NO_OS_STORE, diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 9210a01..693f0d2 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -15,6 +15,7 @@ import { anyTenjinSkill, cliSkillsWired, detectHarnesses, + harnessDetectedBy, harnessFlagFor, harnessInPlay, harnessReads, @@ -25,6 +26,7 @@ import { readSkillFile, shadowedCliSkills, } from '../lib/skill-wiring'; +import { readHermesIntegrationStatus, resolveHermesHomeLenient } from '../lib/hermes'; import type { DirState, HarnessTarget, @@ -42,7 +44,7 @@ import { isSessionPresentable, readSessionFile, scopeSatisfies } from '../lib/se import { sanitizeForTerminal } from '../lib/output'; import { modeGatedPointer, permissionsPointer, recommendedPermissions } from '../lib/permissions'; import { inspectFreeVerbRules, MODE_GATED_RULES } from '../lib/harness-permissions'; -import type { PartialConfig, PublishMode } from '../lib/config'; +import type { PartialConfig, PublishMode, SearchHookMode } from '../lib/config'; import type { ErrorCode } from '../schemas'; import type { Io } from '../lib/output'; import type { @@ -126,6 +128,8 @@ export interface DoctorDeps { now?: () => number; /** Packaged skills to compare the wired copies against; defaults to this build's. */ skillsSourceDir?: string; + /** Hermes home override; defaults through HERMES_HOME using the same resolver as install. */ + hermesHome?: string; /** * Passphrase seams for the wallet verification (#70), which reads the OS * credential store. Tests inject a platform with no store, or a stubbed exec, @@ -179,6 +183,17 @@ export async function collectDoctorChecks( project: project?.layer, }); const baseUrl = settings.baseUrl.value; + const home = deps.homeDir ?? homedir(); + const which = deps.which ?? ((bin: string) => onPath(bin, env)); + const requested = config.install?.harness ?? []; + // NEVER the strict resolver here. Doctor is the command you reach for when + // something is already broken, so a stray relative HERMES_HOME must not abort it + // before a single check runs: it warns on the Hermes check and falls back. + const hermesTarget = + deps.hermesHome === undefined + ? resolveHermesHomeLenient(home, env) + : { home: deps.hermesHome, warning: undefined }; + const hermesHome = hermesTarget.home; const built: BuiltCheck[] = [ checkNode(), @@ -187,15 +202,26 @@ export async function collectDoctorChecks( await checkReadPath(baseUrl, ctx.flags.timeout, deps.fetchImpl), await checkSearchContract(baseUrl, ctx.flags.timeout, deps.fetchImpl), await checkSkills( - deps.homeDir ?? homedir(), - deps.which ?? ((bin) => onPath(bin, env)), - config.install?.harness ?? [], + home, + which, + requested, settings.bazaarPay.value, deps.skillsSourceDir, + hermesHome, ), await checkSession(ctx.dataDir, deps.now ?? Date.now, tryOriginOf(baseUrl)), ]; + const hermes = await checkHermes({ + home, + hermesHome, + which, + requested, + searchMode: config.hooks?.searchMode, + homeWarning: hermesTarget.warning, + }); + if (hermes !== null) built.push(hermes); + // The wallet/custody/balance checks all come from the ACTIVE provider: it owns // describe() and diagnostics(), so doctor never runs its own fs/env probe. for (const result of await checkWallet(ctx, deps, env, settings.rpcUrl.value)) { @@ -454,15 +480,17 @@ async function checkSkills( which: (bin: string) => boolean, requested: readonly HarnessTarget[], bazaarPay: boolean, - skillsSourceDir?: string, + skillsSourceDir: string | undefined, + hermesHome: string, ): Promise { - const present = detectHarnesses(home, which); - const wiring = await readAllWiring(home); + const resolvedHermesHome = hermesHome; + const present = detectHarnesses(home, which, resolvedHermesHome); + const wiring = await readAllWiring(home, resolvedHermesHome); const data = { directories: wiring.map((w) => ({ ...w, - harnessPresent: harnessReads(home, w.dir, present), - requested: harnessRequested(home, w.dir, requested), + harnessPresent: harnessReads(home, w.dir, present, resolvedHermesHome), + requested: harnessRequested(home, w.dir, requested, resolvedHermesHome), })), }; const inPlay = wiring.filter((w) => anyTenjinSkill(w)); @@ -477,15 +505,15 @@ async function checkSkills( // nobody asked to see named. const targeted = requested.length > 0 - ? wiring.filter((w) => harnessInPlay(home, w.dir, present, requested)) + ? wiring.filter((w) => harnessInPlay(home, w.dir, present, requested, resolvedHermesHome)) : []; return { result: { name: 'skills', status: 'warn', required: false, - detail: `No Tenjin skills wired under ${home} (looked in .claude/skills and .agents/skills)`, - fix: targeted.length > 0 ? fixFor(home, targeted) : 'tenjin install', + detail: `No Tenjin skills wired under ${home} (looked in .claude/skills, .agents/skills, and Hermes skills)`, + fix: targeted.length > 0 ? fixFor(home, targeted, resolvedHermesHome) : 'tenjin install', data, }, }; @@ -495,7 +523,7 @@ async function checkSkills( // is the defect, whether it is shadowed, half-installed, hosted-only or absent; a // directory neither detected nor asked for is described but never warned about. const broken = wiring.filter( - (w) => harnessInPlay(home, w.dir, present, requested) && !cliSkillsWired(w), + (w) => harnessInPlay(home, w.dir, present, requested, resolvedHermesHome) && !cliSkillsWired(w), ); if (broken.length > 0) { return { @@ -504,7 +532,7 @@ async function checkSkills( status: 'warn', required: false, detail: `${broken.map(describeProblem).join('; ')}. Full state: ${describeWiring(inPlay)}`, - fix: fixFor(home, broken), + fix: fixFor(home, broken, resolvedHermesHome), data, }, }; @@ -518,7 +546,7 @@ async function checkSkills( // keeps the lane itself safe either way, which is why this is warn, not fail. const payDrift: string[] = []; for (const w of inPlay) { - if (!harnessInPlay(home, w.dir, present, requested)) continue; + if (!harnessInPlay(home, w.dir, present, requested, resolvedHermesHome)) continue; const onDisk = await readSkillFile(join(w.dir, OPTIONAL_PAY_SKILL, 'SKILL.md')); if ((onDisk.kind === 'ok') !== bazaarPay) payDrift.push(w.dir); } @@ -579,6 +607,7 @@ async function checkSkills( fix: fixFor( home, wiring.filter((w) => stale.includes(w.dir)), + resolvedHermesHome, ), data, }, @@ -596,6 +625,65 @@ async function checkSkills( }; } +/** Native Hermes wiring is a separate warn-level check from portable skills. */ +async function checkHermes(args: { + home: string; + hermesHome: string; + which: (bin: string) => boolean; + requested: readonly HarnessTarget[]; + searchMode?: SearchHookMode; + homeWarning?: string; +}): Promise { + const { home, hermesHome, which, requested, searchMode, homeWarning } = args; + const inPlay = + requested.includes('hermes') || harnessDetectedBy(home, 'hermes', which, hermesHome).length > 0; + if (!inPlay) return null; + const status = { + ...(await readHermesIntegrationStatus(hermesHome)), + ...(homeWarning !== undefined ? { homeWarning } : {}), + }; + const ok = + status.mcp === 'configured' && status.plugin === 'installed' && status.activation === 'enabled'; + if (ok && homeWarning === undefined) { + return { + result: { + name: 'hermes', + status: 'ok', + required: false, + detail: `Native Tenjin retrieval and publish-back plugin enabled in ${hermesHome}`, + data: status, + }, + }; + } + const problems: string[] = []; + if (status.mcp === 'stale') { + problems.push(`MCP command missing (${status.mcpCommand ?? 'unknown'})`); + } else if (status.mcp !== 'configured') problems.push(`MCP ${status.mcp}`); + if (status.plugin !== 'installed') problems.push(`plugin ${status.plugin}`); + // Named `activation`, not a second `plugin`: "plugin missing, plugin not-enabled" + // read as one subject twice. + if (status.activation !== 'enabled') problems.push(`activation ${status.activation}`); + if (homeWarning !== undefined) problems.push('HERMES_HOME ignored'); + return { + result: { + name: 'hermes', + status: 'warn', + required: false, + detail: `Hermes Tenjin integration incomplete in ${hermesHome}: ${problems.join(', ')}${ + homeWarning === undefined ? '' : `. ${homeWarning}` + }`, + // `tenjin install --harness hermes` alone is a dead end when the stored mode + // is `off`: it re-runs, withholds the hook code by design, and prints the same + // warning forever. Name the blocker that actually has to move first. + fix: + searchMode === 'off' + ? 'tenjin config set hooks.searchMode auto && tenjin install --harness hermes' + : 'tenjin install --harness hermes', + data: status, + }, + }; +} + /** * How the wired CLI adapter skills compare to the packaged ones. * @@ -702,8 +790,8 @@ function hostedHere(w: HarnessWiring): boolean { * the directories detection picks, so a problem in ~/.agents/skills on a * Claude-only machine needs `--harness shared` spelled out. */ -function fixFor(home: string, dirs: HarnessWiring[]): string { - const flags = [...new Set(dirs.map((w) => harnessFlagFor(home, w.dir)))]; +function fixFor(home: string, dirs: HarnessWiring[], hermesHome: string): string { + const flags = [...new Set(dirs.map((w) => harnessFlagFor(home, w.dir, hermesHome)))]; return `tenjin install ${flags.map((f) => `--harness ${f}`).join(' ')}`; } diff --git a/src/commands/install.test.ts b/src/commands/install.test.ts index c77da47..1009930 100644 --- a/src/commands/install.test.ts +++ b/src/commands/install.test.ts @@ -243,6 +243,11 @@ type Harnesses = Array<{ codexNetworkRule?: string; warnings: string[]; notes: string[]; + hermes?: { + mcp: { status: string }; + plugin: { status: string; scriptPaths: string[] }; + activation: { status: string }; + }; }>; type Data = { dryRun: boolean; @@ -289,6 +294,56 @@ describe('runInstall: harness override', () => { expect(asData(d).harnesses).toHaveLength(1); }); + it('installs and activates the native Hermes plugin when explicitly requested', async () => { + const { data: d } = await runInstall( + { harness: ['hermes'], noWallet: true }, + makeCtx(), + deps({ tenjinCommand: '/opt/tenjin/bin/tenjin', nodeCommand: process.execPath }), + ); + const h = asData(d).harnesses[0]!; + expect(h.harness).toBe('hermes'); + expect(h.skillsDir).toBe(join(home, '.hermes', 'skills')); + expect(h.hermes?.mcp.status).toBe('installed'); + expect(h.hermes?.plugin.status).toBe('installed'); + expect(h.hermes?.activation.status).toBe('installed'); + expect(await readFile(join(home, '.hermes', 'config.yaml'), 'utf8')).toContain( + 'enabled:\n - tenjin', + ); + }); + + // The README's `--no-hooks` row says "Register no hooks this run; writes no + // config", and the Claude path honors it by writing no scripts at all. Anything + // less here (withholding only the `plugins.enabled` line) leaves hook code on + // disk and then names a fix that cannot move the blocker. + it('--no-hooks writes no Hermes hook code, only the MCP entry', async () => { + const { data: d } = await runInstall( + { harness: ['hermes'], noWallet: true, noHooks: true }, + makeCtx(), + deps({ tenjinCommand: '/opt/tenjin/bin/tenjin', nodeCommand: process.execPath }), + ); + const h = asData(d).harnesses[0]!; + expect(h.hermes?.mcp.status).toBe('installed'); + expect(h.hermes?.plugin.status).toBe('skipped'); + expect(h.hermes?.plugin.scriptPaths).toEqual([]); + expect(h.hermes?.activation.status).toBe('skipped'); + await expect( + readFile(join(home, '.hermes', 'plugins', 'tenjin', '__init__.py'), 'utf8'), + ).rejects.toThrow(); + await expect(readFile(join(data, 'hooks', 'tenjin-websearch.mjs'), 'utf8')).rejects.toThrow(); + }); + + it('a stored searchMode of off withholds the plugin and names the real blocker', async () => { + const { data: d } = await runInstall( + { harness: ['hermes'], noWallet: true, searchHooks: 'off' }, + makeCtx(), + deps({ tenjinCommand: '/opt/tenjin/bin/tenjin', nodeCommand: process.execPath }), + ); + const h = asData(d).harnesses[0]!; + expect(h.hermes?.plugin.status).toBe('skipped'); + // Not "re-run `tenjin install --harness hermes`", which loops forever. + expect(h.warnings.join(' ')).toContain('hooks.searchMode auto'); + }); + it('rejects an unknown harness as USAGE / exit 2', async () => { const err = await caught(() => runInstall({ harness: ['cursor'] }, makeCtx(), deps())); expect(err.code).toBe('USAGE'); diff --git a/src/commands/install.ts b/src/commands/install.ts index 7eaed8a..4c5f49a 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -61,6 +61,8 @@ import type { PermissionsResult } from '../lib/harness-permissions'; import { hooksSkipped, hooksUndo, wireSearchHooks } from '../lib/harness-hooks'; import { removeMarkerLines } from '../lib/uninstall'; import type { HooksResult } from '../lib/harness-hooks'; +import { resolveHermesHome, resolveHermesHomeLenient, wireHermesIntegration } from '../lib/hermes'; +import type { HermesIntegrationResult } from '../lib/hermes'; import { confirmChoice, intro as clackIntro, outro as clackOutro, selectOne } from '../lib/clack'; import { sanitizeForTerminal } from '../lib/output'; import type { Io } from '../lib/output'; @@ -204,6 +206,8 @@ interface HarnessResult { codexNetworkRule?: string; notes: string[]; warnings: string[]; + /** Native Hermes MCP/plugin wiring; present only for the Hermes target. */ + hermes?: HermesIntegrationResult; } export interface InstallDeps { @@ -256,6 +260,10 @@ export interface InstallDeps { * keychain under the `tenjin-cli` service. */ walletPassphrase?: PassphraseOverrides; + /** Absolute CLI entrypoint embedded in Hermes' MCP config. */ + tenjinCommand?: string; + /** Absolute Node executable embedded in the Hermes native plugin. */ + nodeCommand?: string; } /** @@ -377,6 +385,14 @@ async function installBody( ); } const which = deps.which ?? ((bin: string) => onPath(bin, env)); + // A relative HERMES_HOME is only fatal when the operator asked for Hermes. On any + // other run it is a stray env var belonging to something else, and taking the + // whole install down over it punishes the wrong machine. + const targetsHermes = parsed.data.harness?.includes('hermes') === true; + const hermesTarget = targetsHermes + ? { home: resolveHermesHome(home, env), warning: undefined } + : resolveHermesHomeLenient(home, env); + const hermesHome = hermesTarget.home; // Human-first is the global output rule (emitSuccess renders humanLines at a TTY // without --json and no envelope). `humanOutput` matches that gate so install @@ -389,7 +405,7 @@ async function installBody( deps.skillsSourceDir ?? resolveSkillsSource(fileURLToPath(new URL('.', import.meta.url))); await assertSkillsSource(skillsSource); - const plans = resolvePlans(parsed.data.harness, home, which); + const plans = resolvePlans(parsed.data.harness, home, hermesHome, which); // Same condition resolvePlans treats as an override, so what gets recorded below is // exactly what overrode detection. const explicitHarness = parsed.data.harness !== undefined && parsed.data.harness.length > 0; @@ -448,6 +464,39 @@ async function installBody( const hooks = await underDataDir(ctx.dataDir, () => resolveHooks({ plans, home, ctx, deps, flag: searchHooksFlag, noHooks, dryRun, canPrompt }), ); + const hermesResult = harnesses.find((result) => result.harness === 'hermes'); + if (hermesResult !== undefined) { + const tenjinCommand = deps.tenjinCommand ?? process.argv[1]; + const nodeCommand = deps.nodeCommand ?? process.execPath; + if (tenjinCommand === undefined || !isAbsolute(tenjinCommand) || !isAbsolute(nodeCommand)) { + throw new CliError( + 'INTERNAL', + 'Hermes integration requires absolute Tenjin and Node executable paths.', + { fix: 'Run `tenjin install --harness hermes` through the installed Tenjin CLI.' }, + ); + } + hermesResult.hermes = await wireHermesIntegration({ + hermesHome, + dataDir: ctx.dataDir, + tenjinCommand, + nodeCommand, + dryRun, + // Activation consent: the operator named Hermes on the command line. + explicit: explicitHarness && targetsHermes, + // Write consent, read off the SAME hooks decision that gates Claude's + // settings.json, because `--no-hooks` promises "writes no config" in the + // README and that promise cannot hold on only one of the two harnesses. + hooks: { enabled: hermesHooksEnabled(hooks), fix: hooks.fix, mode: hooks.mode }, + }); + for (const part of [ + hermesResult.hermes.mcp, + hermesResult.hermes.plugin, + hermesResult.hermes.activation, + ]) { + if (part.warning !== undefined) hermesResult.warnings.push(part.warning); + } + if (hermesTarget.warning !== undefined) hermesResult.warnings.push(hermesTarget.warning); + } // On BOTH paths now: the loop this command sets up needs a key, so a headless // run creates one rather than leaving the operator a setup that stops at the // first buy or publish. @@ -737,6 +786,9 @@ function hooksLine(io: Io, h: HooksResult): string { if (h.skipped === 'harness-not-claude') { return `${paint(io, 'dim', '-')} ${label} not wired (Claude Code only).`; } + if (h.skipped === 'native-harness') { + return `${paint(io, 'green', '✓')} ${label} ${h.mode} mode through the native Hermes plugin. Change: tenjin config set hooks.searchMode `; + } if (h.skipped === 'dry-run') { return `${paint(io, 'dim', '-')} ${label} unchanged (dry run).`; } @@ -753,7 +805,13 @@ function hooksLine(io: Io, h: HooksResult): string { } function harnessLabel(h: Harness): string { - return h === 'claude' ? 'Claude Code' : h === 'codex' ? 'Codex' : 'Agent Skills'; + return h === 'claude' + ? 'Claude Code' + : h === 'codex' + ? 'Codex' + : h === 'hermes' + ? 'Hermes' + : 'Agent Skills'; } /** @@ -1349,7 +1407,8 @@ async function resolvePermissions(args: { // Agent Skills location gate permissions elsewhere, so there is nothing here to // write for them, and guessing at another harness's config would be the kind of // uninvited write this whole module is careful about. - if (!plans.some((p) => p.harness === 'claude')) { + const hasClaude = plans.some((p) => p.harness === 'claude'); + if (!hasClaude) { return withRetraction( permissionsSkipped(plans[0]?.harness ?? 'shared', home, 'harness-not-claude'), ); @@ -1423,8 +1482,10 @@ async function resolveHooks(args: { const { plans, home, ctx, deps, flag, noHooks, dryRun, canPrompt } = args; const dataDir = ctx.dataDir; const stored = (await loadRawConfig(dataDir)).hooks?.searchMode; + const hasClaude = plans.some((p) => p.harness === 'claude'); + const hasHermes = plans.some((p) => p.harness === 'hermes'); - if (!plans.some((p) => p.harness === 'claude')) { + if (!hasClaude && !hasHermes) { const harness = plans[0]?.harness ?? 'shared'; return hooksSkipped( harness, @@ -1438,7 +1499,13 @@ async function resolveHooks(args: { // mode is reported unchanged and a later bare re-run wires them. That is the // difference from `--search-hooks off`, which is a durable statement. if (noHooks) { - return hooksSkipped('claude', home, dataDir, stored ?? DEFAULT_HOOK_MODE, 'declined'); + return hooksSkipped( + hasHermes ? 'hermes' : 'claude', + home, + dataDir, + stored ?? DEFAULT_HOOK_MODE, + 'declined', + ); } const mode = await chooseHookMode(flag, stored, deps, dryRun, canPrompt); @@ -1447,19 +1514,43 @@ async function resolveHooks(args: { // this walkthrough already treats Escape that way, and this one used to be the // single prompt where backing out still wired and persisted a mode. if (mode === null) { - return hooksSkipped('claude', home, dataDir, stored ?? DEFAULT_HOOK_MODE, 'declined'); + return hooksSkipped( + hasHermes ? 'hermes' : 'claude', + home, + dataDir, + stored ?? DEFAULT_HOOK_MODE, + 'declined', + ); } - if (dryRun) return hooksSkipped('claude', home, dataDir, mode, 'dry-run'); + const resultHarness = hasHermes && !hasClaude ? 'hermes' : 'claude'; + if (dryRun) return hooksSkipped(resultHarness, home, dataDir, mode, 'dry-run'); if (mode !== (stored ?? DEFAULT_HOOK_MODE) || stored === undefined) { await persistSearchHookMode(dataDir, mode); } // `off` is a decision not to register anything, so settings.json is not touched // at all. It is NOT the same as an inert script: an operator who later sets the // mode back to `auto` re-runs install, which is what the fix string says. - if (mode === 'off') return hooksSkipped('claude', home, dataDir, mode, 'mode-off'); + if (mode === 'off') return hooksSkipped(resultHarness, home, dataDir, mode, 'mode-off'); + if (!hasClaude) return hooksSkipped('hermes', home, dataDir, mode, 'native-harness'); return wireSearchHooks({ homeDir: home, dataDir, mode }); } +/** + * Whether THIS run may write Hermes hook code, read off the single hooks decision + * so the native path can never be more permissive than Claude's. + * + * Only the three reasons that are an operator choice withhold it. A Claude + * settings.json that could not be read or parsed is a Claude problem: on a machine + * running both, it must not silently cancel the Hermes wiring as well. + */ +function hermesHooksEnabled(hooks: HooksResult): boolean { + return ( + hooks.skipped !== 'declined' && + hooks.skipped !== 'mode-off' && + hooks.skipped !== 'harness-not-claude' + ); +} + /** The stored default for a run that was never asked. */ const DEFAULT_HOOK_MODE: SearchHookMode = CONFIG_DEFAULTS.hooks.searchMode; @@ -1517,23 +1608,28 @@ interface HarnessPlan { function resolvePlans( override: string[] | undefined, home: string, + hermesHome: string, which: (bin: string) => boolean, ): HarnessPlan[] { if (override !== undefined && override.length > 0) { - const plans = override.map((v) => planFor(validateHarness(v), ['override'], true, home)); + const plans = override.map((v) => + planFor(validateHarness(v), ['override'], true, home, hermesHome), + ); return dedupeBySkillsDir(plans); } const plans: HarnessPlan[] = []; // Same two probes doctor's skills check gates its per-directory verdicts on. - const claudeBy = harnessDetectedBy(home, 'claude', which); - const codexBy = harnessDetectedBy(home, 'codex', which); - if (claudeBy.length > 0) plans.push(planFor('claude', claudeBy, true, home)); - if (codexBy.length > 0) plans.push(planFor('codex', codexBy, true, home)); + const claudeBy = harnessDetectedBy(home, 'claude', which, hermesHome); + const codexBy = harnessDetectedBy(home, 'codex', which, hermesHome); + const hermesBy = harnessDetectedBy(home, 'hermes', which, hermesHome); + if (claudeBy.length > 0) plans.push(planFor('claude', claudeBy, true, home, hermesHome)); + if (codexBy.length > 0) plans.push(planFor('codex', codexBy, true, home, hermesHome)); + if (hermesBy.length > 0) plans.push(planFor('hermes', hermesBy, true, home, hermesHome)); if (plans.length === 0) { // Nothing detected: the shared Agent Skills location is the fallback target, so // a harness installed later still finds the skills. - plans.push(planFor('shared', ['fallback'], false, home)); + plans.push(planFor('shared', ['fallback'], false, home, hermesHome)); } return dedupeBySkillsDir(plans); } @@ -1543,9 +1639,17 @@ function planFor( detectedBy: string[], detected: boolean, home: string, + hermesHome: string, ): HarnessPlan { - const skillsDir = harnessTargetDir(home, harness); - return { harness, detected, detectedBy, skillsDir, wiresAgentsMd: harness !== 'claude', home }; + const skillsDir = harnessTargetDir(home, harness, hermesHome); + return { + harness, + detected, + detectedBy, + skillsDir, + wiresAgentsMd: harness !== 'claude' && harness !== 'hermes', + home, + }; } function dedupeBySkillsDir(plans: HarnessPlan[]): HarnessPlan[] { diff --git a/src/commands/uninstall.test.ts b/src/commands/uninstall.test.ts index e43cb44..d00ea19 100644 --- a/src/commands/uninstall.test.ts +++ b/src/commands/uninstall.test.ts @@ -129,6 +129,24 @@ describe('runUninstall — a fully installed machine', () => { }); }); + // `skillsDirsFor` requires a Hermes home precisely so a new caller cannot quietly + // leave that directory behind. Both spellings are covered: the default and an + // absolute HERMES_HOME, which is the one a defaulted argument would have missed. + it('removes the Hermes skills, including under an absolute HERMES_HOME', async () => { + await seedSkill('.hermes/skills', 'tenjin-search'); + const custom = join(home, 'custom-hermes'); + await seedSkill(join('custom-hermes', 'skills'), 'tenjin-publish'); + + const bare = (await runUninstall(makeCtx(), { home, env: {} })).data as UninstallReport; + expect(bare.skills).toHaveLength(1); + expect(existsSync(join(home, '.hermes', 'skills', 'tenjin-search'))).toBe(false); + + const scoped = (await runUninstall(makeCtx(), { home, env: { HERMES_HOME: custom } })) + .data as UninstallReport; + expect(scoped.skills).toHaveLength(1); + expect(existsSync(join(custom, 'skills', 'tenjin-publish'))).toBe(false); + }); + // Ownership, not position: a rule or entry we did not write keeps its place even // when one of ours is removed from in front of it. it('leaves another tool’s hook entry and allow rule exactly where they were', async () => { diff --git a/src/commands/uninstall.ts b/src/commands/uninstall.ts index 7fffb6d..0616595 100644 --- a/src/commands/uninstall.ts +++ b/src/commands/uninstall.ts @@ -34,6 +34,8 @@ import type { CommandContext, CommandResult } from '../context'; export interface UninstallDeps { /** Home whose harness directories are cleaned; tests inject a temp dir. */ home?: string; + /** Environment the Hermes home is resolved from; defaults to process.env. */ + env?: NodeJS.ProcessEnv; } export async function runUninstall( @@ -48,7 +50,7 @@ export async function runUninstall( // at one that does not. const settings = await removeFromSettings(home); const scripts = await removeHookScripts(ctx.dataDir); - const skills = await removeSkills(home); + const skills = await removeSkills(home, deps.env); const markers = await removeMarkerLines(home); const report: UninstallReport = { diff --git a/src/lib/harness-hooks.ts b/src/lib/harness-hooks.ts index ebf8ad0..3f0eefa 100644 --- a/src/lib/harness-hooks.ts +++ b/src/lib/harness-hooks.ts @@ -69,6 +69,7 @@ const HOOK_TIMEOUT_SECONDS = 5; export type HooksSkipReason = | 'harness-not-claude' + | 'native-harness' | 'mode-off' | 'declined' | 'dry-run' @@ -159,6 +160,8 @@ function fixFor(reason: HooksSkipReason): string { switch (reason) { case 'harness-not-claude': return 'Hooks are wired for Claude Code only. Re-run `tenjin install --harness claude` on a machine with Claude Code.'; + case 'native-harness': + return "Hermes uses Tenjin's native plugin adapter; change behavior with `tenjin config set hooks.searchMode `."; case 'mode-off': return 'Enable them with `tenjin config set hooks.searchMode auto`, then re-run `tenjin install`.'; case 'declined': @@ -243,6 +246,28 @@ function specs(dataDir: string): HookSpec[] { ]; } +/** + * Bring the shared standalone search and publish-back scripts up to date without + * registering Claude settings. Native harness adapters (currently Hermes) call + * these same bodies with their own envelope, so validation and local state never + * fork into a second implementation. + */ +export async function writeSharedHookScripts(dataDir: string): Promise<{ + scriptsDir: string; + written: string[]; + websearchPath: string; + stopPath: string; +}> { + const scriptsDir = hooksDir(dataDir); + const written = await writeScripts(specs(dataDir), scriptsDir); + return { + scriptsDir, + written, + websearchPath: join(scriptsDir, WEBSEARCH_HOOK_FILE), + stopPath: join(scriptsDir, STOP_HOOK_FILE), + }; +} + /** * Does the Stop hook ON DISK match what this build would write? * diff --git a/src/lib/hermes.test.ts b/src/lib/hermes.test.ts new file mode 100644 index 0000000..0fbb76c --- /dev/null +++ b/src/lib/hermes.test.ts @@ -0,0 +1,554 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import { + HERMES_PLUGIN_MANIFEST, + HERMES_WEB_SEARCH_TOOL, + hermesConfigPath, + hermesPluginDir, + readHermesIntegrationStatus, + resolveHermesHome, + resolveHermesHomeLenient, + wireHermesIntegration, + wireHermesMcp, +} from './hermes'; +import { CliError } from './errors'; + +const execFileAsync = promisify(execFile); +let home: string; +let dataDir: string; + +beforeEach(async () => { + home = await mkdtemp(join(tmpdir(), 'tenjin-hermes-')); + dataDir = await mkdtemp(join(tmpdir(), 'tenjin-hermes-data-')); +}); +afterEach(async () => { + await rm(home, { recursive: true, force: true }); + await rm(dataDir, { recursive: true, force: true }); +}); + +describe('resolveHermesHome', () => { + it('defaults under HOME and honors an absolute HERMES_HOME', () => { + expect(resolveHermesHome(home, {})).toBe(join(home, '.hermes')); + expect(resolveHermesHome(home, { HERMES_HOME: join(home, 'custom') })).toBe( + join(home, 'custom'), + ); + }); + + it('rejects a relative HERMES_HOME before any write', () => { + expect(() => resolveHermesHome(home, { HERMES_HOME: 'relative/hermes' })).toThrow(CliError); + }); + + // Doctor is the command you reach for when something is already broken, so a + // stray env var belonging to another tool must not take it down before a single + // check runs. Only a run that explicitly targeted Hermes gets the refusal. + it('the lenient resolver warns and falls back instead of throwing', () => { + const resolved = resolveHermesHomeLenient(home, { HERMES_HOME: 'relative/hermes' }); + expect(resolved.home).toBe(join(home, '.hermes')); + expect(resolved.warning).toContain('relative/hermes'); + expect(resolveHermesHomeLenient(home, {}).warning).toBeUndefined(); + }); +}); + +describe('wireHermesMcp', () => { + it('creates a private, idempotent MCP config without harness or wallet env coupling', async () => { + const command = '/opt/tenjin/bin/tenjin'; + expect((await wireHermesMcp(home, false, command)).status).toBe('installed'); + const text = await readFile(hermesConfigPath(home), 'utf8'); + expect(text).toContain(`command: ${JSON.stringify(command)}`); + expect(text).toContain('args: ["mcp"]'); + expect(text).not.toContain('TENJIN_HARNESS'); + expect((await stat(hermesConfigPath(home))).mode & 0o777).toBe(0o600); + expect((await wireHermesMcp(home, false, command)).status).toBe('up-to-date'); + }); + + it('preserves unrelated YAML while adding one child', async () => { + await writeFile( + hermesConfigPath(home), + [ + 'model: llama', + 'mcp_servers:', + ' github:', + ' command: "gh-mcp"', + 'theme: dark', + '', + ].join('\n'), + ); + await wireHermesMcp(home, false, '/opt/tenjin'); + const text = await readFile(hermesConfigPath(home), 'utf8'); + expect(text).toContain(' github:\n command: "gh-mcp"'); + expect(text).toContain(' tenjin:'); + expect(text).toContain('theme: dark'); + expect(text.match(/ {2}tenjin:/g)).toHaveLength(1); + }); + + it.each([ + ['four-space children', 'mcp_servers:\n github:\n command: "gh-mcp"\n'], + ['a sequence', 'mcp_servers:\n - command: "gh-mcp"\n'], + ['inline YAML', 'mcp_servers: { github: { command: gh-mcp } }\n'], + ])('leaves unsupported %s byte-identical', async (_label, yaml) => { + await writeFile(hermesConfigPath(home), yaml); + const result = await wireHermesMcp(home, false, '/opt/tenjin'); + expect(result.status).toBe('conflict'); + expect(await readFile(hermesConfigPath(home), 'utf8')).toBe(yaml); + }); + + it('refuses a user-owned Tenjin entry', async () => { + const yaml = 'mcp_servers:\n tenjin:\n command: "custom"\n'; + await writeFile(hermesConfigPath(home), yaml); + expect((await wireHermesMcp(home, false, '/opt/tenjin')).status).toBe('conflict'); + expect(await readFile(hermesConfigPath(home), 'utf8')).toBe(yaml); + }); + + it('refuses an inline user-owned Tenjin entry without appending a duplicate', async () => { + const yaml = 'mcp_servers:\n tenjin: { command: custom }\n'; + await writeFile(hermesConfigPath(home), yaml); + expect((await wireHermesMcp(home, false, '/opt/tenjin')).status).toBe('conflict'); + expect(await readFile(hermesConfigPath(home), 'utf8')).toBe(yaml); + }); + + // `command` is `process.argv[1]`, so an nvm switch, a pnpm-vs-npm global, or a + // project-local install re-points on the next run. Nothing covered re-point at + // all before: the preservation test above only exercised first insertion. + describe('re-pointing an entry this CLI owns', () => { + it('replaces the block in place instead of stacking marker comments', async () => { + await wireHermesMcp(home, false, '/old/tenjin'); + expect((await wireHermesMcp(home, false, '/new/tenjin')).status).toBe('installed'); + const text = await readFile(hermesConfigPath(home), 'utf8'); + expect(text.match(/tenjin-cli:hermes-mcp/g)).toHaveLength(1); + expect(text.match(/ {2}tenjin:/g)).toHaveLength(1); + expect(text).toContain('command: "/new/tenjin"'); + expect(text).not.toContain('/old/tenjin'); + }); + + it('leaves a following comment and blank line where the operator put them', async () => { + await writeFile( + hermesConfigPath(home), + [ + 'mcp_servers:', + ' # tenjin-cli:hermes-mcp', + ' tenjin:', + ' command: "/old/tenjin"', + ' args: ["mcp"]', + '', + ' # the notes app, no colon in this line', + ' notes:', + ' command: "notes-mcp"', + '', + ].join('\n'), + ); + await wireHermesMcp(home, false, '/new/tenjin'); + const text = await readFile(hermesConfigPath(home), 'utf8'); + expect(text).toContain(' # the notes app, no colon in this line\n notes:'); + expect(text).toContain('command: "notes-mcp"'); + expect(text.match(/tenjin-cli:hermes-mcp/g)).toHaveLength(1); + // The whole diff is the one command line. + expect(text.split('\n').filter((l) => l.includes('/old/tenjin'))).toEqual([]); + expect(text.split('\n')).toHaveLength(10); + }); + + it('re-pointing back to the same command is a no-op', async () => { + await wireHermesMcp(home, false, '/opt/tenjin'); + const before = await readFile(hermesConfigPath(home), 'utf8'); + expect((await wireHermesMcp(home, false, '/opt/tenjin')).status).toBe('up-to-date'); + expect(await readFile(hermesConfigPath(home), 'utf8')).toBe(before); + }); + }); +}); + +describe('wireHermesIntegration', () => { + const commands = { + tenjinCommand: '/opt/tenjin', + nodeCommand: process.execPath, + hooks: { enabled: true, mode: 'auto' as const }, + }; + + it('writes a native plugin, shared scripts, MCP config, and explicit activation', async () => { + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + expect(result.activation.status).toBe('installed'); + expect(await readFile(hermesConfigPath(home), 'utf8')).toContain( + 'plugins:\n enabled:\n - tenjin', + ); + const plugin = await readFile(join(hermesPluginDir(home), '__init__.py'), 'utf8'); + expect(plugin).toContain('ctx.register_hook("pre_tool_call"'); + expect(plugin).toContain('ctx.register_hook("transform_tool_result"'); + expect(plugin).toContain('ctx.register_hook("transform_llm_output"'); + await execFileAsync('python3', [ + '-m', + 'py_compile', + join(hermesPluginDir(home), '__init__.py'), + ]); + const probe = [ + 'import importlib.util, json, sys', + 'spec = importlib.util.spec_from_file_location("tenjin_plugin", sys.argv[1])', + 'mod = importlib.util.module_from_spec(spec)', + 'spec.loader.exec_module(mod)', + 'hooks = {}', + 'class Ctx:', + ' def register_hook(self, name, callback): hooks[name] = callback', + 'mod.register(Ctx())', + 'mod._run = lambda script, payload, timeout: "listing" if script == mod.WEBSEARCH_SCRIPT else "publish"', + 'hooks["pre_tool_call"](tool_name="web_search", args={"query": "q"}, tool_call_id="c1")', + 'tool = hooks["transform_tool_result"](tool_name="web_search", result="web result", tool_call_id="c1")', + 'final = hooks["transform_llm_output"](response_text="answer")', + 'print(json.dumps({"names": sorted(hooks), "tool": tool, "final": final}))', + ].join('\n'); + const { stdout } = await execFileAsync('python3', [ + '-c', + probe, + join(hermesPluginDir(home), '__init__.py'), + ]); + expect(JSON.parse(stdout)).toEqual({ + names: ['pre_tool_call', 'transform_llm_output', 'transform_tool_result'], + tool: 'web result\n\n--- Tenjin marketplace context ---\nlisting\n--- end Tenjin context ---', + final: 'answer\n\n--- Tenjin publish-back reminder ---\npublish', + }); + // `writeSharedHookScripts` writes the whole shared bundle in one pass + // (websearch, dispatch, session-primer, stop), so Hermes wiring reports all + // four even though its own plugin only calls websearch and stop directly. + expect(result.plugin.scriptPaths).toHaveLength(4); + }); + + it('keeps auto-detected code inert until explicitly enabled', async () => { + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: false, + ...commands, + }); + expect(result.activation.status).toBe('disabled'); + expect(result.activation.warning).toContain('--harness hermes'); + expect(await readFile(hermesConfigPath(home), 'utf8')).not.toContain('plugins:'); + }); + + it('never overrides an explicit plugins.disabled entry', async () => { + await writeFile(hermesConfigPath(home), 'plugins:\n disabled:\n - tenjin\n'); + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + expect(result.activation.status).toBe('disabled'); + expect(await readFile(hermesConfigPath(home), 'utf8')).toContain('disabled:\n - tenjin'); + expect(await readFile(hermesConfigPath(home), 'utf8')).not.toContain('enabled:'); + }); + + it('honors an inline plugins.disabled list too', async () => { + await writeFile(hermesConfigPath(home), 'plugins:\n disabled: [tenjin, other]\n'); + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + expect(result.activation.status).toBe('disabled'); + const text = await readFile(hermesConfigPath(home), 'utf8'); + expect(text).toContain('disabled: [tenjin, other]'); + expect(text).not.toContain('enabled:'); + }); + + // The Python probe below builds its own Ctx, so no test can notice a manifest + // Hermes would not read: pin it here instead. Against `hermes_cli/plugins.py`, + // which parses `provides_hooks` and defaults `kind` to `standalone`. + it('pins the manifest to the fields the Hermes loader parses', async () => { + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + const manifest = await readFile(join(hermesPluginDir(home), 'plugin.yaml'), 'utf8'); + expect(manifest).toBe(HERMES_PLUGIN_MANIFEST); + expect(manifest).toContain('kind: standalone'); + expect(manifest).toContain('provides_hooks:\n - pre_tool_call'); + expect(manifest).toContain(' - transform_tool_result'); + expect(manifest).toContain(' - transform_llm_output'); + }); + + // A wrong tool identifier fails exactly the way a wrong manifest field would: + // the callbacks register, never match, and the suite stays green. + it('observes the tool Hermes actually names, and nothing else', async () => { + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + expect(HERMES_WEB_SEARCH_TOOL).toBe('web_search'); + const pluginPath = join(hermesPluginDir(home), '__init__.py'); + expect(await readFile(pluginPath, 'utf8')).toContain('tool_name != "web_search"'); + const probe = [ + 'import importlib.util, json, sys', + 'spec = importlib.util.spec_from_file_location("tenjin_plugin", sys.argv[1])', + 'mod = importlib.util.module_from_spec(spec)', + 'spec.loader.exec_module(mod)', + 'hooks = {}', + 'class Ctx:', + ' def register_hook(self, name, callback): hooks[name] = callback', + 'mod.register(Ctx())', + 'mod._run = lambda script, payload, timeout: "listing"', + // Hermes injects extra kwargs (telemetry_schema_version, task_id, ...), so the + // callbacks have to tolerate them rather than only the documented names. + 'hooks["pre_tool_call"](tool_name="WebSearch", args={"query": "q"}, tool_call_id="c1", telemetry_schema_version=1)', + 'other = hooks["transform_tool_result"](tool_name="WebSearch", result="r", tool_call_id="c1", task_id="t")', + 'hooks["pre_tool_call"](tool_name="web_search", args={"query": "q"}, tool_call_id="c2", turn_id="t1", telemetry_schema_version=1)', + 'mine = hooks["transform_tool_result"](tool_name="web_search", result="r", tool_call_id="c2", duration_ms=3, status="ok")', + 'print(json.dumps({"other": other, "mine": mine}))', + ].join('\n'); + const { stdout } = await execFileAsync('python3', ['-c', probe, pluginPath]); + expect(JSON.parse(stdout)).toEqual({ + other: null, + mine: 'r\n\n--- Tenjin marketplace context ---\nlisting\n--- end Tenjin context ---', + }); + }); + + // Main's session-scoped weak-arm rate limit (#164, hook-scripts.ts + // `batchedThisSession`) keys off the Stop hook payload's `session_id`. An empty + // payload here would batch every Hermes session together, letting the nag + // re-fire every turn of a multi-turn session instead of once — the #162 + // regression the session key exists to prevent. + it('forwards session_id and cwd into the STOP_SCRIPT payload', async () => { + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + const pluginPath = join(hermesPluginDir(home), '__init__.py'); + const probe = [ + 'import importlib.util, json, sys', + 'spec = importlib.util.spec_from_file_location("tenjin_plugin", sys.argv[1])', + 'mod = importlib.util.module_from_spec(spec)', + 'spec.loader.exec_module(mod)', + 'hooks = {}', + 'class Ctx:', + ' def register_hook(self, name, callback): hooks[name] = callback', + 'mod.register(Ctx())', + 'captured = {}', + 'def fake_run(script, payload, timeout):', + ' captured["payload"] = payload', + ' return "publish"', + 'mod._run = fake_run', + 'hooks["transform_llm_output"](response_text="answer", session_id="sess-1", cwd="/proj", task_id="t")', + 'print(json.dumps(captured["payload"]))', + ].join('\n'); + const { stdout } = await execFileAsync('python3', ['-c', probe, pluginPath]); + expect(JSON.parse(stdout)).toEqual({ session_id: 'sess-1', cwd: '/proj' }); + }); + + // The README's `--no-hooks` row promises "writes no config", and the Claude path + // honors it by writing no scripts at all. Withholding only the `plugins.enabled` + // line would leave hook code on disk that the operator never consented to. + it('writes no hook code at all when the hooks decision said no', async () => { + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + hooks: { + enabled: false, + mode: 'auto', + fix: 'Enable them with `tenjin config set hooks.searchMode auto`.', + }, + }); + // `skipped` is about THIS RUN. `disabled` is a claim about the target, and on a + // re-run over a working install it would be a false one. + expect(result.plugin.status).toBe('skipped'); + expect(result.plugin.scriptPaths).toEqual([]); + expect(result.activation.status).toBe('skipped'); + // The warning names the blocker that has to move, not the command just run. + expect(result.plugin.warning).toContain('hooks.searchMode auto'); + await expect( + readFile(join(hermesPluginDir(home), '__init__.py'), 'utf8'), + ).rejects.toMatchObject({ code: 'ENOENT' }); + await expect( + readFile(join(hermesPluginDir(home), 'plugin.yaml'), 'utf8'), + ).rejects.toMatchObject({ code: 'ENOENT' }); + await expect( + readFile(join(dataDir, 'hooks', 'tenjin-websearch.mjs'), 'utf8'), + ).rejects.toThrow(); + await expect(readFile(join(dataDir, 'hooks', 'tenjin-stop.mjs'), 'utf8')).rejects.toThrow(); + // The MCP entry is a server registration, not a hook, so it is still written. + expect(result.mcp.status).toBe('installed'); + expect(await readFile(hermesConfigPath(home), 'utf8')).not.toContain('plugins:'); + }); + + it('writes nothing on dry-run', async () => { + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: true, + explicit: true, + ...commands, + }); + expect(result.mcp.status).toBe('would-install'); + expect(result.plugin.status).toBe('would-install'); + expect(result.activation.status).toBe('would-install'); + // The envelope has to report what WOULD be written, which is the whole shared + // bundle a real run creates (see `writeSharedHookScripts`), not just the two + // scripts this plugin's own hooks call. + expect(result.plugin.scriptPaths).toEqual([ + join(dataDir, 'hooks', 'tenjin-websearch.mjs'), + join(dataDir, 'hooks', 'tenjin-dispatch.mjs'), + join(dataDir, 'hooks', 'tenjin-sessionstart.mjs'), + join(dataDir, 'hooks', 'tenjin-stop.mjs'), + ]); + await expect(readFile(hermesConfigPath(home), 'utf8')).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); +}); + +describe('readHermesIntegrationStatus', () => { + const commands = { + tenjinCommand: process.execPath, + nodeCommand: process.execPath, + hooks: { enabled: true, mode: 'auto' as const }, + }; + + it('a fully wired home reads back green', async () => { + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }); + expect(await readHermesIntegrationStatus(home)).toMatchObject({ + mcp: 'configured', + plugin: 'installed', + activation: 'enabled', + mcpCommand: process.execPath, + }); + }); + + // `command` is `process.argv[1]`, so an `npx`/`pnpm dlx` run bakes a cache path + // that can be pruned later. Deriving the verdict from the marker and a regex + // alone reported Hermes green while Hermes silently failed to start the server. + it('a baked command that no longer exists is stale, not configured', async () => { + await wireHermesMcp(home, false, join(home, 'pruned', 'npx-cache', 'tenjin')); + const status = await readHermesIntegrationStatus(home); + expect(status.mcp).toBe('stale'); + expect(status.mcpCommand).toContain('npx-cache'); + }); + + // One classifier for both sides. A reader more permissive than the writer calls + // `not-enabled` on a shape `planPluginEnable` refuses, and its fix string then + // sends the operator into a conflict it did not predict. + it('reports the conflict the installer would raise, not a false not-enabled', async () => { + await writeFile(hermesConfigPath(home), 'plugins:\n enabled: [other]\n'); + expect((await readHermesIntegrationStatus(home)).activation).toBe('conflict'); + expect( + ( + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + }) + ).activation.status, + ).toBe('conflict'); + }); + + it('an untouched home is missing across the board', async () => { + expect(await readHermesIntegrationStatus(join(home, 'nothing'))).toMatchObject({ + mcp: 'missing', + plugin: 'missing', + activation: 'not-enabled', + }); + }); +}); + +// An agent reads install's JSON. Saying `disabled` about a plugin that is on disk +// and enabled makes it conclude the retrieval reflex is off while it is running, +// which is the one way a status field can be wrong without anything misbehaving. +describe('withholding a write does not misreport the machine', () => { + const commands = { + tenjinCommand: process.execPath, + nodeCommand: process.execPath, + }; + + it('a --no-hooks re-run reports skipped and names the surviving plugin', async () => { + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + hooks: { enabled: true, mode: 'auto' }, + }); + const again = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + hooks: { enabled: false, mode: 'auto', fix: 'Wire them with `tenjin install`.' }, + }); + expect(again.plugin.status).toBe('skipped'); + expect(again.activation.status).toBe('skipped'); + expect(again.plugin.warning).toContain('still in'); + expect(again.plugin.warning).toContain('keeps running'); + // Inert is the strongest thing on offer: no command deletes the plugin dir or + // the `plugins.enabled` entry, so the note must not promise removal. + expect(again.plugin.warning).toContain('Make it inert with'); + expect(again.plugin.warning).not.toMatch(/\bRemove it\b|\buninstall\b/i); + // Install's envelope and doctor's now describe the same machine. + expect(await readHermesIntegrationStatus(home)).toMatchObject({ + plugin: 'installed', + activation: 'enabled', + }); + }); + + it('with the mode stored off the surviving plugin is named as inert, not running', async () => { + await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + hooks: { enabled: true, mode: 'auto' }, + }); + const again = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + hooks: { enabled: false, mode: 'off' }, + }); + expect(again.plugin.warning).toContain('inert'); + expect(again.plugin.warning).not.toContain('keeps running'); + }); + + it('says nothing about a survivor when there is none', async () => { + const result = await wireHermesIntegration({ + hermesHome: home, + dataDir, + dryRun: false, + explicit: true, + ...commands, + hooks: { enabled: false, mode: 'auto' }, + }); + expect(result.plugin.status).toBe('skipped'); + expect(result.plugin.warning).not.toContain('still in'); + }); +}); diff --git a/src/lib/hermes.ts b/src/lib/hermes.ts new file mode 100644 index 0000000..c8d551c --- /dev/null +++ b/src/lib/hermes.ts @@ -0,0 +1,686 @@ +import { readFile, stat } from 'node:fs/promises'; +import { isAbsolute, join } from 'node:path'; +import { writeFileAtomic } from './atomic-json'; +import { CliError } from './errors'; +import { hasCode } from './errno'; +import { writeSharedHookScripts } from './harness-hooks'; +import { + DISPATCH_HOOK_FILE, + SESSIONSTART_HOOK_FILE, + STOP_HOOK_FILE, + WEBSEARCH_HOOK_FILE, +} from './hook-scripts'; +import type { SearchHookMode } from './config'; + +export const HERMES_MCP_MARKER = 'tenjin-cli:hermes-mcp'; +export const HERMES_PLUGIN_NAME = 'tenjin'; + +/** + * The Hermes tool this plugin observes. Getting this string wrong fails silently: + * the callbacks register, never match, and the suite stays green. Pinned against + * `tools/web_tools.py`, where `WEB_SEARCH_SCHEMA["name"] == "web_search"`. + */ +export const HERMES_WEB_SEARCH_TOOL = 'web_search'; + +/** + * `~/.hermes/plugins//plugin.yaml`, pinned against the loader rather than + * against the manifests Hermes ships. + * + * `hermes_cli/plugins.py::_parse_manifest` reads `data.get("provides_hooks", [])`; + * the `hooks:` key used by the in-repo example plugins is silently dropped by that + * same parser, so this emits the field the loader can actually consume. `kind` + * defaults to `standalone` and is spelled out because the default is not + * documented in the manifest examples. None of it is load-bearing: hooks are + * registered by `register(ctx)` calling `ctx.register_hook`, which is checked + * against a module-level VALID_HOOKS set and never against the manifest. The + * manifest is what `hermes plugins list` shows the operator, so it still has to be + * true. See {@link https://github.com/NousResearch/hermes-agent/blob/main/hermes_cli/plugins.py}. + */ +export const HERMES_PLUGIN_MANIFEST = [ + 'name: tenjin', + 'version: "1.0.0"', + 'description: "Check Tenjin before Hermes web searches and surface unresolved searches at turn end."', + 'author: "Tenjin"', + 'kind: standalone', + 'provides_hooks:', + ' - pre_tool_call', + ' - transform_tool_result', + ' - transform_llm_output', + '', +].join('\n'); + +/** + * An ACTION this run took, never a claim about the machine. + * + * `skipped` and `disabled` are the pair to keep apart. `disabled` is a statement + * about the target: the operator's `plugins.disabled` entry was honored, or + * auto-detection left the code inert. `skipped` is a statement about this run + * only: the hooks decision withheld the write, and whatever an earlier run put on + * disk is still there and still whatever it already was. A `--no-hooks` re-run + * over a working install reports `skipped` while `doctor` reports the plugin + * installed and enabled, and both are true; reporting `disabled` there told an + * agent the plugin was off while it was running. + */ +export type HermesWriteStatus = + 'installed' | 'up-to-date' | 'would-install' | 'disabled' | 'skipped' | 'conflict'; + +export interface HermesWriteResult { + path: string; + status: HermesWriteStatus; + warning?: string; +} + +export interface HermesIntegrationResult { + home: string; + explicit: boolean; + mcp: HermesWriteResult & { command?: string }; + plugin: HermesWriteResult & { manifestPath: string; scriptPaths: string[] }; + activation: HermesWriteResult; +} + +export interface HermesIntegrationStatus { + home: string; + /** `stale` means the marker and shape are ours but the baked command no longer exists. */ + mcp: 'configured' | 'stale' | 'missing' | 'conflict'; + plugin: 'installed' | 'missing' | 'partial'; + activation: 'enabled' | 'disabled' | 'not-enabled' | 'conflict'; + /** The command Hermes would run, when one could be parsed out of a managed entry. */ + mcpCommand?: string; + /** Set when a relative HERMES_HOME was ignored in favor of the default. */ + homeWarning?: string; +} + +export interface HermesHomeResolution { + home: string; + /** Present when HERMES_HOME was unusable and `~/.hermes` was used instead. */ + warning?: string; +} + +const RELATIVE_HERMES_HOME_FIX = + 'Set HERMES_HOME to an absolute directory, or unset it to use ~/.hermes.'; + +/** + * Hermes honors HERMES_HOME; reject a relative override before writing anywhere. + * Use this ONLY where the operator explicitly targeted Hermes, so the refusal is + * about the thing they asked for. Every other caller wants + * {@link resolveHermesHomeLenient}: a stray relative value in the environment is + * not a reason to take down a command that was never going to touch Hermes. + */ +export function resolveHermesHome(home: string, env: NodeJS.ProcessEnv): string { + const resolved = resolveHermesHomeLenient(home, env); + if (resolved.warning !== undefined) { + throw new CliError('CONFIG_INVALID', 'HERMES_HOME must be an absolute path', { + fix: RELATIVE_HERMES_HOME_FIX, + }); + } + return resolved.home; +} + +/** The same resolution, downgraded to a warning plus the `~/.hermes` fallback. */ +export function resolveHermesHomeLenient( + home: string, + env: NodeJS.ProcessEnv, +): HermesHomeResolution { + const configured = env.HERMES_HOME?.trim(); + if (configured === undefined || configured.length === 0) return { home: join(home, '.hermes') }; + if (isAbsolute(configured)) return { home: configured }; + return { + home: join(home, '.hermes'), + warning: `HERMES_HOME is set to a relative path (${configured}) and was ignored; using ~/.hermes. ${RELATIVE_HERMES_HOME_FIX}`, + }; +} + +export function hermesSkillsDir(hermesHome: string): string { + return join(hermesHome, 'skills'); +} + +export function hermesConfigPath(hermesHome: string): string { + return join(hermesHome, 'config.yaml'); +} + +export function hermesPluginDir(hermesHome: string): string { + return join(hermesHome, 'plugins', HERMES_PLUGIN_NAME); +} + +/** Read-only status used by doctor; it never normalizes or writes user YAML. */ +export async function readHermesIntegrationStatus( + hermesHome: string, +): Promise { + const config = await readOptional(hermesConfigPath(hermesHome)); + const lines = config === null ? [] : normalizedLines(config); + const mcpRoot = rootIndexes(lines, 'mcp_servers')[0]; + let mcp: HermesIntegrationStatus['mcp'] = 'missing'; + let mcpCommand: string | undefined; + if (mcpRoot !== undefined) { + const end = topLevelEnd(lines, mcpRoot + 1); + const tenjin = lines.findIndex( + (line, i) => i > mcpRoot && i < end && /^ {2}tenjin:\s*(?:#.*)?$/.test(line), + ); + if (tenjin >= 0) { + const childEnd = blockEnd(lines, tenjin + 1, end, 2); + const managed = lines[tenjin - 1]?.trim() === `# ${HERMES_MCP_MARKER}`; + mcpCommand = managed + ? managedMcpCommand(lines.slice(tenjin, childEnd).join('\n')) + : undefined; + // A managed entry whose baked command has since been deleted is worse than a + // missing one: Hermes fails to start the server and doctor would otherwise + // report green. `command` is `process.argv[1]`, so an npx/dlx cache path or a + // node version switch makes this an ordinary outcome, not an exotic one. + mcp = + mcpCommand === undefined ? 'conflict' : (await exists(mcpCommand)) ? 'configured' : 'stale'; + } + } + const pluginPath = join(hermesPluginDir(hermesHome), '__init__.py'); + const manifestPath = join(hermesPluginDir(hermesHome), 'plugin.yaml'); + const [pluginSource, manifest] = await Promise.all([ + readOptional(pluginPath), + readOptional(manifestPath), + ]); + const plugin = + pluginSource !== null && manifest !== null + ? 'installed' + : pluginSource === null && manifest === null + ? 'missing' + : 'partial'; + const lists = inspectPluginLists(config); + // The conflict verdict comes from the WRITER's planner, never a second model of + // the same YAML. A reader that is more permissive than the writer (an inline + // `plugins.enabled: [x]` reading `not-enabled`) hands out a fix that walks + // straight into a refusal. + const activation: HermesIntegrationStatus['activation'] = lists.disabled + ? 'disabled' + : lists.enabled + ? 'enabled' + : planPluginEnable(config).kind === 'conflict' + ? 'conflict' + : 'not-enabled'; + return { + home: hermesHome, + mcp, + plugin, + activation, + ...(mcpCommand !== undefined ? { mcpCommand } : {}), + }; +} + +/** + * Install Hermes' native plugin and additive MCP entry. + * + * TWO independent decisions gate this, and folding them together is the bug this + * signature exists to prevent. `hooks` is whether the operator consented to hook + * code at all this run: it is the SAME decision that gates Claude's settings.json + * write, so `--no-hooks` must leave the scripts and the plugin unwritten here too, + * exactly as the README's row promises. `explicit` is narrower and is only about + * ACTIVATION: auto-detection may put inert files on disk, but never opts the + * operator into executing third-party code. + * + * The MCP entry is deliberately outside both. It is a server registration, not a + * hook, and it is what `--no-hooks` users still want. + */ +export async function wireHermesIntegration(opts: { + hermesHome: string; + dataDir: string; + tenjinCommand: string; + nodeCommand: string; + dryRun: boolean; + explicit: boolean; + hooks: { enabled: boolean; fix?: string; mode: SearchHookMode }; +}): Promise { + const { hermesHome, dataDir, tenjinCommand, nodeCommand, dryRun, explicit, hooks } = opts; + const mcp = await wireHermesMcp(hermesHome, dryRun, tenjinCommand); + const websearchPath = join(dataDir, 'hooks', WEBSEARCH_HOOK_FILE); + const stopPath = join(dataDir, 'hooks', STOP_HOOK_FILE); + if (!hooks.enabled) { + const pluginDir = hermesPluginDir(hermesHome); + return { + home: hermesHome, + explicit, + mcp, + plugin: { + path: join(pluginDir, '__init__.py'), + manifestPath: join(pluginDir, 'plugin.yaml'), + status: 'skipped', + scriptPaths: [], + warning: `No Hermes hook code was written this run.${ + hooks.fix === undefined ? '' : ` ${hooks.fix}` + }${await survivingPluginNote(hermesHome, hooks.mode)}`, + }, + activation: { path: hermesConfigPath(hermesHome), status: 'skipped' }, + }; + } + // The preview has to name every file a real run would write, not just the two + // this plugin's own hooks call: `writeSharedHookScripts` writes the whole + // shared bundle in one pass (see its docstring), so a dry-run that stopped at + // websearch+stop would under-report what lands on disk. + const shared = dryRun + ? { + written: [ + websearchPath, + join(dataDir, 'hooks', DISPATCH_HOOK_FILE), + join(dataDir, 'hooks', SESSIONSTART_HOOK_FILE), + stopPath, + ], + websearchPath, + stopPath, + } + : await writeSharedHookScripts(dataDir); + const plugin = await wireHermesPlugin({ + hermesHome, + nodeCommand, + websearchPath: shared.websearchPath, + stopPath: shared.stopPath, + dryRun, + scriptPaths: shared.written, + }); + const activation = await wireHermesPluginActivation(hermesHome, dryRun, explicit); + return { home: hermesHome, explicit, mcp, plugin, activation }; +} + +/** + * What an earlier run left behind, when this run wrote nothing. + * + * Withholding the write is not an uninstall, so the warning has to say what is + * still on the machine or an agent reads `skipped` as "off". Whether it still + * RUNS is a separate question: the generated scripts read `hooks.searchMode` on + * every invocation, so an enabled plugin is inert while the stored mode is `off`. + * Inert is the strongest thing on offer: no command in this CLI deletes the plugin + * directory or the `plugins.enabled` entry, so the note must not promise removal. + */ +async function survivingPluginNote(hermesHome: string, mode: SearchHookMode): Promise { + const existing = await readHermesIntegrationStatus(hermesHome); + if (existing.plugin !== 'installed' || existing.activation !== 'enabled') return ''; + const where = `An enabled plugin from an earlier run is still in ${hermesPluginDir(hermesHome)}`; + return mode === 'off' + ? ` ${where}; it stays inert while \`hooks.searchMode\` is off.` + : ` ${where} and keeps running; this run opted out of writing, not out of the plugin. Make it inert with \`tenjin config set hooks.searchMode off\`.`; +} + +export async function wireHermesMcp( + hermesHome: string, + dryRun: boolean, + command: string, +): Promise { + const path = hermesConfigPath(hermesHome); + const existing = await readOptional(path); + const plan = planHermesMcp(existing, command); + if (plan.kind === 'same') return { path, status: 'up-to-date', command: plan.command }; + if (plan.kind === 'conflict') return { path, status: 'conflict', warning: plan.warning }; + if (dryRun) return { path, status: 'would-install', command }; + await writeFileAtomic(path, plan.content, { mode: 0o600, dirMode: 0o700 }); + return { path, status: 'installed', command }; +} + +async function wireHermesPlugin(opts: { + hermesHome: string; + nodeCommand: string; + websearchPath: string; + stopPath: string; + dryRun: boolean; + scriptPaths: string[]; +}): Promise { + const dir = hermesPluginDir(opts.hermesHome); + const path = join(dir, '__init__.py'); + const manifestPath = join(dir, 'plugin.yaml'); + const source = hermesPluginSource(opts.nodeCommand, opts.websearchPath, opts.stopPath); + const manifest = HERMES_PLUGIN_MANIFEST; + const currentSource = await readOptional(path); + const currentManifest = await readOptional(manifestPath); + if (currentSource === source && currentManifest === manifest) { + return { path, manifestPath, status: 'up-to-date', scriptPaths: opts.scriptPaths }; + } + if (opts.dryRun) { + return { path, manifestPath, status: 'would-install', scriptPaths: opts.scriptPaths }; + } + await writeFileAtomic(path, source, { mode: 0o600, dirMode: 0o700 }); + await writeFileAtomic(manifestPath, manifest, { mode: 0o600, dirMode: 0o700 }); + return { path, manifestPath, status: 'installed', scriptPaths: opts.scriptPaths }; +} + +async function wireHermesPluginActivation( + hermesHome: string, + dryRun: boolean, + explicit: boolean, +): Promise { + const path = hermesConfigPath(hermesHome); + const existing = await readOptional(path); + const state = inspectPluginLists(existing); + if (state.disabled) { + return { + path, + status: 'disabled', + warning: + 'Hermes config explicitly disables the Tenjin plugin; that choice was left untouched.', + }; + } + if (state.enabled) return { path, status: 'up-to-date' }; + if (!explicit) { + return { + path, + status: 'disabled', + warning: + 'The native Hermes plugin was installed but not enabled. Enable it with `tenjin install --harness hermes`.', + }; + } + const plan = planPluginEnable(existing); + if (plan.kind === 'conflict') return { path, status: 'conflict', warning: plan.warning }; + if (dryRun) return { path, status: 'would-install' }; + await writeFileAtomic(path, plan.content, { mode: 0o600, dirMode: 0o700 }); + return { path, status: 'installed' }; +} + +type TextPlan = + | { kind: 'same'; command: string } + | { kind: 'write'; content: string } + | { kind: 'conflict'; warning: string }; + +function planHermesMcp(existing: string | null, command: string): TextPlan { + if (existing === null || existing.trim().length === 0) { + return { kind: 'write', content: `mcp_servers:\n${mcpEntry(command)}\n` }; + } + const lines = normalizedLines(existing); + const roots = rootIndexes(lines, 'mcp_servers'); + if (roots.length > 1) return conflict('config.yaml contains duplicate mcp_servers mappings'); + if (roots.length === 0) { + if (lines.some((line) => /^mcp_servers\s*:/.test(line))) { + return conflict('config.yaml uses an unsupported inline mcp_servers value'); + } + return { kind: 'write', content: appendBlock(existing, `mcp_servers:\n${mcpEntry(command)}`) }; + } + const root = roots[0]!; + const end = topLevelEnd(lines, root + 1); + if (!supportedChildren(lines, root + 1, end)) { + return conflict( + 'config.yaml uses unsupported indentation or sequence syntax under mcp_servers', + ); + } + const tenjin = lines.findIndex( + (line, i) => i > root && i < end && /^ {2}tenjin:\s*(?:#.*)?$/.test(line), + ); + if (tenjin < 0 && lines.some((line, i) => i > root && i < end && /^ {2}tenjin\s*:/.test(line))) { + return conflict('config.yaml uses an unsupported inline mcp_servers.tenjin value'); + } + if (tenjin >= 0) { + const childEnd = blockEnd(lines, tenjin + 1, end, 2); + const block = lines.slice(tenjin, childEnd).join('\n'); + const managed = lines[tenjin - 1]?.trim() === `# ${HERMES_MCP_MARKER}`; + const current = managed ? managedMcpCommand(block) : undefined; + if (current === command) return { kind: 'same', command }; + if (current === undefined) { + return conflict('config.yaml already defines mcp_servers.tenjin; it was left untouched'); + } + // Splice FROM the marker, because `mcpEntry` re-emits it: starting at `tenjin` + // would leave the old marker in place and stack a second one every re-point. + // `command` is `process.argv[1]`, so an nvm switch or a pnpm-vs-npm global + // makes re-pointing routine rather than rare. + const next = [...lines.slice(0, tenjin - 1), mcpEntry(command), ...lines.slice(childEnd)].join( + '\n', + ); + return { kind: 'write', content: withFinalNewline(next) }; + } + const next = [...lines.slice(0, end), mcpEntry(command), ...lines.slice(end)].join('\n'); + return { kind: 'write', content: withFinalNewline(next) }; +} + +function planPluginEnable(existing: string | null): Exclude { + if (existing === null || existing.trim().length === 0) { + return { kind: 'write', content: 'plugins:\n enabled:\n - tenjin\n' }; + } + const lines = normalizedLines(existing); + const roots = rootIndexes(lines, 'plugins'); + if (roots.length > 1) return conflict('config.yaml contains duplicate plugins mappings'); + if (roots.length === 0) { + if (lines.some((line) => /^plugins\s*:/.test(line))) { + return conflict('config.yaml uses an unsupported inline plugins value'); + } + return { kind: 'write', content: appendBlock(existing, 'plugins:\n enabled:\n - tenjin') }; + } + const root = roots[0]!; + const end = topLevelEnd(lines, root + 1); + if (!supportedChildren(lines, root + 1, end)) { + return conflict('config.yaml uses unsupported indentation or sequence syntax under plugins'); + } + const enabled = lines.findIndex( + (line, i) => i > root && i < end && /^ {2}enabled:\s*(?:#.*)?$/.test(line), + ); + if (enabled < 0) { + if (lines.some((line, i) => i > root && i < end && /^ {2}enabled\s*:/.test(line))) { + return conflict('config.yaml uses unsupported inline syntax under plugins.enabled'); + } + const next = [ + ...lines.slice(0, root + 1), + ' enabled:', + ' - tenjin', + ...lines.slice(root + 1), + ].join('\n'); + return { kind: 'write', content: withFinalNewline(next) }; + } + const listEnd = blockEnd(lines, enabled + 1, end, 2); + const entries = lines + .slice(enabled + 1, listEnd) + .filter((line) => line.trim() && !/^\s*#/.test(line)); + if (entries.some((line) => !/^ {4}-\s+[^\s].*$/.test(line))) { + return conflict('config.yaml uses unsupported syntax under plugins.enabled'); + } + const next = [...lines.slice(0, listEnd), ' - tenjin', ...lines.slice(listEnd)].join('\n'); + return { kind: 'write', content: withFinalNewline(next) }; +} + +function inspectPluginLists(existing: string | null): { enabled: boolean; disabled: boolean } { + if (existing === null) return { enabled: false, disabled: false }; + const lines = normalizedLines(existing); + const root = rootIndexes(lines, 'plugins')[0]; + if (root === undefined) return { enabled: false, disabled: false }; + const end = topLevelEnd(lines, root + 1); + const has = (key: string): boolean => { + const inline = lines.find( + (line, i) => + i > root && i < end && new RegExp(`^ {2}${key}:\\s*\\[(.*)\\]\\s*(?:#.*)?$`).test(line), + ); + if (inline !== undefined) { + const body = inline.match(/\[(.*)\]/)?.[1] ?? ''; + return body + .split(',') + .map((value) => value.trim().replace(/^["']|["']$/g, '')) + .includes('tenjin'); + } + const start = lines.findIndex( + (line, i) => i > root && i < end && new RegExp(`^ {2}${key}:\\s*(?:#.*)?$`).test(line), + ); + if (start < 0) return false; + const stop = blockEnd(lines, start + 1, end, 2); + return lines + .slice(start + 1, stop) + .some((line) => /^ {4}-\s+["']?tenjin["']?\s*(?:#.*)?$/.test(line)); + }; + return { enabled: has('enabled'), disabled: has('disabled') }; +} + +function hermesPluginSource(nodeCommand: string, websearchPath: string, stopPath: string): string { + return `"""Tenjin native Hermes hooks. Generated by \`tenjin install\`; safe to delete.""" +from __future__ import annotations + +import json +import subprocess +import threading +import time +from collections import OrderedDict + +NODE = ${JSON.stringify(nodeCommand)} +WEBSEARCH_SCRIPT = ${JSON.stringify(websearchPath)} +STOP_SCRIPT = ${JSON.stringify(stopPath)} +_HINTS = OrderedDict() +_LOCK = threading.Lock() +_MAX_HINTS = 128 +_MAX_AGE_SECONDS = 300 + + +def _key(kwargs): + call_id = kwargs.get("tool_call_id") + if call_id: + return "call:" + str(call_id) + return "turn:" + str(kwargs.get("session_id", "")) + ":" + str(kwargs.get("turn_id", "")) + + +def _run(script, payload, timeout): + try: + completed = subprocess.run( + [NODE, script, "--hermes"], + input=json.dumps(payload), + text=True, + capture_output=True, + timeout=timeout, + check=False, + ) + if completed.returncode != 0 or not completed.stdout or len(completed.stdout) > 65536: + return None + parsed = json.loads(completed.stdout) + context = parsed.get("context") if isinstance(parsed, dict) else None + return context if isinstance(context, str) and context else None + except Exception: + return None + + +def _prune(now): + stale = [key for key, (at, _) in _HINTS.items() if now - at > _MAX_AGE_SECONDS] + for key in stale: + _HINTS.pop(key, None) + while len(_HINTS) > _MAX_HINTS: + _HINTS.popitem(last=False) + + +def _pre_tool_call(tool_name="", args=None, **kwargs): + if tool_name != ${JSON.stringify(HERMES_WEB_SEARCH_TOOL)} or not isinstance(args, dict): + return None + context = _run(WEBSEARCH_SCRIPT, {"tool_name": tool_name, "args": args}, 3.0) + if context: + now = time.monotonic() + with _LOCK: + _prune(now) + _HINTS[_key(kwargs)] = (now, context) + return None + + +def _transform_tool_result(tool_name="", result=None, **kwargs): + if tool_name != ${JSON.stringify(HERMES_WEB_SEARCH_TOOL)} or not isinstance(result, str): + return None + with _LOCK: + item = _HINTS.pop(_key(kwargs), None) + if item is None: + return None + return result + "\\n\\n--- Tenjin marketplace context ---\\n" + item[1] + "\\n--- end Tenjin context ---" + + +def _transform_llm_output(response_text="", **kwargs): + if not isinstance(response_text, str): + return None + # STOP_SCRIPT reads these under the same names as Claude's Stop hook payload + # (sessionIdOf/cwdOf in hook-scripts.ts): an empty payload leaves every + # session batched together, which is exactly the once-per-session weak-arm + # nag main's session-scoping (#164) exists to prevent. + payload = {"session_id": kwargs.get("session_id", "")} + cwd = kwargs.get("cwd") + if isinstance(cwd, str): + payload["cwd"] = cwd + context = _run(STOP_SCRIPT, payload, 2.0) + if not context: + return None + return response_text + "\\n\\n--- Tenjin publish-back reminder ---\\n" + context + + +def register(ctx): + ctx.register_hook("pre_tool_call", _pre_tool_call) + ctx.register_hook("transform_tool_result", _transform_tool_result) + ctx.register_hook("transform_llm_output", _transform_llm_output) +`; +} + +function mcpEntry(command: string): string { + return [ + ` # ${HERMES_MCP_MARKER}`, + ' tenjin:', + ` command: ${JSON.stringify(command)}`, + ' args: ["mcp"]', + ].join('\n'); +} + +function managedMcpCommand(block: string): string | undefined { + const wrapped = `\n${block}\n`; + if (!/\n {4}args:\s*\[\s*["']mcp["']\s*\]\s*(?:#.*)?(?:\n|$)/.test(wrapped)) return undefined; + return wrapped.match(/\n {4}command:\s*["']([^"']+)["']\s*(?:#.*)?(?:\n|$)/)?.[1]; +} + +async function readOptional(path: string): Promise { + try { + return await readFile(path, 'utf8'); + } catch (err) { + if (hasCode(err, 'ENOENT')) return null; + throw new CliError('CONFIG_INVALID', `Could not read Hermes file at ${path}`, { + fix: `Check that ${path} is a readable regular file.`, + cause: err, + }); + } +} + +function normalizedLines(text: string): string[] { + return text.replace(/\r\n/g, '\n').split('\n'); +} + +function rootIndexes(lines: string[], key: string): number[] { + return lines.flatMap((line, i) => (new RegExp(`^${key}:\\s*(?:#.*)?$`).test(line) ? [i] : [])); +} + +function topLevelEnd(lines: string[], start: number): number { + for (let i = start; i < lines.length; i += 1) { + const line = lines[i] ?? ''; + if (!line.trim() || /^\s*#/.test(line)) continue; + if (!/^\s/.test(line)) return i; + } + return lines.length; +} + +/** + * One past the last line that belongs to the mapping opened at `start - 1`. + * + * Membership is INDENT, never a sibling-key regex. A probe keyed on a colon does + * not recognize a plain comment as a sibling, which silently makes the next key's + * comment part of THIS block, and a re-point splice then deletes it. Blank lines + * and comments are ambiguous by nature, so they belong only when a deeper line + * still follows; trailing ones stay with whatever comes next. + */ +function blockEnd(lines: string[], start: number, limit: number, indent: number): number { + let end = start; + for (let i = start; i < limit; i += 1) { + const line = lines[i] ?? ''; + if (!line.trim() || /^\s*#/.test(line)) continue; + if (line.length - line.trimStart().length <= indent) return end; + end = i + 1; + } + return end; +} + +async function exists(path: string): Promise { + try { + await stat(path); + return true; + } catch { + return false; + } +} + +function supportedChildren(lines: string[], start: number, end: number): boolean { + const first = lines.slice(start, end).find((line) => line.trim() && !/^\s*#/.test(line)); + return first === undefined || (/^ {2}\S/.test(first) && !/^ {2}-/.test(first)); +} + +function appendBlock(existing: string, block: string): string { + const prefix = existing.endsWith('\n') ? existing : `${existing}\n`; + return `${prefix}\n${block}\n`; +} + +function withFinalNewline(text: string): string { + return text.endsWith('\n') ? text : `${text}\n`; +} + +function conflict(warning: string): { kind: 'conflict'; warning: string } { + return { kind: 'conflict', warning }; +} diff --git a/src/lib/hook-scripts.test.ts b/src/lib/hook-scripts.test.ts index 5c76643..bbbf242 100644 --- a/src/lib/hook-scripts.test.ts +++ b/src/lib/hook-scripts.test.ts @@ -58,19 +58,21 @@ interface HookRun { /** * Write the script and run it exactly as a harness would: stdin in, stdout out. + * `args` carries the harness selector the native adapters pass (`--hermes`), and * `env` is what the launching harness would have exported: the caller handoff for * the User-Agent cases, `TENJIN_PUBLISH_MODE` or `HOME` for the mode cases. */ async function runScript( source: string, stdin: string, + args: string[] = [], env: Record = {}, ): Promise { const path = join(scriptDir, `hook-${Math.random().toString(36).slice(2)}.mjs`); await writeFile(path, source, { mode: 0o755 }); const started = Date.now(); return await new Promise((resolve, reject) => { - const child = spawn(process.execPath, [path], { + const child = spawn(process.execPath, [path, ...args], { stdio: ['pipe', 'pipe', 'pipe'], // A CLEAN environment plus whatever the case sets, rather than an inherited // one. The hook reads `TENJIN_PUBLISH_MODE` and `HOME`, so inheriting would @@ -200,6 +202,26 @@ describe('the update signal on hook output', () => { expect(JSON.parse(run.stdout)).toBeTruthy(); }); + // Two features share one `emit`: the update line and the Hermes envelope. A + // resolution that kept only one of them leaves every Claude-shaped test green. + it('rides inside the Hermes context envelope too', async () => { + const { baseUrl } = await serveJson((_body, base) => ({ status: 200, json: hit(base) })); + await writeConfig({ baseUrl }); + await writeSignal({ current: '0.1.0-alpha.6', latest: '0.1.0-alpha.7' }); + + const run = await runScript( + websearchHookScript(dataDir), + JSON.stringify({ tool_name: 'web_search', args: { query: 'a question' } }), + ['--hermes'], + ); + expect(run.code).toBe(0); + const parsed = JSON.parse(run.stdout) as { context?: string }; + expect(parsed).not.toHaveProperty('hookSpecificOutput'); + expect(parsed.context).toContain( + 'tenjin-cli 0.1.0-alpha.7 is available (you have 0.1.0-alpha.6)', + ); + }); + it('says nothing when no newer version is recorded', async () => { const { baseUrl } = await serveJson((_body, base) => ({ status: 200, json: hit(base) })); await writeConfig({ baseUrl }); @@ -497,7 +519,7 @@ describe('WebSearch hook: the CLI identity on the wire', () => { json: hit(base), })); await writeConfig({ baseUrl }); - await runScript(websearchHookScript(dataDir), webSearchInput('a question'), env); + await runScript(websearchHookScript(dataDir), webSearchInput('a question'), [], env); return userAgents()[0]; }; @@ -572,6 +594,18 @@ function callerComposingTo(length: number): string { } describe('WebSearch hook: modes', () => { + it('uses Hermes web_search input and emits its native context envelope', async () => { + await writeConfig({ hooks: { searchMode: 'remind' } }); + const run = await runScript( + websearchHookScript(dataDir), + JSON.stringify({ tool_name: 'web_search', args: { query: 'a question' } }), + ['--hermes'], + ); + expect(run.code).toBe(0); + expect(run.stderr).toBe(''); + expect(JSON.parse(run.stdout)).toEqual({ context: REMIND_LINE }); + }); + it('remind emits the static line and sends nothing', async () => { const { baseUrl, hits } = await serveJson((_body, base) => ({ status: 200, @@ -696,6 +730,30 @@ describe('Stop hook: open-loop collection', () => { expect(loopLines(run)).toHaveLength(1); }); + // The search half has had a `--hermes` test since the envelope landed; this is + // the publish-back half, and its body is the one #131 rewrote. The Python plugin + // sends a bare `{}` and reads only `context`, so a regression in either the + // envelope or the payload tolerance would silently cost Hermes the whole nag + // while all 30 Claude-shaped stop tests stayed green. + it('emits the Hermes context envelope from the same nag body', async () => { + await seedSearches([OPEN_MISS]); + const run = await runScript(stopHookScript(dataDir), '{}', ['--hermes']); + expect(run.code).toBe(0); + expect(run.stderr).toBe(''); + const parsed = JSON.parse(run.stdout) as { context?: string }; + expect(parsed).not.toHaveProperty('hookSpecificOutput'); + const text = parsed.context ?? ''; + // Not a second copy of the text: the same body Claude gets, current wording and + // all. The publish.mode line is asserted here too, because it is resolved on the + // way into the body: a Hermes envelope that carried the loop line without it + // would be a second, quietly weaker rendering of the same nag. + expect(text).toContain('publish.mode=review: publishing asks first.'); + expect(text).toContain(`'${OPEN_MISS.question}' was a MISS`); + expect(text).toContain(`tenjin publish --search-id ${OPEN_MISS.searchId}`); + expect(text).toContain(`tenjin outcome --search-id ${OPEN_MISS.searchId} --status regenerated`); + expect(text).not.toContain('candidate add'); + }); + it('nags exactly once: the second run is silent', async () => { await seedSearches([OPEN_MISS]); const first = await runScript(stopHookScript(dataDir), stopInput); @@ -1272,7 +1330,7 @@ describe('Stop hook: the resolved publish mode leads the block', () => { await writeConfig({ publish: { mode: 'review' } }); const text = injected( - await runScript(stopHookScript(dataDir), stopInput, { + await runScript(stopHookScript(dataDir), stopInput, [], { TENJIN_PUBLISH_MODE: 'full-auto', }), ) ?? ''; @@ -1317,7 +1375,7 @@ describe('Stop hook: the resolved publish mode leads the block', () => { ); const text = injected( - await runScript(stopHookScript(dataDir), stopIn(project), { + await runScript(stopHookScript(dataDir), stopIn(project), [], { TENJIN_PUBLISH_MODE: 'auto', }), ) ?? ''; @@ -1410,7 +1468,8 @@ describe('Stop hook: the resolved publish mode leads the block', () => { JSON.stringify({ publish: { mode: 'auto' } }), ); const text = - injected(await runScript(stopHookScript(dataDir), stopIn(cwd), { HOME: home })) ?? ''; + injected(await runScript(stopHookScript(dataDir), stopIn(cwd), [], { HOME: home })) ?? + ''; expect(text.split('\n')[0]).toBe('publish.mode=review: publishing asks first.'); } finally { await rm(base, { recursive: true, force: true }); @@ -1437,7 +1496,8 @@ describe('Stop hook: the resolved publish mode leads the block', () => { const cwd = join(home, 'work'); await mkdir(cwd, { recursive: true }); const text = - injected(await runScript(stopHookScript(dataDir), stopIn(cwd), { HOME: home })) ?? ''; + injected(await runScript(stopHookScript(dataDir), stopIn(cwd), [], { HOME: home })) ?? + ''; expect(text.split('\n')[0]).toBe( 'publish.mode=auto: a clean publish proceeds without asking.', ); @@ -1526,7 +1586,7 @@ describe('Stop hook: the resolved publish mode leads the block', () => { it('falls back to the file when the env names no mode', async () => { await seedSearches([OPEN_MISS]); await writeConfig({ publish: { mode: 'auto' } }); - const text = injected(await runScript(stopHookScript(dataDir), stopInput, {})) ?? ''; + const text = injected(await runScript(stopHookScript(dataDir), stopInput, [], {})) ?? ''; expect(text.split('\n')[0]).toBe('publish.mode=auto: a clean publish proceeds without asking.'); }); @@ -1535,7 +1595,9 @@ describe('Stop hook: the resolved publish mode leads the block', () => { await writeConfig({ publish: { mode: 'auto' } }); const text = injected( - await runScript(stopHookScript(dataDir), stopInput, { TENJIN_PUBLISH_MODE: 'whatever' }), + await runScript(stopHookScript(dataDir), stopInput, [], { + TENJIN_PUBLISH_MODE: 'whatever', + }), ) ?? ''; expect(text.split('\n')[0]).toBe('publish.mode=auto: a clean publish proceeds without asking.'); }); diff --git a/src/lib/hook-scripts.ts b/src/lib/hook-scripts.ts index 0b838b2..4fcf3f6 100644 --- a/src/lib/hook-scripts.ts +++ b/src/lib/hook-scripts.ts @@ -144,6 +144,7 @@ import { dirname, join } from 'node:path'; import { homedir } from 'node:os'; const DATA_DIR = ${JSON.stringify(dataDir)}; +const IS_HERMES = process.argv.includes('--hermes'); // The DESIGN budget, not the hard bound. This is an event-loop timer, so it fires // only when the loop is free: a synchronous read that blocks (a FIFO at the config @@ -415,7 +416,10 @@ function emit(hookEventName, additionalContext) { try { const extra = updateLine(); const context = extra === null ? additionalContext : additionalContext + '\\n' + extra; - writeFileSync(1, JSON.stringify({ hookSpecificOutput: { hookEventName, additionalContext: context } })); + const output = IS_HERMES + ? { context } + : { hookSpecificOutput: { hookEventName, additionalContext: context } }; + writeFileSync(1, JSON.stringify(output)); } catch { // A closed or full stdout is not this hook's problem to report. } @@ -742,8 +746,11 @@ async function main() { if (!isRecord(input)) return quiet(); // Defense in depth behind the settings.json matcher: this hook is for WebSearch // and nothing else, and it must never fire on WebFetch. - if (input.tool_name !== 'WebSearch') return quiet(); - const toolInput = isRecord(input.tool_input) ? input.tool_input : {}; + const expectedTool = IS_HERMES ? 'web_search' : 'WebSearch'; + if (input.tool_name !== expectedTool) return quiet(); + const toolInput = IS_HERMES + ? (isRecord(input.args) ? input.args : {}) + : (isRecord(input.tool_input) ? input.tool_input : {}); const question = typeof toolInput.query === 'string' ? toolInput.query.trim() : ''; // A query over the server's cap is not truncated into a different question, it // is simply not looked up. diff --git a/src/lib/skill-heal.test.ts b/src/lib/skill-heal.test.ts index 73f4a04..8c812ea 100644 --- a/src/lib/skill-heal.test.ts +++ b/src/lib/skill-heal.test.ts @@ -73,8 +73,8 @@ function heal(io: Io, env: NodeJS.ProcessEnv = {}): Promise { return healWiredSkills({ io, env, homeDir: home, skillsSourceDir: SKILLS_SRC }); } -const claudeDir = (): string => skillsDirsFor(home)[0]!; -const sharedDir = (): string => skillsDirsFor(home)[1]!; +const claudeDir = (): string => skillsDirsFor(home, join(home, '.hermes'))[0]!; +const sharedDir = (): string => skillsDirsFor(home, join(home, '.hermes'))[1]!; /** What an older build left behind: our frontmatter, someone else's body. */ const stale = (name: string): string => diff --git a/src/lib/skill-heal.ts b/src/lib/skill-heal.ts index b393716..944816c 100644 --- a/src/lib/skill-heal.ts +++ b/src/lib/skill-heal.ts @@ -12,6 +12,7 @@ import { skillsDirsFor, } from './skill-wiring'; import { OPTIONAL_SKILL_NAMES, resolveSkillsSource } from './skills-source'; +import { resolveHermesHomeLenient } from './hermes'; export interface HealDeps { io: Io; @@ -64,7 +65,9 @@ export async function healWiredSkills(deps: HealDeps): Promise { const source = deps.skillsSourceDir ?? packagedSource(); if (source === null) return; - const targets = healable(home); + // Lenient on purpose: an unattended healer is the last place that should + // refuse to run over a stray relative HERMES_HOME. + const targets = healable(home, resolveHermesHomeLenient(home, env).home); if (targets.length === 0) return; await heal(targets, source, deps.io); } catch { @@ -111,9 +114,9 @@ interface Target { * edited under a healthy SKILL.md is restored on the same pass. The other gate, * that the file is ours at all, needs its content and so lives at the write. */ -function healable(home: string): Target[] { +function healable(home: string, hermesHome: string): Target[] { const found: Target[] = []; - for (const dir of skillsDirsFor(home)) { + for (const dir of skillsDirsFor(home, hermesHome)) { if (!isRealDirectory(dir)) continue; for (const name of [...CLI_SKILL_NAMES, ...OPTIONAL_SKILL_NAMES]) { if (!isRealDirectory(join(dir, name))) continue; diff --git a/src/lib/skill-placement.ts b/src/lib/skill-placement.ts index c09af20..085019e 100644 --- a/src/lib/skill-placement.ts +++ b/src/lib/skill-placement.ts @@ -8,6 +8,7 @@ import type { Io } from './output'; import { readSkillFile, skillFrontmatterName, skillsDirsFor } from './skill-wiring'; import { installSkill } from './skill-writer'; import { OPTIONAL_PAY_SKILL, SKILL_NAMES, resolveSkillsSource } from './skills-source'; +import { resolveHermesHomeLenient } from './hermes'; /** * Optional skills, where PRESENCE is the whole mechanism: the tenjin-pay skill @@ -58,7 +59,7 @@ export async function placeOptionalSkill( */ export async function syncBazaarSkill( enabled: boolean, - deps: { io: Io; homeDir?: string; skillsSourceDir?: string }, + deps: { io: Io; homeDir?: string; skillsSourceDir?: string; env?: NodeJS.ProcessEnv }, ): Promise { const home = deps.homeDir ?? homedir(); if (!isAbsolute(home)) return; @@ -69,8 +70,12 @@ export async function syncBazaarSkill( } catch { return; // no readable source: nothing safe to write, nothing to remove FROM } + const env = deps.env ?? process.env; + // Lenient, like `skill-heal` and `uninstall`: a stray relative HERMES_HOME must + // not stop an operator's own `config set bazaarPay`. Resolving it at all is + // what puts the Hermes skills directory in scope for the sync. const touched: string[] = []; - for (const dir of skillsDirsFor(home)) { + for (const dir of skillsDirsFor(home, resolveHermesHomeLenient(home, env).home)) { try { if (lstatSync(dir, { throwIfNoEntry: false })?.isDirectory() !== true) continue; const consented = SKILL_NAMES.some( diff --git a/src/lib/skill-wiring.test.ts b/src/lib/skill-wiring.test.ts index f1c205f..184f102 100644 --- a/src/lib/skill-wiring.test.ts +++ b/src/lib/skill-wiring.test.ts @@ -24,8 +24,11 @@ import { } from './skill-wiring'; let home: string; +/** Required at every call site now, so the tests spell it out the way callers do. */ +let hermesHome: string; beforeEach(async () => { home = await mkdtemp(join(tmpdir(), 'tenjin-wiring-')); + hermesHome = join(home, '.hermes'); }); afterEach(async () => { await rm(home, { recursive: true, force: true }); @@ -119,16 +122,17 @@ describe('harnessFlagFor', () => { it('maps each skills directory to the --harness value that targets it', () => { // A bare `tenjin install` never targets ~/.agents/skills on a Claude-only // machine, so a fix line naming it has to say `--harness shared`. - expect(harnessFlagFor(home, join(home, '.claude', 'skills'))).toBe('claude'); - expect(harnessFlagFor(home, join(home, '.agents', 'skills'))).toBe('shared'); + expect(harnessFlagFor(home, join(home, '.claude', 'skills'), hermesHome)).toBe('claude'); + expect(harnessFlagFor(home, join(home, '.agents', 'skills'), hermesHome)).toBe('shared'); }); }); describe('skillsDirsFor', () => { - it('covers Claude Code and the shared Agent Skills location, in install order', () => { - expect(skillsDirsFor(home)).toEqual([ + it('covers Claude Code, shared Agent Skills, and Hermes in install order', () => { + expect(skillsDirsFor(home, hermesHome)).toEqual([ join(home, '.claude', 'skills'), join(home, '.agents', 'skills'), + join(hermesHome, 'skills'), ]); }); }); @@ -193,50 +197,64 @@ describe('readAllWiring', () => { for (const name of [...CLI_SKILL_NAMES, HOSTED_SKILL_NAME]) { await seed(join(home, '.agents', 'skills'), name); } - const [claude, shared] = await readAllWiring(home); + const [claude, shared] = await readAllWiring(home, hermesHome); expect(cliSkillsWired(claude!)).toBe(false); expect(cliSkillsWired(shared!)).toBe(true); }); }); describe('harness detection', () => { + it('does not confuse a standalone React Native hermes binary for Hermes Agent', () => { + expect(harnessDetectedBy(home, 'hermes', (bin) => bin === 'hermes', hermesHome)).toEqual([]); + }); + + it('detects Hermes Agent from its home and records the binary only as corroboration', async () => { + await mkdir(join(home, '.hermes')); + expect(harnessDetectedBy(home, 'hermes', (bin) => bin === 'hermes', hermesHome)).toEqual([ + 'home-dir', + 'binary', + ]); + }); + const noBinaries = (): boolean => false; it('names both probes: the home dir and the binary', async () => { await mkdir(join(home, '.codex'), { recursive: true }); - expect(harnessDetectedBy(home, 'codex', noBinaries)).toEqual(['home-dir']); - expect(harnessDetectedBy(home, 'claude', (b) => b === 'claude')).toEqual(['binary']); - expect(harnessDetectedBy(home, 'claude', noBinaries)).toEqual([]); + expect(harnessDetectedBy(home, 'codex', noBinaries, hermesHome)).toEqual(['home-dir']); + expect(harnessDetectedBy(home, 'claude', (b) => b === 'claude', hermesHome)).toEqual([ + 'binary', + ]); + expect(harnessDetectedBy(home, 'claude', noBinaries, hermesHome)).toEqual([]); }); it('a directory is only judged when a harness HERE reads it', async () => { - const [claudeDir, sharedDir] = skillsDirsFor(home) as [string, string]; + const [claudeDir, sharedDir] = skillsDirsFor(home, hermesHome) as [string, string]; await mkdir(join(home, '.claude'), { recursive: true }); - const claudeOnly = detectHarnesses(home, noBinaries); - expect(claudeOnly).toEqual({ claude: true, codex: false }); - expect(harnessReads(home, claudeDir, claudeOnly)).toBe(true); + const claudeOnly = detectHarnesses(home, noBinaries, hermesHome); + expect(claudeOnly).toEqual({ claude: true, codex: false, hermes: false }); + expect(harnessReads(home, claudeDir, claudeOnly, hermesHome)).toBe(true); // The leftover-mirror case: nothing here reads ~/.agents/skills. - expect(harnessReads(home, sharedDir, claudeOnly)).toBe(false); + expect(harnessReads(home, sharedDir, claudeOnly, hermesHome)).toBe(false); - const both = detectHarnesses(home, (b) => b === 'codex'); - expect(harnessReads(home, sharedDir, both)).toBe(true); + const both = detectHarnesses(home, (b) => b === 'codex', hermesHome); + expect(harnessReads(home, sharedDir, both, hermesHome)).toBe(true); }); it('with NO harness detected the shared dir is still judged: it is the fallback target', () => { - const [claudeDir, sharedDir] = skillsDirsFor(home) as [string, string]; - const none = detectHarnesses(home, noBinaries); - expect(none).toEqual({ claude: false, codex: false }); - expect(harnessReads(home, claudeDir, none)).toBe(false); - expect(harnessReads(home, sharedDir, none)).toBe(true); + const [claudeDir, sharedDir] = skillsDirsFor(home, hermesHome) as [string, string]; + const none = detectHarnesses(home, noBinaries, hermesHome); + expect(none).toEqual({ claude: false, codex: false, hermes: false }); + expect(harnessReads(home, claudeDir, none, hermesHome)).toBe(false); + expect(harnessReads(home, sharedDir, none, hermesHome)).toBe(true); }); it('harnessTargetDir maps every target the way install writes it', () => { - const [claudeDir, sharedDir] = skillsDirsFor(home) as [string, string]; - expect(harnessTargetDir(home, 'claude')).toBe(claudeDir); + const [claudeDir, sharedDir] = skillsDirsFor(home, hermesHome) as [string, string]; + expect(harnessTargetDir(home, 'claude', hermesHome)).toBe(claudeDir); // Codex and the shared fallback are the same directory, hence one dir, two flags. - expect(harnessTargetDir(home, 'codex')).toBe(sharedDir); - expect(harnessTargetDir(home, 'shared')).toBe(sharedDir); + expect(harnessTargetDir(home, 'codex', hermesHome)).toBe(sharedDir); + expect(harnessTargetDir(home, 'shared', hermesHome)).toBe(sharedDir); }); }); @@ -244,32 +262,34 @@ describe('an explicitly requested harness', () => { const noBinaries = (): boolean => false; it('puts a directory in play that detection alone would skip', async () => { - const [claudeDir, sharedDir] = skillsDirsFor(home) as [string, string]; + const [claudeDir, sharedDir] = skillsDirsFor(home, hermesHome) as [string, string]; await mkdir(join(home, '.claude'), { recursive: true }); - const claudeOnly = detectHarnesses(home, noBinaries); + const claudeOnly = detectHarnesses(home, noBinaries, hermesHome); // `tenjin install --harness shared` on this machine: nothing DETECTED reads the // shared dir, but the user named it, so it is still this machine's business. - expect(harnessReads(home, sharedDir, claudeOnly)).toBe(false); - expect(harnessRequested(home, sharedDir, ['shared'])).toBe(true); - expect(harnessInPlay(home, sharedDir, claudeOnly, ['shared'])).toBe(true); + expect(harnessReads(home, sharedDir, claudeOnly, hermesHome)).toBe(false); + expect(harnessRequested(home, sharedDir, ['shared'], hermesHome)).toBe(true); + expect(harnessInPlay(home, sharedDir, claudeOnly, ['shared'], hermesHome)).toBe(true); // And the record says nothing about the other directory. - expect(harnessRequested(home, claudeDir, ['shared'])).toBe(false); - expect(harnessInPlay(home, claudeDir, claudeOnly, ['shared'])).toBe(true); // detected + expect(harnessRequested(home, claudeDir, ['shared'], hermesHome)).toBe(false); + expect(harnessInPlay(home, claudeDir, claudeOnly, ['shared'], hermesHome)).toBe(true); // detected }); it('a recorded `codex` covers the shared directory it writes to', () => { - const [claudeDir, sharedDir] = skillsDirsFor(home) as [string, string]; - expect(harnessRequested(home, sharedDir, ['codex'])).toBe(true); - expect(harnessRequested(home, claudeDir, ['codex'])).toBe(false); + const [claudeDir, sharedDir] = skillsDirsFor(home, hermesHome) as [string, string]; + expect(harnessRequested(home, sharedDir, ['codex'], hermesHome)).toBe(true); + expect(harnessRequested(home, claudeDir, ['codex'], hermesHome)).toBe(false); }); it('an empty record changes nothing', async () => { - const [claudeDir, sharedDir] = skillsDirsFor(home) as [string, string]; + const [claudeDir, sharedDir] = skillsDirsFor(home, hermesHome) as [string, string]; await mkdir(join(home, '.claude'), { recursive: true }); - const claudeOnly = detectHarnesses(home, noBinaries); + const claudeOnly = detectHarnesses(home, noBinaries, hermesHome); for (const dir of [claudeDir, sharedDir]) { - expect(harnessInPlay(home, dir, claudeOnly, [])).toBe(harnessReads(home, dir, claudeOnly)); + expect(harnessInPlay(home, dir, claudeOnly, [], hermesHome)).toBe( + harnessReads(home, dir, claudeOnly, hermesHome), + ); } }); }); diff --git a/src/lib/skill-wiring.ts b/src/lib/skill-wiring.ts index dd7602b..a8c7231 100644 --- a/src/lib/skill-wiring.ts +++ b/src/lib/skill-wiring.ts @@ -44,8 +44,19 @@ export interface HarnessWiring { state: DirState; } -export function skillsDirsFor(home: string): string[] { - return [join(home, '.claude', 'skills'), join(home, '.agents', 'skills')]; +/** + * `hermesHome` is REQUIRED on every function in this module, and deliberately has + * no `join(home, '.hermes')` default. A default makes a wrong value invisible at + * the call site, and the failure it hides is silent: an unattended caller that + * omits it (`skill-heal`) simply stops covering the Hermes skills directory under + * a custom HERMES_HOME. A missing argument is a compile error instead. + */ +export function skillsDirsFor(home: string, hermesHome: string): string[] { + return [ + join(home, '.claude', 'skills'), + join(home, '.agents', 'skills'), + join(hermesHome, 'skills'), + ]; } /** @@ -55,12 +66,14 @@ export function skillsDirsFor(home: string): string[] { * with, and a second copy of that mapping is exactly the drift this module exists to * prevent. */ -export const HARNESS_TARGETS = ['claude', 'codex', 'shared'] as const; +export const HARNESS_TARGETS = ['claude', 'codex', 'hermes', 'shared'] as const; export type HarnessTarget = (typeof HARNESS_TARGETS)[number]; /** The skills directory a target writes to. `codex` and `shared` share ~/.agents/skills. */ -export function harnessTargetDir(home: string, harness: HarnessTarget): string { - return harness === 'claude' ? join(home, '.claude', 'skills') : join(home, '.agents', 'skills'); +export function harnessTargetDir(home: string, harness: HarnessTarget, hermesHome: string): string { + if (harness === 'claude') return join(home, '.claude', 'skills'); + if (harness === 'hermes') return join(hermesHome, 'skills'); + return join(home, '.agents', 'skills'); } /** @@ -68,8 +81,10 @@ export function harnessTargetDir(home: string, harness: HarnessTarget): string { * ~/.agents/skills by default, so a bare `tenjin install` cannot clear a problem * found there. */ -export function harnessFlagFor(home: string, dir: string): string { - return dir === join(home, '.claude', 'skills') ? 'claude' : 'shared'; +export function harnessFlagFor(home: string, dir: string, hermesHome: string): string { + if (dir === join(home, '.claude', 'skills')) return 'claude'; + if (dir === join(hermesHome, 'skills')) return 'hermes'; + return 'shared'; } /** The harnesses `install` probes for. `shared` is a fallback target, never detected. */ @@ -85,22 +100,33 @@ export function harnessDetectedBy( home: string, harness: DetectableHarness, which: (bin: string) => boolean, + hermesHome: string, ): string[] { const reasons: string[] = []; - if (existsSync(join(home, `.${harness}`))) reasons.push('home-dir'); - if (which(harness)) reasons.push('binary'); + const harnessHome = harness === 'hermes' ? hermesHome : join(home, `.${harness}`); + const hasHome = existsSync(harnessHome); + if (hasHome) reasons.push('home-dir'); + // A common JavaScript engine binary is also named `hermes`; unlike Claude and + // Codex it is not sufficient evidence on its own. Pair it with Hermes' home. + if (which(harness) && (harness !== 'hermes' || hasHome)) reasons.push('binary'); return reasons; } export interface HarnessPresence { claude: boolean; codex: boolean; + hermes: boolean; } -export function detectHarnesses(home: string, which: (bin: string) => boolean): HarnessPresence { +export function detectHarnesses( + home: string, + which: (bin: string) => boolean, + hermesHome: string, +): HarnessPresence { return { - claude: harnessDetectedBy(home, 'claude', which).length > 0, - codex: harnessDetectedBy(home, 'codex', which).length > 0, + claude: harnessDetectedBy(home, 'claude', which, hermesHome).length > 0, + codex: harnessDetectedBy(home, 'codex', which, hermesHome).length > 0, + hermes: harnessDetectedBy(home, 'hermes', which, hermesHome).length > 0, }; } @@ -111,8 +137,16 @@ export function detectHarnesses(home: string, which: (bin: string) => boolean): * shared directory is still in play, because that is the fallback target `install` * writes to, so a half-written fallback install is still reported. */ -export function harnessReads(home: string, dir: string, present: HarnessPresence): boolean { - return harnessFlagFor(home, dir) === 'claude' ? present.claude : present.codex || !present.claude; +export function harnessReads( + home: string, + dir: string, + present: HarnessPresence, + hermesHome: string, +): boolean { + const target = harnessFlagFor(home, dir, hermesHome); + if (target === 'claude') return present.claude; + if (target === 'hermes') return present.hermes; + return present.codex || (!present.claude && !present.hermes); } /** @@ -125,8 +159,9 @@ export function harnessRequested( home: string, dir: string, requested: readonly HarnessTarget[], + hermesHome: string, ): boolean { - return requested.some((h) => harnessTargetDir(home, h) === dir); + return requested.some((h) => harnessTargetDir(home, h, hermesHome) === dir); } /** @@ -139,8 +174,12 @@ export function harnessInPlay( dir: string, present: HarnessPresence, requested: readonly HarnessTarget[], + hermesHome: string, ): boolean { - return harnessReads(home, dir, present) || harnessRequested(home, dir, requested); + return ( + harnessReads(home, dir, present, hermesHome) || + harnessRequested(home, dir, requested, hermesHome) + ); } /** @@ -179,9 +218,9 @@ export async function readHarnessWiring(dir: string): Promise { return { dir, exists, skills, state: classify(skills) }; } -export async function readAllWiring(home: string): Promise { +export async function readAllWiring(home: string, hermesHome: string): Promise { const out: HarnessWiring[] = []; - for (const dir of skillsDirsFor(home)) out.push(await readHarnessWiring(dir)); + for (const dir of skillsDirsFor(home, hermesHome)) out.push(await readHarnessWiring(dir)); return out; } diff --git a/src/lib/uninstall.ts b/src/lib/uninstall.ts index 4af098f..a04e883 100644 --- a/src/lib/uninstall.ts +++ b/src/lib/uninstall.ts @@ -24,6 +24,7 @@ const HOOK_SCRIPT_FILES = [ STOP_HOOK_FILE, ] as const; import { hooksDir } from './paths'; +import { resolveHermesHomeLenient } from './hermes'; import { SHIPPED_SKILL_FILES } from './skills-source'; import { resolveThroughLink } from './skill-writer'; import { @@ -325,10 +326,17 @@ export async function removeHookScripts(dataDir: string): Promise<{ * with their own is not ours to delete just for sitting at our path, and neither * is a directory reached through a symlink. */ -export async function removeSkills(homeDir: string): Promise { +export async function removeSkills( + homeDir: string, + env: NodeJS.ProcessEnv = process.env, +): Promise { const removed: string[] = []; const names = [...CLI_SKILL_NAMES, ...OPTIONAL_SKILL_NAMES, HOSTED_SKILL_NAME]; - for (const dir of skillsDirsFor(homeDir)) { + // Lenient, like `skill-heal`: uninstall is a cleanup command, so a stray + // relative HERMES_HOME must not stop it. Resolving it at all is what puts the + // Hermes skills directory in scope; `skillsDirsFor` requires the argument + // precisely so a new caller cannot quietly leave that directory behind. + for (const dir of skillsDirsFor(homeDir, resolveHermesHomeLenient(homeDir, env).home)) { if (lstatSync(dir, { throwIfNoEntry: false })?.isDirectory() !== true) continue; for (const name of names) { const skillDir = join(dir, name);