feat: Refactor the project into hooks and skills

[leslie1992-dqp]

No description provided.

[Memtensor-AI]

🤖 Open Code Review

Target: PR #38
Task: e772d2252d981f22
Base: main
Head: dqp-memos-cloud-cli

🔍 OpenCodeReview found 40 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.

---

1. .github/scripts/prepare-cli-release.test.mjs (L709-L712)

The error-message regex was not updated to match the new pyproject_toml value. The input was changed to "0.1.0-beta.14" but the expected pattern still matches pyproject_toml=1.0.6, so this assertion will never pass.

Suggested fix:

💡 Suggested Change

Before:

        pyproject_toml: "0.1.0-beta.14",
        python_init: "1.0.7",
      }),
    /pyproject_toml=1\.0\.6/,

After:

        pyproject_toml: "0.1.0-beta.14",
        python_init: "1.0.7",
      }),
    /pyproject_toml=0\.1\.0-beta\.14/,

---

2. .github/workflows/fork-safe-test.yml (L37)

pytest is installed here but the very next step (Run Python unit tests, line 40) still invokes python -m unittest discover. The new test file tests/test_native_hooks.py uses pytest-specific features (@pytest.mark.parametrize, tmp_path fixture). Those tests will be silently skipped or fail when discovered by unittest, so the CI job may pass green while real test failures go undetected. Either change the test runner to python -m pytest tests/ or rewrite the parametrized/fixture-dependent tests to be unittest-compatible.

---

3. src/memos_cli/config.py (L57-L63)

Unrecognized string values fall through to bool(value) instead of returning default. Any non-empty string that doesn't match a known token (e.g. MEMOS_MULTI_VIEW_ENABLED=garbage) evaluates as truthy and silently enables the feature flag instead of respecting the default (False). Move the fallback return inside the isinstance(value, str) branch:

if isinstance(value, str):
    normalized = value.strip().lower()
    if normalized in {"true", "1", "yes", "on"}:
        return True
    if normalized in {"false", "0", "no", "off"}:
        return False
    return default   # unrecognized string → honour default
return bool(value)   # non-str, non-bool, non-None (e.g. int)

---

4. src/memos_cli/commands/hook.py (L25)

agent.strip().lower() is partial normalization. normalize_hook_agent also applies alias remapping (e.g. claude-code → claude, github-copilot → copilot, traecn → trae-cn). If a user passes one of those aliases, the deprecation hint will echo the alias back instead of the canonical name.

Use get_hook_agent_spec(agent).agent to get the fully-normalized canonical name, since _validate_agent already guarantees the call succeeds at this point.

💡 Suggested Change

Before:

    typer.echo(f"`memos hook install` is deprecated; use `memos init --agent {agent.strip().lower()}`.", err=True)

After:

    typer.echo(f"`memos hook install` is deprecated; use `memos init --agent {get_hook_agent_spec(agent).agent}`.", err=True)

---

5. src/memos_cli/commands/hook.py (L36)

Same partial-normalization issue as in install: agent.strip().lower() does not apply the alias → canonical remapping from normalize_hook_agent, so an alias is echoed instead of the canonical name. Replace with get_hook_agent_spec(agent).agent.

💡 Suggested Change

Before:

        f"`memos hook uninstall` is deprecated; use `memos uninstall --agent {agent.strip().lower()} --yes`.",

After:

        f"`memos hook uninstall` is deprecated; use `memos uninstall --agent {get_hook_agent_spec(agent).agent} --yes`.",

---

6. src/memos_cli/commands/init.py (L126-L134)

Dead code: the deepseek block appears twice in succession. The new block (lines 126-129) always returns when DSH_HOME is set and non-blank, making the original block (lines 131-134) unreachable for that path. When DSH_HOME is absent/blank, both blocks run and read the same env var identically. The original block must be removed.

💡 Suggested Change

Before:

    if normalized == "deepseek":
        dsh_home = os.getenv("DSH_HOME")
        if dsh_home and dsh_home.strip():
            return Path(os.path.abspath(Path(dsh_home).expanduser())) / "skills"

    if normalized == "deepseek":
        dsh_home = os.getenv("DSH_HOME")
        if dsh_home and dsh_home.strip():
            return Path(os.path.abspath(Path(dsh_home).expanduser())) / "skills"

