fix(memos): address review feedback on PRs #1813 and #1817

[chiefmojo]

Summary

Addresses all feedback from the Memtensor-AI reviews on PRs #1813 (HTTP transport + JSON-RPC endpoint) and #1817 (bridge startup race, orphan trace fix, RPC loopback auth). This branch consolidates both PRs plus the review fixes.

Changes

Ship-stoppers

• auth.ts: Added comment documenting the direct-bind assumption on the loopback RPC exemption. If a reverse proxy is ever introduced, socket.remoteAddress reflects the proxy IP and X-Forwarded-For must be consulted instead — auth will bypass silently without this documented.

• __init__.py: Fixed unbound http_bridge / http_bridge_late — the type annotation and constructor were on the same line, so a constructor exception left the variable unbound when the except block called .close(). Extracted _connect_http_bridge() helper which deduplicates the three identical connect-and-register sites and correctly initializes to None before constructing.

• capture.ts + memory-core.ts: Extracted "dirty_reward_rescore" to core/pipeline/recovery-constants.ts as RECOVERY_REASONS.DIRTY_REWARD_RESCORE; both files and the test now import the shared constant. A rename in the recovery path no longer silently breaks the orphan-skip guard in capture.ts.

Bugs

• rpc.ts: Batch notifications were serializing as null (JSON-RPC 2.0 §6 violation). Added type-predicate filter to strip undefined results; all-notifications batch now returns 204 instead of an empty array (also forbidden by §6).

• daemon_manager.py: ss -tlnp is Linux-only — on macOS daemon_pid was always None, causing kill_zombie_bridges to kill every bridge.cjs process including the daemon itself. Added lsof -iTCP:18800 -sTCP:LISTEN macOS fallback; skips killing entirely if PID still cannot be identified.

Should-fix / nits

• daemon_manager.py: import re moved to module level; added startup_lock_active() helper.
• __init__.py: Cold-start re-probe delay is now conditional on startup_lock_active() — skipped on first cold launch.
• .gitignore: Removed redundant bare data entry.
• memory-core.test.ts: Updated assertions to use RECOVERY_REASONS.DIRTY_REWARD_RESCORE.

Related

Addresses review feedback on #1813 and #1817.

🤖 Generated with Claude Code

[Memtensor-AI]

Let me review this PR carefully.

Needs changes — a few issues worth addressing before merge

Overall this is solid work: the unbound-variable fix, the shared constant extraction, and the macOS lsof fallback are all good improvements. A few things stood out:

Issues

1. bridge_client.py — Callable used without import

MemosHttpClient.register_host_handler and on_event/on_log use Callable in type hints, but the diff doesn't show it being imported. The existing MemosBridgeClient may already have it via TYPE_CHECKING, but if Callable isn't in the runtime namespace this will raise NameError at instantiation time.

2. bridge_client.py — entry["event"].set() deleted

The diff ends with the removal of entry["event"].set() at line 422 (the last line of _resolve). That line wakes up the caller blocked in request(). If this deletion is real and not a diff artifact, MemosBridgeClient.request() will hang forever waiting on the event. Please confirm this is just a context boundary in the diff.

3. daemon_manager.py — kill_zombie_bridges race with short-lived PIDs

Between ps aux and os.kill(pid, SIGTERM), the PID could have been recycled. This is a classic TOCTOU issue. Low probability on most systems, but on containers with short PID namespaces it's more realistic. Consider checking /proc/{pid}/cmdline (Linux) or using psutil if available before sending the signal.

4. daemon_manager.py — ss parsing is fragile

The regex r"pid=(\d+)" works for current ss output, but ss -tlnp output format varies across distutils versions. Since you already fall through to lsof, consider trying lsof first on all platforms (it's available on Linux too) for consistency, and only falling back to ss if lsof is missing.

5. __init__.py — viewer_status reassignment inside elif branch

elif viewer_status == "free":
    if startup_lock_active():
        time.sleep(1.0)
        viewer_status = probe_viewer_status()
    if viewer_status == "running_memos":
        ...

If startup_lock_active() is False, the second if checks the original "free" value and correctly skips. That's fine. But if the re-probe returns something other than "running_memos" (e.g. "blocked"), execution falls through to the if self._bridge is None stdio path, which is correct but not obvious. A brief comment would help future readers.

6. rpc.ts — diff is truncated, can't verify the 204 behavior

The description mentions batch notifications now return 204 instead of [], but the rpc.ts diff isn't included. If the response is truly empty (no body), ensure the Content-Type header isn't set to application/json — some HTTP clients choke on 204 + Content-Type: application/json with no body.

Nits

• .gitignore: The comment # ── Runtime / ephemeral (Violet addition 2026-05-27) ── has a future date (2026). Typo?
• __init__.py: The bare except Exception: pass around kill_zombie_bridges() swallows all errors silently. At minimum a logger.debug would help troubleshooting.
• auth.ts diff is also truncated — can't verify the documented loopback assumption comment was actually added.

What looks good

• The _connect_http_bridge() extraction properly initializes to None before construction — exactly the fix needed for the unbound variable bug.
• RECOVERY_REASONS constant extraction is clean and the type export is a nice touch.
• The macOS lsof fallback with the "skip if unknown" safety net is the right call.
• Timeout bump from 15s → 45s with adaptive backoff is pragmatic.

[chiefmojo]

Closing in favor of applying review fixes directly to #1813 and #1817 — keeps each PR focused and reviewable.