fix: remove dead code in PolarDB and redact chat request logs

[fuxicodex]

Summary

Cleaning pass on two files found during a full project audit:

• src/memos/graph_dbs/polardb.py (–597 lines): removes six unreferenced legacy methods (edge_exists_old, get_edges_old, get_neighbors_by_tag_old, get_grouped_counts1, get_all_memory_items_old, get_neighbors_by_tag_ccl) plus unreachable code in drop_database. All of them referenced self.connection, which is never assigned in the class (only self.connection_pool is), so they would raise AttributeError if ever called. __del__ is fixed to close the real connection pool via closeall() instead of checking the phantom self.connection.
• src/memos/api/handlers/chat_handler.py: request logging no longer prints the full Pydantic model, which could leak memory content (query/history/system_prompt) and the business_key auth credential. A whitelisted summary now logs safe fields and masks business_key.
• .gitignore: ignore FuXi CLI local state (.fuxi/).

Why

• Dead code accumulates risk: the _old/_ccl variants silently crash on self.connection.
• Connection pool was never closed on destruction (pool exhaustion risk under long-lived daemon).
• Chat request logs could expose user memory content and business credentials.

Test plan

• python -m py_compile passes for both modified .py files
• AST check: all live methods intact, zero self.connection (non-pool) references remain
• Full-repo grep: deleted methods have no remaining callers
• Functional test of the log helper: masks business_key, excludes query/prompt, keeps whitelist fields (3 cases pass)

Checklist

• Scoped: one logical change set (dead code + log redaction + gitignore)
• Public API behavior unchanged

🤖 Generated with FuXi

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2377
Task: f5f98c6fc820c44e
Base: main
Head: fix/cleanup-dead-code-log-redaction

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

---

1. apps/memos-local-plugin/adapters/deepseek-harness/index.ts (L424-L429)

When error is an Error instance, error.stack already includes the error message as its first line (e.g. Error: <message>\n at ...). Concatenating both message and stack causes the message text to appear twice in the log output.

Consider either logging only the stack (which subsumes the message), or logging only the message (and dropping the stack entirely, which keeps the warning concise).

💡 Suggested Change

Before:

        const message =
          error instanceof Error ? error.message : String(error);
        const stack = error instanceof Error && error.stack ? `\n${error.stack}` : "";
        ctx.logger.warn(
          `memos-local-memory: session event ignored (${event.type}): ${message}${stack}`,
        );

After:

        const message =
          error instanceof Error ? error.message : String(error);
        const stack = error instanceof Error && error.stack ? `\n${error.stack}` : "";
        ctx.logger.warn(
          `memos-local-memory: session event ignored (${event.type}): ${stack || message}`,
        );

---

2. src/memos/api/config.py (L243)

This fix correctly uses lazy %s formatting, but it now causes the full Nacos config dict to be emitted to the log. Nacos properties typically carry secrets (database passwords, API keys, access tokens, etc.). Logging the entire data_props dict risks leaking credentials into log aggregation systems or log files.

The previous form logger.info("nacos config:", data_props) effectively suppressed the dict (extra positional args are silently ignored by logging when there is no matching % placeholder), so this change is a functional behavior change — not just a style fix.

Consider redacting sensitive values before logging, e.g. only log the keys:

logger.info("nacos config keys: %s", list(data_props.keys()))

or mask values for known secret keys.

---

3. src/memos/graph_dbs/polardb.py (L1156-L1164)

Every other cypher() call in this file uses the standard $$ dollar-quote delimiter, but get_path uses a named tag $cypher$. While PostgreSQL itself supports named dollar-quoting, Apache AGE's cypher() function argument is parsed by AGE's own SQL wrapper and may not accept tagged dollar-quotes, which would cause a runtime SyntaxError for this entire code path. Use $$ for consistency and safety.

💡 Suggested Change