After:

    if normalized == "deepseek":
        dsh_home = os.getenv("DSH_HOME")
        if dsh_home and dsh_home.strip():
            return Path(os.path.abspath(Path(dsh_home).expanduser())) / "skills"

---

7. src/memos_cli/commands/init.py (L544-L550)

STANDALONE_FRONTMATTER is shared across all three modes of _build_standalone_guidance (native hook, plugin, and plain CLI), but its description now hard-codes native-hook wording. Agents using standalone format in CLI or plugin mode (e.g. trae, cline) will receive a frontmatter description that falsely claims they are native-hook integrations. The description should be either mode-neutral or constructed dynamically per mode inside _build_standalone_guidance.

💡 Suggested Change

Before:

STANDALONE_FRONTMATTER = """\
---
description: MemOS native hook integration - automatic retrieval and capture are owned by the hook
alwaysApply: true
---

"""

After:

def _build_standalone_guidance(
    agent: str,
    *,
    memos_plugin: bool = False,
    native_hook: bool = False,
) -> str:
    """Build standalone guidance with frontmatter (for Trae rules format)."""
    if native_hook:
        description = "MemOS native hook integration - automatic retrieval and capture are owned by the hook"
        heading = "## MemOS Native Hook Mode"
    elif memos_plugin:
        description = "MemOS memory management (plugin mode) - search and store context for every conversation"
        heading = "## MemOS Plugin Mode"
    else:
        description = "MemOS memory management - search and store context for every conversation"
        heading = "## MemOS CLI"
    frontmatter = f"---\ndescription: {description}\nalwaysApply: true\n---\n\n"
    template = _guidance_template_path().read_text(encoding="utf-8")
    content = _guidance_section(template, heading) or template.rstrip()
    return f"{frontmatter}{content}\n"

---

8. src/memos_cli/commands/init.py (L920-L932)

Silently swallowed rollback failures: both inner except Exception: pass blocks discard any error that occurs during cleanup without logging or notifying the user. If rollback fails, the installation is left in an inconsistent state with no indication to the user. At minimum, print a warning to console so the user knows manual cleanup may be required.

💡 Suggested Change

Before:

    except Exception as exc:
        if native_hook and hook_path is not None:
            try:
                uninstall_hook(agent)
            except Exception:
                pass
            try:
                _install_bundled_skills(agent, native_hook=False)
                _install_agent_guidance(agent, native_hook=False)
            except Exception:
                pass
        console.print(f"\n[red]Error:[/] {exc}")
        raise typer.Exit(1)

After:

    except Exception as exc:
        if native_hook and hook_path is not None:
            try:
                uninstall_hook(agent)
            except Exception as rollback_exc:
                console.print(f"[yellow]Warning:[/] Hook rollback failed: {rollback_exc}")
            try:
                _install_bundled_skills(agent, native_hook=False)
                _install_agent_guidance(agent, native_hook=False)
            except Exception as rollback_exc:
                console.print(f"[yellow]Warning:[/] Skill/guidance rollback failed: {rollback_exc}")
        console.print(f"\n[red]Error:[/] {exc}")
        raise typer.Exit(1)

---

9. src/memos_cli/executable.py (L38-L40)

sys.argv[0] is appended as the highest-priority candidate, but in many invocations it is not a path to a memos binary — e.g. -c, <string>, __main__.py, or a bare module name under python -m. The name filter on line 59 guards against wrong names, but if a file named memos exists in the current working directory it will be returned before the PATH-derived or npm-derived binary.

Consider either requiring the path to be absolute before adding it, or moving it below the shutil.which candidate so authoritative install locations take precedence.

💡 Suggested Change

Before:

    argv0 = sys.argv[0] if sys.argv else ""
    if argv0:
        candidates.append(Path(argv0).expanduser())

