From 213665ddbc17c8c191363ecd19d52553cd85e259 Mon Sep 17 00:00:00 2001 From: kiwipaulrob Date: Sun, 2 Aug 2026 18:04:33 +1200 Subject: [PATCH] fix(memos-local-plugin): use POSIX-ERE-compatible pgrep pattern for hermes chat detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #1915 pattern used `(?:\s+\S+)*` — a PCRE non-capturing group. `pgrep -f` on Linux compiles patterns with glibc's POSIX ERE, which has no non-capturing groups, so every call failed with: pgrep: regex error: Invalid preceding regular expression `isHermesChatRunning()` swallows the error and returns `false`, so the daemon viewer was permanently stuck on "disconnected" even with a `hermes chat` session attached. The JS unit test passed because JavaScript RegExp accepts `(?:…)` — the proxy was not faithful for ERE. Replace the non-capturing group with a plain capturing group `(\s+\S+)*`, which is valid in both POSIX ERE and JavaScript RegExp, and update the doc comment to warn that the pattern must stay within ERE. Add a regression test that runs the real `pgrep` binary and asserts it does not exit 2 (regex syntax error), so a PCRE-ism can never silently return. --- .../bridge/hermes-process.ts | 21 +++++++++-------- .../tests/unit/bridge/hermes-process.test.ts | 23 ++++++++++++++++--- 2 files changed, 32 insertions(+), 12 deletions(-) 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); }); });