Before:

        query = f"""
            SELECT * FROM cypher('{self.db_name}_graph', $cypher$
                MATCH p = (n:Memory {{id: '{source_safe}'}})-[*1..{hops}]-(m:Memory {{id: '{target_safe}'}})
                {user_clause}
                RETURN [x IN nodes(p) | x.id] AS path_ids
                ORDER BY length(p) ASC
                LIMIT 1
            $cypher$) AS (path_ids agtype)
        """

After:

        query = f"""
            SELECT * FROM cypher('{self.db_name}_graph', $$
                MATCH p = (n:Memory {{id: '{source_safe}'}})-[*1..{hops}]-(m:Memory {{id: '{target_safe}'}})
                {user_clause}
                RETURN [x IN nodes(p) | x.id] AS path_ids
                ORDER BY length(p) ASC
                LIMIT 1
            $$) AS (path_ids agtype)
        """

---

4. src/memos/graph_dbs/polardb.py (L66-L69)

The regex [0-9A-Za-z_.\-]* allows zero-length matches, so an empty string passes validation and is returned as ''. In get_neighbors and get_path, when user_name resolves to an empty string (not None), the guard if user_name: is False, so the tenant clause is skipped — that's fine. However, if a caller ever passes id='' or source_id='', the Cypher pattern becomes WHERE a.id = '', silently matching every node with a blank id instead of raising. More critically, the docstring says "Empty values are allowed (they simply never match any node)" — this claim is wrong: '' does match nodes whose id property is an empty string. Consider rejecting empty strings explicitly to prevent silent no-match or cross-tenant surprises.

💡 Suggested Change

Before:

    text = str(value or "")
    if not re.fullmatch(r"[0-9A-Za-z_.\-]*", text):
        raise ValueError(f"Invalid identifier for Cypher embedding: {value!r}")
    return text

After:

    text = str(value) if value is not None else ""
    if not text:
        raise ValueError(f"Identifier must not be empty: {value!r}")
    if not re.fullmatch(r"[0-9A-Za-z_.\-]+", text):
        raise ValueError(f"Invalid identifier for Cypher embedding: {value!r}")
    return text

---

5. src/memos/graph_dbs/polardb.py (L1007-L1009)

The if user_name: truthiness check correctly skips the clause when user_name is None. However, _get_config_value('user_name') can return an empty string '' (e.g. when the config key exists but has a blank value). An empty string is falsy in Python, so the clause is still skipped — but this is fragile: if _cypher_safe_id were ever changed to allow empty strings without raising, the silent no-filter path would persist. Consider an explicit if user_name is not None and user_name != '': check to make the intent unambiguous.

💡 Suggested Change

Before:

        if user_name:
            user_safe = _cypher_safe_id(user_name)
            user_clause = f" AND a.user_name = '{user_safe}'"

After:

        if user_name is not None and user_name != "":
            user_safe = _cypher_safe_id(user_name)
            user_clause = f" AND a.user_name = '{user_safe}'"

---

6. src/memos/graph_dbs/neo4j.py (L870-L876)

In non-multi-db mode, shortestPath finds the globally shortest path between the endpoints and then the WHERE all(x IN nodes(p) WHERE x.user_name = $user_name) predicate is applied afterward. If that shortest path contains any intermediate node from a different tenant, the WHERE clause rejects the whole path and [] is returned — even when a longer same-tenant path exists. The endpoint-level node_filter addresses this only for start/end nodes, not for intermediate hops.

Replace shortestPath with a variable-length MATCH + ORDER BY length + LIMIT 1 so the filter is evaluated over all candidate paths:

MATCH p = (n:Memory {id: $source_id, user_name: $user_name})-[*1..{hops}]-(m:Memory {id: $target_id, user_name: $user_name})
WHERE all(x IN nodes(p) WHERE x.user_name = $user_name)
RETURN [x IN nodes(p) | x.id] AS path_ids
ORDER BY length(p) ASC
LIMIT 1