After:

    argv0 = sys.argv[0] if sys.argv else ""

    current_bin = Path(sys.executable).resolve().parent
    candidates.extend([current_bin / "memos", current_bin / "memos.exe", current_bin / "memos.js"])

    which_memos = shutil.which("memos")
    if which_memos:
        candidates.append(Path(which_memos).expanduser())

    # argv0 appended after authoritative sources so a CWD-relative path can't shadow them
    if argv0 and Path(argv0).expanduser().is_absolute():
        candidates.append(Path(argv0).expanduser())

---

10. src/memos_cli/executable.py (L29-L30)

Subprocess failures (timeout, permission error, unexpected exit) are silently swallowed. When npm is on PATH but the prefix lookup fails, the caller gets no indication that the npm candidate was skipped, making it hard to diagnose a missing npm-installed binary.

A debug-level log before returning keeps normal output clean while remaining visible under verbose mode.

💡 Suggested Change

Before:

    except (OSError, subprocess.SubprocessError):
        return None

After:

    except (OSError, subprocess.SubprocessError) as exc:
        import logging
        logging.debug("npm prefix -g failed: %s", exc)
        return None

---

11. src/memos_cli/executable.py (L55-L56)

When not running inside a PyInstaller bundle, _bundle_root() returns Path(__file__).resolve().parents[2], which for a standard site-packages install resolves to something like lib/python3.x/ — not a directory that contains a bin/memos. The candidates are harmlessly rejected by is_file(), but the intent diverges from reality for non-bundle installs.

Guard this block so the bundle candidates are only added when sys._MEIPASS is actually set.

💡 Suggested Change

Before:

    bundle_bin = _bundle_root() / "bin"
    candidates.extend([bundle_bin / "memos", bundle_bin / "memos.exe", bundle_bin / "memos.js"])

After:

    if getattr(sys, "_MEIPASS", None):
        bundle_bin = _bundle_root() / "bin"
        candidates.extend([bundle_bin / "memos", bundle_bin / "memos.exe", bundle_bin / "memos.js"])

---

12. src/memos_cli/hooks/host_templates.py (L63-L65)

The generated _run_memos function catches all Exception and logs only at DEBUG level. A fatal misconfiguration — missing executable, wrong MEMOS_ARGV path, permission error — produces a FileNotFoundError or PermissionError that is silently swallowed. The Hermes plugin then returns None from _pre_llm_call, giving the user no memory context and no visible signal that hooks are broken. Consider logging at WARNING or ERROR level when the subprocess cannot even be launched (i.e. the exception is raised before completed is assigned), so the user gets an actionable message.

💡 Suggested Change

Before:

    except Exception:
        logger.debug("MemOS hook failed for %s", event, exc_info=True)
        return {{}}

After:

    except FileNotFoundError:
        logger.warning("MemOS executable not found (%s). Memory hook disabled.", MEMOS_ARGV[0])
        return {{}}
    except Exception:
        logger.debug("MemOS hook failed for %s", event, exc_info=True)
        return {{}}

---

13. src/memos_cli/hooks/host_templates.py (L256-L275)

The single broad try/except Exception wraps stdin reading, JSON parsing, payload normalization, and subprocess execution together. A json.JSONDecodeError on malformed stdin, or any exception from _normalize, is caught and silently replaced with an empty {} output and exit code 0 — indistinguishable from a successful no-op. This makes payload parsing failures completely invisible to callers. Split the block so that JSON/normalization errors are handled (and reported) separately from the subprocess call, or at minimum re-raise non-subprocess errors rather than swallowing them.

💡 Suggested Change

Before:

    try:
        payload = json.loads(sys.stdin.read() or "{{}}")
        normalized = _normalize(payload)
        command = [*MEMOS_ARGV]
        if event:
            command.extend(["--event", event])
        completed = subprocess.run(
            command,
            input=json.dumps(normalized, ensure_ascii=False),
            text=True,
            capture_output=True,
            check=False,
        )
        sys.stdout.write(completed.stdout or "{{}}")
        sys.stderr.write(completed.stderr or "")
        raise SystemExit(completed.returncode)
    except Exception as exc:
        print(f"[memos antigravity adapter] {{exc}}", file=sys.stderr)
        print("{{}}")
        raise SystemExit(0)

