diff --git a/apps/memos-local-plugin/bridge.cts b/apps/memos-local-plugin/bridge.cts index 81848acf7..010131ca5 100644 --- a/apps/memos-local-plugin/bridge.cts +++ b/apps/memos-local-plugin/bridge.cts @@ -169,6 +169,9 @@ async function main(): Promise { const { startHttpServer } = (await importEsm( runtimeModule("server/http.ts", "dist/server/http.js") )) as typeof import("./server/http.js"); + const { isHermesChatRunning } = (await importEsm( + runtimeModule("bridge/hermes-process.ts", "dist/bridge/hermes-process.js") + )) as typeof import("./bridge/hermes-process.js"); const rootDir = pluginRoot(); const pkgVersion = require(path.join(rootDir, "package.json")).version; @@ -269,6 +272,7 @@ async function main(): Promise { ? createBridgeStatusTracker( path.join(home.root, BRIDGE_STATUS_FILE), args.daemon, + isHermesChatRunning, ) : null; @@ -565,7 +569,11 @@ function classifyErrorCode(err: unknown): string { return "unknown"; } -function createBridgeStatusTracker(statusFile: string, daemon: boolean): { +function createBridgeStatusTracker( + statusFile: string, + daemon: boolean, + isHermesChatRunning: () => boolean, +): { snapshot(): BridgeStatusSnapshot; markConnected(): void; markDisconnected(message: string): void; @@ -679,18 +687,6 @@ function createBridgeStatusTracker(statusFile: string, daemon: boolean): { }; } -function isHermesChatRunning(): boolean { - try { - const out = childProcess.execFileSync("pgrep", ["-f", "hermes chat"], { - encoding: "utf8", - timeout: 1000, - }); - return out.trim().length > 0; - } catch { - return false; - } -} - void main().catch((err) => { const detail = err instanceof Error ? err.stack ?? err.message : String(err); process.stderr.write( diff --git a/apps/memos-local-plugin/bridge/hermes-process.ts b/apps/memos-local-plugin/bridge/hermes-process.ts new file mode 100644 index 000000000..867029879 --- /dev/null +++ b/apps/memos-local-plugin/bridge/hermes-process.ts @@ -0,0 +1,108 @@ +/** + * Hermes chat process detection. + * + * The viewer daemon (`bridge.cts --daemon`) wants to know whether the + * user has a `hermes chat` session attached to *some* bridge stdio + * process so it can upgrade its own `"disconnected"` status to + * `"reconnecting"` instead of leaving the viewer permanently red. + * + * Implementation: shell out to `pgrep -f `. The regex has to + * cover all three CLI invocation shapes the Hermes CLI supports: + * + * • no flags — `hermes chat` + * • flags after the subcommand — `hermes chat --skills memory-routing` + * • global flags before it — `hermes --skills memory-routing chat` + * + * The third shape is the bug reported in #1915: the previous literal + * pattern `"hermes chat"` requires the two tokens to be contiguous and + * therefore misses any invocation with a global flag (`--skills`, + * `-m`, `--provider`, …) between them. + * + * The current pattern is `hermes\s.*chat\b`: + * + * • `hermes\s` — the binary basename followed by whitespace (the + * separator that always sits between the binary and either the + * first flag or the subcommand). Anchoring on whitespace stops it + * from matching `hermesctl`, `hermes-helper`, etc. + * • `.*` — any flags between the binary and the subcommand + * (empty in the no-flags case). + * • `chat\b` — `chat` ending at a word boundary, so it does *not* + * match `chatter`, `chat-server`, or a flag value like + * `--profile=chats`. + * + * `pgrep -f` on Linux uses glibc's ERE engine, which supports + * `\s`/`\b` as GNU extensions. JavaScript's `RegExp` supports the same + * tokens natively, so this module also exports + * `matchesHermesChatCommandLine()` for unit tests — exercising the + * pattern as a JS regex is a faithful proxy for the pgrep-side + * behaviour without requiring a real Hermes binary or a fork of the + * pgrep process in CI. + */ +// eslint-disable-next-line @typescript-eslint/no-require-imports +import * as childProcess from "node:child_process"; + +/** + * pgrep `-f` pattern used by `isHermesChatRunning`. + * + * Exported separately so callers — and tests — can introspect the exact + * string we hand to `pgrep` and confirm we have not silently regressed + * back to a literal substring match. + */ +export const HERMES_CHAT_PROCESS_PATTERN = "hermes\\s.*chat\\b"; + +/** + * JS-side equivalent of `pgrep -f HERMES_CHAT_PROCESS_PATTERN`. + * + * Used by unit tests to verify the regex matches every documented + * Hermes invocation shape and rejects unrelated command lines. Kept + * deliberately stateless — callers should pass the full + * `/proc//cmdline`-style command-line string. + */ +export function matchesHermesChatCommandLine(commandLine: string): boolean { + return new RegExp(HERMES_CHAT_PROCESS_PATTERN).test(commandLine); +} + +/** + * Shape of the `execFileSync`-compatible helper that + * `isHermesChatRunning` shells out through. Carved out as a named type + * so tests can pass a `vi.fn()` without depending on Node's overloaded + * `ExecFileSyncOptions` union. + */ +export type ExecFileSyncLike = ( + file: string, + args: readonly string[], + options: { encoding: "utf8"; timeout: number }, +) => string; + +const defaultExecFileSync: ExecFileSyncLike = (file, args, options) => + childProcess.execFileSync(file, [...args], options) as unknown as string; + +/** + * Returns `true` when `pgrep -f` finds at least one process whose full + * command line matches `HERMES_CHAT_PROCESS_PATTERN`. + * + * `pgrep` exits non-zero when there is no match, when it cannot be + * found, or on permission errors — all of which we collapse into + * `false` because the caller only uses the boolean to decide whether + * to upgrade `"disconnected"` to `"reconnecting"`. Surfacing the + * difference would just turn a UI hint into a noisy crash path. + * + * `execFile` is overridable so the unit test can inject a stub instead + * of mocking `node:child_process` globally — a Node ESM namespace is + * frozen at import time, so spy-based mocking is brittle. Injection is + * the same dependency pattern the rest of the bridge uses (see + * `startStdioServer`'s `stdin` / `stdout` options). + */ +export function isHermesChatRunning( + execFile: ExecFileSyncLike = defaultExecFileSync, +): boolean { + try { + const out = execFile("pgrep", ["-f", HERMES_CHAT_PROCESS_PATTERN], { + encoding: "utf8", + timeout: 1000, + }); + return out.trim().length > 0; + } catch { + return false; + } +} diff --git a/apps/memos-local-plugin/tests/unit/bridge/hermes-process.test.ts b/apps/memos-local-plugin/tests/unit/bridge/hermes-process.test.ts new file mode 100644 index 000000000..e68054dcd --- /dev/null +++ b/apps/memos-local-plugin/tests/unit/bridge/hermes-process.test.ts @@ -0,0 +1,116 @@ +/** + * Hermes chat process detection — regex + pgrep wrapper. + * + * Regression for #1915: the daemon-mode bridge's `applyStaleRule()` + * used `pgrep -f "hermes chat"` as a literal substring, so any + * invocation with a global flag between the binary and the + * subcommand (`hermes --skills memory-routing chat`) was silently + * missed and the viewer was stuck on `"disconnected"`. + * + * The pattern under test is `hermes\s.*chat\b` — these cases lock in + * the exact shape of the fix. + */ +import { describe, expect, it, vi } from "vitest"; + +import { + HERMES_CHAT_PROCESS_PATTERN, + isHermesChatRunning, + matchesHermesChatCommandLine, +} from "../../../bridge/hermes-process.js"; + +describe("HERMES_CHAT_PROCESS_PATTERN", () => { + it("is the documented #1915 regex (locks in the wire format pgrep sees)", () => { + // If this string ever changes, audit `bridge.cts` callers and the + // issue description before adjusting — the constant is the only + // surface that fixes the substring-detection bug. + expect(HERMES_CHAT_PROCESS_PATTERN).toBe("hermes\\s.*chat\\b"); + }); +}); + +describe("matchesHermesChatCommandLine", () => { + // ─── positive cases (must match) ─────────────────────────────── + it("matches plain `hermes chat` with no flags", () => { + expect( + matchesHermesChatCommandLine("/usr/local/bin/hermes chat"), + ).toBe(true); + }); + + it("matches `hermes chat --skills …` (chat-level flags, the old happy path)", () => { + expect( + matchesHermesChatCommandLine( + "/usr/local/bin/hermes chat --skills memory-routing", + ), + ).toBe(true); + }); + + it("matches `hermes --skills … chat` (global long flag before subcommand) — #1915", () => { + expect( + matchesHermesChatCommandLine( + "/usr/local/lib/hermes-agent/venv/bin/hermes --skills memory-routing chat", + ), + ).toBe(true); + }); + + it("matches `hermes -m gpt-4 chat` (global short flag before subcommand) — #1915", () => { + expect( + matchesHermesChatCommandLine("/usr/local/bin/hermes -m gpt-4 chat"), + ).toBe(true); + }); + + it("matches `hermes --provider openai --skills mem chat` (multiple global flags) — #1915", () => { + expect( + matchesHermesChatCommandLine( + "/usr/local/bin/hermes --provider openai --skills mem chat", + ), + ).toBe(true); + }); + + // ─── negative cases (must NOT match) ─────────────────────────── + it("does not match `hermes status` (different subcommand)", () => { + expect( + matchesHermesChatCommandLine("/usr/local/bin/hermes status"), + ).toBe(false); + }); + + it("does not match `hermes chatter` (chat must end at a word boundary)", () => { + expect( + matchesHermesChatCommandLine("/usr/local/bin/hermes chatter"), + ).toBe(false); + }); + + it("does not match a process with neither `hermes` nor `chat`", () => { + expect(matchesHermesChatCommandLine("/bin/bash -c sleep 5")).toBe(false); + }); + + it("does not match `hermesctl …` (binary name needs a whitespace separator)", () => { + // Catches the foot-gun of dropping the `\s` anchor and matching + // `hermesctl --chat-log=...` against the pattern. + expect( + matchesHermesChatCommandLine("/usr/local/bin/hermesctl --chat-log=foo"), + ).toBe(false); + }); +}); + +describe("isHermesChatRunning", () => { + it("calls pgrep with the documented #1915 regex pattern", () => { + const spy = vi.fn().mockReturnValue("12345\n"); + expect(isHermesChatRunning(spy)).toBe(true); + expect(spy).toHaveBeenCalledWith( + "pgrep", + ["-f", HERMES_CHAT_PROCESS_PATTERN], + { encoding: "utf8", timeout: 1000 }, + ); + }); + + it("returns false when pgrep prints only whitespace (no match)", () => { + const spy = vi.fn().mockReturnValue("\n"); + expect(isHermesChatRunning(spy)).toBe(false); + }); + + it("returns false when pgrep throws (e.g. exit 1 on no match, binary missing)", () => { + const spy = vi.fn().mockImplementation(() => { + throw new Error("Command failed with exit code 1"); + }); + expect(isHermesChatRunning(spy)).toBe(false); + }); +});