The developer's own comment in the code acknowledges the post-filter semantics of shortestPath for endpoints, but the same flaw applies to intermediate nodes.

💡 Suggested Change

Before:

        hops = max(1, min(max_depth, 10))
        query = f"""
                MATCH p = shortestPath((n:Memory {{id: $source_id{node_filter}}})-[*1..{hops}]-(m:Memory {{id: $target_id{node_filter}}}))
                {all_filter}
                RETURN [x IN nodes(p) | x.id] AS path_ids
                LIMIT 1
            """

After:

        hops = max(1, min(max_depth, 10))
        if not self.config.use_multi_db:
            query = f"""
                    MATCH p = (n:Memory {{id: $source_id{node_filter}}})-[*1..{hops}]-(m:Memory {{id: $target_id{node_filter}}})
                    {all_filter}
                    RETURN [x IN nodes(p) | x.id] AS path_ids
                    ORDER BY length(p) ASC
                    LIMIT 1
                """
        else:
            query = f"""
                    MATCH p = shortestPath((n:Memory {{id: $source_id}})-[*1..{hops}]-(m:Memory {{id: $target_id}}))
                    RETURN [x IN nodes(p) | x.id] AS path_ids
                    LIMIT 1
                """

---

7. src/memos/graph_dbs/neo4j.py (L870)

max_depth values outside [1, 10] are silently clamped (e.g. 0 becomes 1, 15 becomes 10) with no warning or error. A caller passing max_depth=0 intending "no traversal" or passing a large value and not realising it was truncated will receive a result that is silently different from what was requested. Either validate and raise a ValueError for out-of-range inputs, or at minimum log a warning when clamping occurs.

💡 Suggested Change

Before:

        hops = max(1, min(max_depth, 10))

After:

        if max_depth < 1:
            raise ValueError(f"max_depth must be >= 1, got {max_depth}")
        hops = min(max_depth, 10)
        if hops != max_depth:
            logger.warning("get_path: max_depth=%d exceeds cap of 10; clamped to 10", max_depth)

---

8. src/memos/api/handlers/chat_handler.py (L89-L90)

The truthy check data.get("business_key") treats an empty-string key (business_key="") the same as an absent key, logging None in both cases. This makes it impossible to distinguish "no key provided" from "an empty string was provided", which could hide misconfiguration in logs.

Additionally, the pattern of excluding business_key from _CHAT_REQ_LOG_WHITELIST and then re-adding it in masked form is fragile: if a future maintainer adds business_key to the whitelist (thinking it was accidentally omitted), the dict comprehension on line 88 would already populate safe["business_key"] with the real value before this masking line overwrites it — the masking would still work, but only by accident of ordering.

Consider using a _MASKED_FIELDS set for clarity, and check for None explicitly:

💡 Suggested Change

Before:

        if "business_key" in data:
            safe["business_key"] = "***" if data.get("business_key") else None

After:

        if "business_key" in data:
            safe["business_key"] = "***" if data["business_key"] is not None else None

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (3/3 executed). memos_python_core/changed-python-source: 3/3. Duration: 8s

Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Error details

Tests failed. Failed cases:

Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: All 387 failures occur in the shared authed_viewer fixture at conftest.py:372 before any test logic runs. The viewer login consistently returns 401 'login required', indicating the viewer user account is not present or the authentication configuration no longer accepts the fixture's credentials.
Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Viewer login is returning 401 unauthenticated during test fixture setup in conftest.py, preventing the tests from executing their actual assertions. This is a shared authentication fixture failure affecting all 10 failing tests in the same file.
Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Failed tests:

• test_llm_slot_has_only_whitelisted_keys
• test_embedder_slot_has_only_whitelisted_keys
• test_llm_and_embedder_slots_have_no_sensitive_keys
• test_provider_and_model_are_string_type
• test_provider_and_model_do_not_reflect_script_or_sql
• test_runtime_fields_are_not_overridden_by_disk_config
• test_concurrent_llm_model_provider_stable
• test_concurrent_embedder_model_provider_stable
• test_concurrent_slots_shape_no_torn_read
• test_concurrent_no_field_downgrade_to_null