After:

    try:
        payload = json.loads(sys.stdin.read() or "{{}}")
    except json.JSONDecodeError as exc:
        print(f"[memos antigravity adapter] bad stdin JSON: {{exc}}", file=sys.stderr)
        print("{{}}")
        raise SystemExit(1)
    try:
        normalized = _normalize(payload)
        command = [*MEMOS_ARGV]
        if event:
            command.extend(["--event", event])
        completed = subprocess.run(
            command,
            input=json.dumps(normalized, ensure_ascii=False),
            text=True,
            capture_output=True,
            check=False,
        )
        sys.stdout.write(completed.stdout or "{{}}")
        sys.stderr.write(completed.stderr or "")
        raise SystemExit(completed.returncode)
    except Exception as exc:
        print(f"[memos antigravity adapter] {{exc}}", file=sys.stderr)
        print("{{}}")
        raise SystemExit(0)

---

14. src/memos_cli/hooks/installer.py (L115-L117)

If os.fdopen(fd, ...) raises before entering the with block (e.g., an out-of-memory error or invalid mode on the platform), the raw file descriptor fd created by mkstemp is never closed. The finally clause only attempts to unlink the temp file by name; it has no visibility into whether fd was transferred to a file object yet.

The standard fix is to guard fd independently so it is closed on any os.fdopen failure:

try:
    handle = os.fdopen(fd, "w", encoding="utf-8")
except BaseException:
    os.close(fd)
    raise
with handle:
    ...

Note: state_store.py's HookStateStore.save uses the same pattern and has the same issue.

💡 Suggested Change

Before:

    fd, temporary_name = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=path.parent)
    try:
        with os.fdopen(fd, "w", encoding="utf-8") as handle:

After:

    fd, temporary_name = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=path.parent)
    try:
        try:
            handle = os.fdopen(fd, "w", encoding="utf-8")
        except BaseException:
            os.close(fd)
            raise
        with handle:
            handle.write(content)
            handle.flush()
            os.fsync(handle.fileno())
        os.chmod(temporary_name, mode)
        os.replace(temporary_name, path)
    finally:
        try:
            os.unlink(temporary_name)
        except FileNotFoundError:
            pass

---

15. src/memos_cli/hooks/installer.py (L348)

When neither python3 nor python is on PATH (Windows, NixOS, minimal containers), this returns the hardcoded string /usr/bin/python3. The fallback is baked silently into the generated adapter script and will produce a broken, non-executable hook at runtime rather than a clear error at install time. Callers have no way to distinguish a resolved interpreter from this placeholder.

Raise HookConfigError when no interpreter is found so the install fails loudly:

💡 Suggested Change

Before:

    return shutil.which("python3") or shutil.which("python") or "/usr/bin/python3"

After:

def _antigravity_adapter_python() -> str:
    """Resolve a Python interpreter for the small local payload adapter."""
    interpreter = shutil.which("python3") or shutil.which("python")
    if interpreter is None:
        raise HookConfigError(
            "Unable to locate a Python interpreter (python3 or python) in PATH; "
            "required to install the Antigravity hook adapter"
        )
    return interpreter

---

16. src/memos_cli/hooks/installer.py (L387-L388)

ANTIGRAVITY_HOOK_NAME is the generic string "memos-memory". Any user-owned cordis plugin that happens to use the same id value will be silently removed when _set_deepseek_plugin_registered is called with registered=False, and will be overwritten when called with registered=True. There is no additional discriminator (e.g., a source field, a comment, or checking the name path) to distinguish a MemOS-managed row from a coincidentally named user plugin.

Add a secondary discriminator to the managed row and check it in _is_managed_row:

💡 Suggested Change

Before:

    def _is_managed_row(row: Any) -> bool:
        return isinstance(row, dict) and row.get("id") == ANTIGRAVITY_HOOK_NAME

