From a07756f6d7ed2ea2e4e3365381a9570d3eb36577 Mon Sep 17 00:00:00 2001 From: Deshraj Yadav Date: Wed, 29 Jul 2026 02:02:29 -0700 Subject: [PATCH] fix(mem0-agent): accept --session-id after the subcommand The generated manifest writes 'mem0-agent context --session-id $CLAUDE_SESSION_ID', but the flag was only on the top-level parser, so argparse rejected it and every SessionStart hook exited 2 -- the plugin would install and silently do nothing. Both orders now work, and a new test runs every command line in the manifest through the parser so a manifest/CLI mismatch fails the build. Co-Authored-By: Claude Fable 5 --- integrations/mem0-agent/src/mem0_agent/cli.py | 18 +++++++- integrations/mem0-agent/tests/test_cli.py | 43 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/integrations/mem0-agent/src/mem0_agent/cli.py b/integrations/mem0-agent/src/mem0_agent/cli.py index 8723107dd..cdfb33f02 100644 --- a/integrations/mem0-agent/src/mem0_agent/cli.py +++ b/integrations/mem0-agent/src/mem0_agent/cli.py @@ -374,7 +374,23 @@ def cmd_config(args) -> int: def main(argv: list[str] | None = None) -> int: p = argparse.ArgumentParser(prog="mem0-agent", description="Coding-agent memory") p.add_argument("--session-id") - sub = p.add_subparsers(dest="cmd", required=True) + + # Also accepted AFTER the subcommand, which is how hook manifests naturally write it + # ("mem0-agent context --session-id X"). SUPPRESS keeps the subparser from clobbering + # a value that was given before the subcommand. + common = argparse.ArgumentParser(add_help=False) + common.add_argument("--session-id", default=argparse.SUPPRESS, + help="session identifier supplied by the editor") + + _add_parser = p.add_subparsers(dest="cmd", required=True).add_parser + + def add(name: str, **kw): + return _add_parser(name, parents=[common], **kw) + + class _Sub: + add_parser = staticmethod(add) + + sub = _Sub() sp = sub.add_parser("setup", help="apply project configuration") sp.set_defaults(fn=cmd_setup) diff --git a/integrations/mem0-agent/tests/test_cli.py b/integrations/mem0-agent/tests/test_cli.py index 30220534d..15abcdc83 100644 --- a/integrations/mem0-agent/tests/test_cli.py +++ b/integrations/mem0-agent/tests/test_cli.py @@ -139,3 +139,46 @@ def test_hook_manifest_commands_all_exist(): assert sub in known, f"manifest invokes unknown subcommand {sub!r}" found += 1 assert found >= 6 + + +def test_session_id_accepted_on_either_side_of_the_subcommand(monkeypatch): + """The hook manifest writes `mem0-agent context --session-id X`. argparse only + accepts a top-level flag BEFORE the subcommand, so without a per-subcommand copy + every SessionStart hook exits 2 and the plugin silently does nothing.""" + seen = [] + monkeypatch.setattr(cli, "cmd_context", lambda a: seen.append(getattr(a, "session_id", None)) or 0) + cli.main(["context", "--session-id", "AFTER"]) + cli.main(["--session-id", "BEFORE", "context"]) + assert seen == ["AFTER", "BEFORE"] + + +def test_every_manifest_command_parses_verbatim(monkeypatch): + """Every command line in the generated manifest must parse. + + This is the test that would have caught SessionStart exiting 2 on install: + the manifest wrote the global --session-id flag after the subcommand. + """ + import pathlib + import shlex + + manifest = pathlib.Path(__file__).resolve().parents[1] / "hooks/hooks.json" + data = json.loads(manifest.read_text()) + + ran = [] + for name in ("cmd_context", "cmd_observe", "cmd_flush", "cmd_assist_error"): + monkeypatch.setattr(cli, name, lambda a, _n=name: ran.append(_n) or 0) + + checked = 0 + for entries in data["hooks"].values(): + for entry in entries: + for hook in entry.get("hooks", []): + raw = hook["command"].strip("() ").split(">/dev/null")[0] + tokens = shlex.split(raw) + idx = tokens.index("mem0-agent") + argv = [t for t in tokens[idx + 1:] if t != "&"] + # shell vars like "$CLAUDE_SESSION_ID" become a literal in the test + argv = ["session-x" if t.startswith("$") else t for t in argv] + assert cli.main(argv) == 0, f"manifest command did not run: {raw}" + checked += 1 + assert checked >= 6 + assert ran, "the manifest should invoke real subcommands"