diff --git a/apps/memos-local-plugin/bridge/hermes-process.ts b/apps/memos-local-plugin/bridge/hermes-process.ts index 6c2384f96..b550a1c2a 100644 --- a/apps/memos-local-plugin/bridge/hermes-process.ts +++ b/apps/memos-local-plugin/bridge/hermes-process.ts @@ -18,22 +18,25 @@ * therefore misses any invocation with a global flag (`--skills`, * `-m`, `--provider`, …) between them. * - * The current pattern is `hermes(?:\s+\S+)*\s+chat\b`: + * The current pattern is `hermes(\s+\S+)*\s+chat\b`: * * • `hermes` — the binary basename. - * • `(?:\s+\S+)*` — any complete argv-style tokens between the + * • `(\s+\S+)*` — any complete argv-style tokens between the * binary and the subcommand. * • `\s+chat\b` — a standalone `chat` token, so it does *not* * match `chatter`, `chat-server`, `--chat-log`, or a flag value * like `--profile=chat`. * * `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. + * `\s`/`\b` as GNU extensions. ⚠️ The pattern MUST stay within POSIX + * ERE — in particular it must NOT use `(?:…)` non-capturing groups, + * which are PCRE-only: glibc ERE rejects the whole pattern with + * "Invalid preceding regular expression", `pgrep` exits 2, and + * `isHermesChatRunning()` silently reports `false`, leaving the + * viewer stuck on `"disconnected"`. A plain capturing group `(…)` + * is valid in both ERE and JavaScript's `RegExp`, so + * `matchesHermesChatCommandLine()` can still proxy the pattern for + * unit tests without a real Hermes binary or a fork of pgrep in CI. */ // eslint-disable-next-line @typescript-eslint/no-require-imports import * as childProcess from "node:child_process"; @@ -45,7 +48,7 @@ import * as childProcess from "node:child_process"; * 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+\\S+)*\\s+chat\\b"; +export const HERMES_CHAT_PROCESS_PATTERN = "hermes(\\s+\\S+)*\\s+chat\\b"; /** * JS-side equivalent of `pgrep -f HERMES_CHAT_PROCESS_PATTERN`. 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 index 2f900c7b0..ddd397d57 100644 --- a/apps/memos-local-plugin/tests/unit/bridge/hermes-process.test.ts +++ b/apps/memos-local-plugin/tests/unit/bridge/hermes-process.test.ts @@ -7,10 +7,14 @@ * subcommand (`hermes --skills memory-routing chat`) was silently * missed and the viewer was stuck on `"disconnected"`. * - * The pattern under test is `hermes(?:\s+\S+)*\s+chat\b` — these cases - * lock in the exact shape of the fix. + * The pattern under test is `hermes(\s+\S+)*\s+chat\b` — these cases + * lock in the exact shape of the fix. It must stay valid POSIX ERE + * (no `(?:…)` groups): glibc ERE rejects non-capturing groups, so a + * PCRE-ism in the pattern makes every `pgrep -f` call fail with a + * regex error and `isHermesChatRunning()` return `false` forever. */ import { describe, expect, it, vi } from "vitest"; +import { spawnSync } from "node:child_process"; import { HERMES_CHAT_PROCESS_PATTERN, @@ -23,7 +27,20 @@ describe("HERMES_CHAT_PROCESS_PATTERN", () => { // 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+\\S+)*\\s+chat\\b"); + expect(HERMES_CHAT_PROCESS_PATTERN).toBe("hermes(\\s+\\S+)*\\s+chat\\b"); + }); + + it("compiles under glibc POSIX ERE — pgrep must not exit 2 (regex error)", () => { + // Regression for the `(?:…)` non-capturing group: JS RegExp accepts + // it, but glibc ERE (what `pgrep -f` uses on Linux) rejects the + // whole pattern with "Invalid preceding regular expression". Run the + // real binary so a PCRE-ism can never silently sneak back in. + // exit 0 = match, 1 = no match (both fine), 2 = regex syntax error. + const result = spawnSync("pgrep", ["-f", HERMES_CHAT_PROCESS_PATTERN], { + encoding: "utf8", + timeout: 2000, + }); + expect(result.status).not.toBe(2); }); });