After:

    MANAGED_SOURCE = "memos-cli"

    def _is_managed_row(row: Any) -> bool:
        return (
            isinstance(row, dict)
            and row.get("id") == ANTIGRAVITY_HOOK_NAME
            and row.get("_source") == MANAGED_SOURCE
        )

    # ... and when inserting:
    patch.append({"insert": [{"id": ANTIGRAVITY_HOOK_NAME, "name": plugin_path, "_source": MANAGED_SOURCE}]})

---

17. src/memos_cli/hooks/installer.py (L916-L917)

HookStateStore.clear() runs unconditionally via finally, including when an inner uninstall function raises an exception. If, for example, _uninstall_hermes_plugin fails partway through (leaving the hook still installed and active), the state store is wiped. The running hook will then lose its pending turn data for the next session, causing a missed memory capture — while the hook itself continues to run.

Consider only clearing state when uninstallation actually succeeded, or at minimum not clearing it when the try block exits via an exception:

💡 Suggested Change

Before:

    finally:
        HookStateStore(agent=spec.agent).clear()

After:

    removed = False
    try:
        if spec.install_style == "plugin_py":
            removed = bool(_uninstall_hermes_plugin(spec))
            return path if removed else None
        # ... other branches ...
    finally:
        if removed:
            HookStateStore(agent=spec.agent).clear()

---

18. src/memos_cli/hooks/installer.py (L519-L521)

_validate_cline_ide_hook_targets() is called explicitly here and then called a second time inside _install_cline_ide_hooks(argv) at the bottom of this branch. The outer call is redundant: the inner call inside _install_cline_ide_hooks is the one that actually guards the write. If a future refactor removes the outer call (mistakenly believing it is the guard), the check remains; if someone removes the inner call, the outer call here would become the last line of defence without being obviously load-bearing. Ownership of the check should live in one place.

---

19. src/memos_cli/hooks/agents.py (L99-L101)

The guard checks .strip() but constructs Path(...) from the original unstripped string. If the env var contains leading/trailing spaces (e.g., OPENCODE_CONFIG_DIR=' /home/user/.config '), the resulting path embeds those spaces instead of resolving to the intended directory. Strip before passing to Path.

💡 Suggested Change

Before:

configured_dir = os.getenv("OPENCODE_CONFIG_DIR")
            if configured_dir and configured_dir.strip():
                return Path(configured_dir).expanduser() / "plugins" / "memos-memory.js"

After:

configured_dir = os.getenv("OPENCODE_CONFIG_DIR")
            if configured_dir and configured_dir.strip():
                return Path(configured_dir.strip()).expanduser() / "plugins" / "memos-memory.js"

---

20. src/memos_cli/hooks/agents.py (L105-L107)

Same strip-before-Path issue as the opencode branch. The .strip() guard rejects blank strings but the raw (possibly space-padded) value is still passed to Path(os.path.abspath(Path(configured).expanduser())). Use configured.strip() for the path construction.

💡 Suggested Change

Before:

configured = os.getenv("DSH_HOME")
            if configured and configured.strip():
                home = Path(os.path.abspath(Path(configured).expanduser()))

After:

configured = os.getenv("DSH_HOME")
            if configured and configured.strip():
                home = Path(os.path.abspath(Path(configured.strip()).expanduser()))

---

21. src/memos_cli/hooks/agents.py (L303-L304)

Same strip-before-Path issue in the shared helper. The configured string is used as-is for Path(configured).expanduser() even though the guard only checks configured.strip(). Fix: strip the value before constructing the Path.

💡 Suggested Change

Before:

    configured = os.getenv(env_name)
    return Path(configured).expanduser() if configured and configured.strip() else fallback

After:

    configured = os.getenv(env_name)
    return Path(configured.strip()).expanduser() if configured and configured.strip() else fallback

---

22. src/memos_cli/hooks/runner.py (L248-L255)

Both except Exception: blocks in run_stdin silently drop the exception. Every other error-logging call in this file captures exc and includes type(exc).__name__ and the message (e.g. the backend/store calls). Here the diagnosis message carries zero information about why parsing or dispatch failed, making production debugging of malformed payloads very hard.

Capture the exception in both handlers and include the detail in the diagnosis message.