Error details

Tests failed. Failed cases: test_llm_slot_has_only_whitelisted_keys, test_embedder_slot_has_only_whitelisted_keys, test_llm_and_embedder_slots_have_no_sensitive_keys, test_provider_and_model_are_string_type, test_provider_and_model_do_not_reflect_script_or_sql [advisory, non-gating] AI-generated tests on branch test/auto-gen-6a76389d7ce0b3cd-20260916181613: 89/97 passed, 8 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/cleanup-dead-code-log-redaction

[fuxicodex]

autotest 状态说明（红 ≠ 代码问题）

autotest 目前是 FAILURE，但根据 bot 在 issue 里的留言，失败与本次改动无关：

1. 两次 ENV ISSUE（08:28、09:14）：共享 authed_viewer fixture 登录返回 401，conftest.py:372 处 387 个测试在 fixture 阶段全部失败 —— 是测试环境凭据/账号问题，非代码问题。
2. 10:44 advisory 失败：bot 在临时分支 test/auto-gen-6a76389d7ce0b3cd-20260916181613（现已删除）上自生成测试，8 个失败（llm_slot/embedder_slot 白名单、并发形状测试）。但本仓库不存在 llm_slot/embedder_slot 任何代码——这些是 bot 针对不存在的抽象幻觉出来的测试。留言自己也标注了 "advisory, non-gating — 不影响 PR 判定"。
3. 首次运行（07:23）曾 PASSED。

open-code-review 保持 SUCCESS（10 条 non-blocking finding，本次已处理其中可复现的实质项，见下）。

本次新增修复（commit 93ed838）

针对 OCR 10 条中可复现的实质问题：

• neo4j get_path：租户过滤推进端点 node pattern（user_name: $user_name），避免 cross-tenant 短路路径丢弃真实路径；同时修复 multi-db 模式下引用不存在的 $user_name 参数的隐患。
• polardb get_path：租户过滤从仅两端扩展到全部中间节点（all(x IN nodes(p) ...)），堵住共享图模式下的跨租户路径泄露。
• chat_handler 日志：except AttributeError → hasattr 判断（不再吞 model_dump() 内部异常）；日志 helper 加顶层异常兜底永不抛错；logger.info 改 lazy %s 格式。

get_context_chain（OCR #2/#9）未改：现实现与 postgres 后端（postgres.py:790）一致，属既有行为。

请求：能否忽略不合规的 autotest 噪音并合并？如需重跑 autotest，也请告知。

[fuxicodex]

状态更新：已同步 main，OCR 正在重跑

Branch 已 merge 最新 origin/main（7e6c5238），"此分支与基础分支不兼容" 的提示已解除，GitHub 确认 mergeable。

当前阻塞链说明：

1. open-code-review：此前显示 FAILURE 是 bot 自身执行故障（"execution failed; retry required"），非审查不通过。本次 push 后已在重新运行。OCR 此前提出的 10 条 finding 均为 non-blocking，其中可复现的实质问题已在 93ed838 处理（neo4j/polardb get_path 租户过滤、chat_handler 日志安全），详见我 09-20 的留言。
2. autotest：被 OCR 阻塞排队中，尚未开始。
3. review：仍缺一位有 write 权限的 reviewer approval（REVIEW_REQUIRED）。

参考先例：#2384 同样出现 OCR execution failed，最终正常合并。

如 OCR 重跑再次因 bot 故障报红，烦请 maintainer 帮忙重跑或忽略该项，并安排 approval。谢谢！

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Error details

All 5 tests passed successfully in the memos_python_core/changed-python-source phase with exit code 0. No failures to analyze.

Branch: fix/cleanup-dead-code-log-redaction