💡 Suggested Change

Before:

    except Exception:
        _diagnose("invalid hook payload; continuing")
        return _emit({})
    try:
        return _emit(run_payload(payload, agent=agent, fallback_event=event))
    except Exception:
        _diagnose("hook failed; continuing")
        return _emit({})

After:

    except Exception as exc:
        _diagnose(f"invalid hook payload ({type(exc).__name__}): {exc}; continuing")
        return _emit({})
    try:
        return _emit(run_payload(payload, agent=agent, fallback_event=event))
    except Exception as exc:
        _diagnose(f"hook failed ({type(exc).__name__}): {exc}; continuing")
        return _emit({})

---

23. src/memos_cli/hooks/runner.py (L140-L143)

Exception detail is silently dropped here. If store.save fails (disk full, permissions error, serialization bug), the diagnosis message contains no information about the cause. More importantly, when this fails, store.consume in the add phase will find nothing, causing the turn's memory to be permanently lost with no useful diagnostics. Capture exc and include it in the message, consistent with the pattern used for backend I/O calls.

💡 Suggested Change

Before:

        try:
            store.save(state)
        except Exception:
            _diagnose("could not persist turn state; continuing")

After:

        except Exception as exc:
            _diagnose(f"could not persist turn state ({type(exc).__name__}): {exc}; continuing")

---

24. src/memos_cli/hooks/runner.py (L235-L236)

The outer try that this except closes wraps virtually the entire add-phase body: store.consume(), the turn_id mismatch nullification (state = None), the early-return guards for cursor/cline, extract_transcript_prompt, is_cancelled, extract_final_answer, and the nested backend/store try blocks. Any exception anywhere in that control-flow logic — including a programming error such as a missing attribute on state, a wrong return type from extract_final_answer, or a logic bug in the cursor/cline guard — is caught and reported as a generic 'could not process turn state', indistinguishable from a transient store failure. The fail-open intent justifies wrapping external I/O, not internal control flow. Logic exceptions should propagate to the outermost run_stdin handler, which is the designated fail-open boundary.

---

25. src/memos_cli/hooks/runner.py (L233-L234)

Exception detail is again silently dropped on the restore path (store.save(state) after a failed transcript read). Capture exc and include type and message for consistency with all other I/O error handling in the file.

💡 Suggested Change

Before:

                    except Exception:
                        _diagnose("could not restore pending turn state")

After:

                    except Exception as exc:
                        _diagnose(f"could not restore pending turn state ({type(exc).__name__}): {exc}")

---

26. src/memos_cli/hooks/runner.py (L151-L153)

This pattern appears in both the search and add phases. set_runtime_options is called with a side-effectful write to what is very likely a module-level singleton (the name implies global runtime state), and config.defaults.framework is mutated on the shared config object passed in by the caller. If run_payload is ever invoked concurrently — e.g. from a multi-threaded test harness, or if a future hook runner processes events in parallel — one call's framework value can be overwritten by another while the first is mid-flight through backend_factory(config). The mutation should use a local copy of config or be passed through the call chain rather than written to shared state.

---

27. src/memos_cli/hooks/runner.py (L221)

When store.consume() returns a state but the turn_id mismatch check nullifies it (state = None, lines above), the fallback for conversation_id is conversation_id_for(payload, spec.agent) re-derived from the add-phase payload. If that payload encodes the session differently than the search-phase payload did (different session field present, different ordering in session_key's priority list), the memory is written under a different conversation_id than the one used during retrieval. This silently corrupts per-conversation memory scoping. At minimum, add a comment confirming that conversation_id_for is deterministic across search and add payloads for the same session.

---

28. src/memos_cli/hooks/runner.py (L45)

aliases comes from a HookAgentSpec which is a frozen dataclass constructed once per agent. The set comprehension rebuilds and lowercases every alias string on every call to _event_matches, which is on the hot path (called for every hook event). Pre-normalizing the aliases at spec construction time (or converting the tuple to a frozenset[str] of already-lowercased values) eliminates the repeated allocation.

💡 Suggested Change

Before:

    return normalized in {alias.strip().lower() for alias in aliases}

After:

    return normalized in {a.strip().lower() for a in aliases}  # pre-normalize at spec build instead

---

29. src/memos_cli/hooks/payload.py (L374-L379)

Path traversal vulnerability: session_id is read directly from the untrusted hook payload and joined into a filesystem path without sanitization. A value such as ../../.ssh/id_rsa would resolve outside the intended session-state/ directory. Resolve both the base and the final path, then assert containment before using it:

base = Path(copilot_home).expanduser().resolve() / "session-state"
candidate = (base / str(session_id).strip()).resolve()
if base in candidate.parents:
    fallback = candidate / "events.jsonl"
else:
    fallback = None
if fallback is not None and fallback not in transcript_paths:
    transcript_paths.append(fallback)

---

30. src/memos_cli/hooks/payload.py (L548-L554)

Broad except Exception: return "" silently swallows all errors from both normalize_search_response and format_memories_markdown with no logging. A bug in either function produces an invisible failure. At minimum log at DEBUG/WARNING level before returning, or narrow the catch to the specific exceptions these helpers can raise.

---

31. src/memos_cli/hooks/payload.py (L524-L529)

When no user message is found in the transcript, last_user remains -1, so materialized[-1 + 1:] equals materialized[0:] — the entire list. The function then returns the last assistant message from the full transcript instead of "". This can surface stale assistant content as a response to a non-existent prompt. Add an explicit guard:

if last_user == -1:
    return ""

---

32. src/memos_cli/hooks/payload.py (L278)

The generator expression's loop variable value shadows the value assigned in the enclosing for name in (...) loop. Python resolves this correctly at runtime, but a reader scanning from the any() call upward will see value in scope and may assume the generator iterates over payload values. Rename the generator variable to avoid the confusion:

return any(kw in reason for kw in ("cancel", "abort", "interrupt", "terminat"))

---

33. src/memos_cli/hooks/payload.py (L419-L422)

A consistently malformed transcript silently produces an empty record list — no diagnostic is emitted. The comment explains the intent but a debug-level log would allow diagnosing host-format regressions without changing normal behavior:

except json.JSONDecodeError:
    logger.debug("Skipping malformed transcript line: %.120s", line)
    continue

---

34. src/memos_cli/hooks/payload.py (L186-L187)

These aliases are defined but not referenced anywhere else in this file. If no external module imports them they are dead code that blurs the module's public API. Remove them, or add an __all__ entry and a comment if they are intentionally exported for backward compatibility.

---

35. src/memos_cli/hooks/state_store.py (L171-L174)

If the process is killed after os.replace(path, temporary_name) succeeds but before the finally: os.unlink(temporary_name) runs, the state file is left at a .consumed-XXXXXX.json path indefinitely. _is_managed_path only matches [0-9a-f]{64}.json, so neither cleanup() nor clear() will ever collect these orphaned files — they accumulate on every crashed or timed-out process.

A simpler approach that avoids the gap: keep the temp name under the managed pattern so cleanup picks it up, or use the deletion-in-finally only after confirming the replace succeeded via a boolean flag.

💡 Suggested Change

Before:

            fd, temporary_name = tempfile.mkstemp(prefix=".consumed-", suffix=".json", dir=self.root)
            os.close(fd)
            os.unlink(temporary_name)
            os.replace(path, temporary_name)

After:

            fd, temporary_name = tempfile.mkstemp(prefix=".consumed-", suffix=".json", dir=self.root)
            os.close(fd)
            os.unlink(temporary_name)
            replaced = False
            try:
                os.replace(path, temporary_name)
                replaced = True
            except FileNotFoundError:
                return None
            # ... read and parse ...

---

36. src/memos_cli/hooks/state_store.py (L142-L143)

Calling cleanup() unconditionally on every load() (and consume()) triggers an O(n) directory scan that reads and JSON-parses every managed state file on every hook event. For a tool that processes a hook call per user turn across many concurrent sessions, this scan grows proportionally to the number of live sessions in the 24-hour TTL window.

Additionally, cleanup() treats any file that raises an exception during json.load() as expired and deletes it. A transient I/O error (e.g., a competing write observed mid-read on some filesystems) on a valid sibling state file would silently destroy that file from a read operation, not a write operation.

Consider decoupling cleanup from reads: run it lazily (e.g., on save() only, or probabilistically, or in a separate maintenance call) rather than on every load()/consume().

---

37. tests/test_init_guidance_paths.py (L349-L353)

Several write_text calls in the new test setup omit encoding="utf-8", while every other write_text in this file specifies it. The content is ASCII-only now so there is no runtime mismatch, but if the platform default encoding is not UTF-8 (e.g. CP1252 on Windows) and the file content ever gains non-ASCII characters, the write would silently produce a different byte sequence than the UTF-8 read performed internally by _install_bundled_skills. Adding encoding="utf-8" aligns with the rest of the file and eliminates the latent risk.

💡 Suggested Change

Before:

            (source / "SKILL.md").write_text("must run memos search and memos add\n")
            (source / "SKILL.native-hook.md").write_text("native hook owns lifecycle\n")
            (references / "memos-add.md").write_text("memos add\n")
            (references / "memos-get.md").write_text("memos get\n")
            (references / "memos-search.md").write_text("memos search\n")

After:

            (source / "SKILL.md").write_text("must run memos search and memos add\n", encoding="utf-8")
            (source / "SKILL.native-hook.md").write_text("native hook owns lifecycle\n", encoding="utf-8")
            (references / "memos-add.md").write_text("memos add\n", encoding="utf-8")
            (references / "memos-get.md").write_text("memos get\n", encoding="utf-8")
            (references / "memos-search.md").write_text("memos search\n", encoding="utf-8")

---

38. tests/test_init_guidance_paths.py (L371-L372)

Same missing encoding="utf-8" in the deepseek test setup for consistency and to guard against platform-default encoding divergence.

💡 Suggested Change

Before:

            (source / "SKILL.md").write_text("skill\n")
            (source / "SKILL.native-hook.md").write_text("native hook owns lifecycle\n")

After:

            (source / "SKILL.md").write_text("skill\n", encoding="utf-8")
            (source / "SKILL.native-hook.md").write_text("native hook owns lifecycle\n", encoding="utf-8")

---

39. tests/test_native_hooks.py (L709-L711)

Dead code: the if agent == "cursor": block is unreachable. The parametrize list for this test contains eight agents (trae, trae-cn, antigravity, hermes, cline, copilot, opencode, openclaw) but not "cursor", so the two assertions inside this guard are never executed. Either add a ("cursor", Path(".cursor/hooks.json")) entry to the parametrize list so these cursor-specific spec properties are actually verified, or remove the dead block entirely.

---

40. tests/test_native_hooks.py (L1015-L1016)

Dead code: yaml_file is always False across all three parametrized cases, so the yaml.safe_load(...) branch is never reached. Remove the yaml_file parameter and both ternaries, replacing them with a plain json.loads(target_path.read_text()) call. If a YAML-config agent was meant to be covered here, it should be added to the parametrize list.

💡 Suggested Change

Before:

    data = yaml.safe_load(target_path.read_text()) if yaml_file else json.loads(target_path.read_text())
    for event in events:

After:

    data = json.loads(target_path.read_text())
    for event in events:

---

🧹 Filtered 1 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 1).

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (109/109 executed). memos_github_open_source/smoke: 1/1, memos_python_core/changed-repo-python: 108/108. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-e772d2252d981f22-20260826145246: 291/291 passed — these do NOT affect the PR verdict; review the branch manually.

Branch: dqp-memos-cloud-cli

[Memtensor-AI]

⚠️ Automated Test Results: NO TEST SCOPE

Automated tests were not run because the changed files do not map to an executable test scope.

Details: No executable test scope maps to the changed files. Automated tests were not run; add env.yaml source_mapping + execution for this path family, then rerun. Changed files: (none detected)
Manual review or env.yaml source_mapping/execution coverage is required before merge.

Branch: dqp-memos-cloud-cli