Skip to content

Fix/plugin windows runtime lifecycle - #2230

Merged
syzsunshine219 merged 4 commits into
mainfrom
fix/plugin-windows-runtime-lifecycle
Aug 7, 2026
Merged

Fix/plugin windows runtime lifecycle#2230
syzsunshine219 merged 4 commits into
mainfrom
fix/plugin-windows-runtime-lifecycle

Conversation

@Hun-ger

@Hun-ger Hun-ger commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR fixes several Windows-specific configuration, runtime lifecycle, and installation issues in the MemOS local plugin while preserving existing behavior on macOS and Linux.

Problems solved

Implementation approach

  • Added platform-aware runtime-home resolution for Windows:

    • Explicitly configured home paths take precedence.
    • Previously selected paths are persisted through a marker.
    • Existing legacy data is preferred to preserve compatibility with previous versions.
    • The resolved home is explicitly propagated between the provider, bridge, and Viewer processes.
    • Existing SQLite data is not automatically moved.

  • Restored and enforced the adapter-specific runtime ports:

    • Hermes Viewer: 18800
    • OpenClaw Viewer: 18799
    • Missing Hermes configuration is created from the Hermes template with port 18800.
    • Legacy Hermes configurations incorrectly written with port 18799 are migrated once, with a backup created before modification.
    • Runtime configuration responses expose the effective adapter port.

  • Improved Viewer port detection:

    • Supports connection-refused codes 61, 111, and Windows 10061.
    • Uses conservative port states to distinguish available, occupied, unreachable, and unknown conditions.
    • Keeps localhost traffic on a direct connection path.

  • Added a Windows-safe restart flow:

    • Prevents the plugin from terminating itself when no supervisor is available to relaunch it.
    • Provides agent-specific manual restart instructions for Hermes and OpenClaw.
    • Keeps the existing supervised restart behavior on macOS and Linux unchanged.
    • Improves Viewer state reporting so an offline Viewer is distinguishable from a model request failure.

  • Hardened the Windows installer:

    • Uses staged installation and rollback to avoid leaving a partially installed plugin.
    • Treats npm installation and native rebuild failures as fatal.
    • Verifies the native better-sqlite3 module after installation.
    • Preserves existing OpenClaw hooks and unrelated plugin configuration.
    • Avoids writing unsupported plugins.installs entries while remaining compatible with older OpenClaw configurations.

  • Updated the local plugin version to 2.0.14-beta.1.

No new runtime dependencies are introduced by this change.

Related Issues (Required):

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Unit Test
  • Test Script Or Test Steps (please provide)

Automated verification

npm run test:unit
npm run lint
npm run build:package
git diff --check

Results:

  • 158 unit test files passed
  • 1,337 tests passed
  • 1 test skipped
  • TypeScript type checking passed
  • Local npm package build passed
  • Git whitespace validation passed

Windows installation and runtime verification

The generated 2.0.14 npm package was installed on a Windows machine for both Hermes and OpenClaw.

Verified scenarios:

  1. Install and upgrade the local package using the updated PowerShell installer.

  2. Confirm existing Hermes/OpenClaw configuration and OpenClaw hooks are preserved.

  3. Save configuration and restart Hermes.

  4. Confirm Hermes Viewer becomes available at 127.0.0.1:18800.

  5. Restart OpenClaw using the displayed OpenClaw-specific instructions.

  6. Confirm OpenClaw Viewer becomes available at 127.0.0.1:18799.

  7. Validate OpenClaw configuration:

    openclaw config validate --json
openclaw gateway status --deep --require-rpc

  8. Run a real OpenClaw agent turn and verify:

    • The agent produces a response.
    • memos_search succeeds.
    • The agent_end hook triggers memory_add.
    • The memory is stored successfully.

  9. Verify Windows clear-data behavior and confirm data is removed without terminating the Viewer before the response is delivered.

  10. Verify legacy runtime data remains discoverable and no automatic SQLite migration occurs.

Checklist

  • I have performed a self-review of my own code | 我已自行检查了自己的代码
  • I have commented my code in hard-to-understand areas | 我已在难以理解的地方对代码进行了注释
  • I have added tests that prove my fix is effective or that my feature works | 我已添加测试以证明我的修复有效或功能正常
  • I have created related documentation issue/PR in [MemOS-Docs](https://github.com/MemTensor/[MemOS-Docs](https://github.com/MemTensor/MemOS-Docs)) (if applicable) | 我已在 MemOS-Docs 中创建了相关的文档 issue/PR（如果适用）
  • I have linked the issue to this PR (if applicable) | 我已将 issue 链接到此 PR（如果适用）
  • I have mentioned the person who will review this PR | 我已提及将审查此 PR 的人

Reviewer Checklist

@Memtensor-AI Memtensor-AI added area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 6, 2026
@Memtensor-AI

Memtensor-AI commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2230
Task: e245d1faac77ccd5
Base: main
Head: fix/plugin-windows-runtime-lifecycle

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

1. apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py (L2255-L2259)

runtime_home is computed twice with identical expressions (lines 2255 and 2259). The second assignment immediately overwrites the first before any mutation occurs, making the first computation dead. If _resolved_memos_runtime_home() has any side-effect or its return value could differ between calls (e.g., due to a file-system race or env change), the two values could silently diverge and _prepare_shared_bridge would operate on a different path than MemosBridgeClient. Compute it once and reuse.

💡 Suggested Change

Before:

            runtime_home = self._runtime_home or _resolved_memos_runtime_home()
            _prepare_shared_bridge(runtime_home, cleanup_legacy_zombies=True)
            new_bridge: MemosBridgeClient | None = None
            try:
                runtime_home = self._runtime_home or _resolved_memos_runtime_home()

After:

            runtime_home = self._runtime_home or _resolved_memos_runtime_home()
            _prepare_shared_bridge(runtime_home, cleanup_legacy_zombies=True)
            new_bridge: MemosBridgeClient | None = None
            try:
                runtime_env = dict(self._runtime_env or _memos_runtime_env_snapshot(runtime_home))

2. apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py (L1736-L1740)

The user-supplied values["viewer_port"] is silently discarded and replaced with the hard-coded literal 18800. Any value the user or a test passes in is completely ignored. This makes the viewer_port parameter in get_config_specs (default 18800, required=False) a no-op, which is surprising and will confuse callers who expect their value to be persisted. If the port must be fixed, the parameter should either be removed from get_config_specs or the method should validate/reject non-18800 values explicitly rather than silently overwriting them.

💡 Suggested Change

Before:

        if "viewer_port" in values:
            # Keep the legacy setup field for host compatibility, but the
            # Hermes adapter owns :18800. Persist the effective value so the
            # YAML file never advertises a port the runtime will not bind.
            payload["viewer"] = {"port": 18800}

After:

        if "viewer_port" in values:
            # The Hermes adapter is pinned to :18800; reject any other value
            # rather than silently discarding it.
            port = int(values["viewer_port"])
            if port != 18800:
                raise ValueError(f"viewer_port must be 18800, got {port}")
            payload["viewer"] = {"port": 18800}

3. apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py (L403-L405)

_viewer_start_lock already accepts runtime_home=None as its default and handles None internally via (runtime_home or _plugin_root()). The ternary is redundant — passing runtime_home unconditionally is identical in all cases and eliminates a branch that could silently diverge if the signature changes.

💡 Suggested Change

Before:

        lock_context = (
            _viewer_start_lock(runtime_home) if runtime_home is not None else _viewer_start_lock()
        )

After:

        lock_context = _viewer_start_lock(runtime_home)

4. apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py (L334-L346)

The trailing except Exception catch-all swallows any remaining unexpected exception (e.g., from _LOOPBACK_OPENER.open, resp.read, or header access) and silently returns "unknown". This makes it impossible to distinguish a real programming error from a benign transport anomaly. Since every recoverable transport error is already handled by the urllib.error.HTTPError, urllib.error.URLError, and TimeoutError branches above, the bare except Exception is broader than necessary. Consider removing it entirely, or at minimum logging the unexpected error before returning.

💡 Suggested Change

Before:

    except urllib.error.URLError as err:
        reason = getattr(err, "reason", None)
        error_codes = {
            getattr(reason, "errno", None),
            getattr(reason, "winerror", None),
        }
        if isinstance(reason, ConnectionRefusedError) or error_codes & {61, 111, 10061}:
            return "free"
        return "unknown"
    except TimeoutError:
        return "unknown"
    except Exception:
        return "unknown"

After:

    except urllib.error.URLError as err:
        reason = getattr(err, "reason", None)
        error_codes = {
            getattr(reason, "errno", None),
            getattr(reason, "winerror", None),
        }
        if isinstance(reason, ConnectionRefusedError) or error_codes & {61, 111, 10061}:
            return "free"
        return "unknown"
    except TimeoutError:
        return "unknown"
    except Exception:
        logger.debug("MemOS: unexpected error probing %s", url, exc_info=True)
        return "unknown"

5. apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py (L315-L320)

The OSError handler only recognises three numeric codes for address-in-use (48=macOS EADDRINUSE, 98=Linux EADDRINUSE, 10048=Windows WSAEADDRINUSE). A more robust alternative is to check isinstance(err, OSError) and err.errno == errno.EADDRINUSE using the stdlib errno module, which is portable across all platforms and future-proof against new OS mappings. Additionally, error_codes always contains None when winerror is absent on non-Windows, which is harmless but needlessly pollutes the set.

💡 Suggested Change

Before:

    except OSError as err:
        error_codes = {getattr(err, "errno", None), getattr(err, "winerror", None)}
        if error_codes & {48, 98, 10048}:
            return "occupied"
        return "unknown"
    return "free"

After:

    except OSError as err:
        import errno as errno_mod
        if err.errno == errno_mod.EADDRINUSE:
            return "occupied"
        return "unknown"
    return "free"

6. apps/memos-local-plugin/adapters/hermes/memos_provider/daemon_manager.py (L440-L454)

log_handle is opened without a with statement and closed manually at line 454. If subprocess.Popen raises (e.g., FileNotFoundError for the node binary), the except block on line 455 attempts log_handle.close() with a # type: ignore[possibly-undefined] guard — meaning the type checker itself flagged that log_handle may be undefined at that point (if log_file.open() itself raises). The safer pattern is to open the file inside a with block so the OS handle is always released regardless of where the exception originates.

💡 Suggested Change

Before:

                log_handle = log_file.open("a", encoding="utf-8")
                _viewer_process = subprocess.Popen(
                    _bridge_command(daemon=True, runtime_home=runtime_home),
                    cwd=str(plugin_root),
                    stdout=log_handle,
                    stderr=subprocess.STDOUT,
                    stdin=subprocess.DEVNULL,
                    text=True,
                    start_new_session=True,
                    env={
                        **os.environ,
                        **({"MEMOS_HOME": str(runtime_home.resolve())} if runtime_home else {}),
                    },
                )
                log_handle.close()

After:

                with log_file.open("a", encoding="utf-8") as log_handle:
                    _viewer_process = subprocess.Popen(
                        _bridge_command(daemon=True, runtime_home=runtime_home),
                        cwd=str(plugin_root),
                        stdout=log_handle,
                        stderr=subprocess.STDOUT,
                        stdin=subprocess.DEVNULL,
                        text=True,
                        start_new_session=True,
                        env={
                            **os.environ,
                            **({
                                "MEMOS_HOME": str(runtime_home.resolve())
                            } if runtime_home else {}),
                        },
                    )

7. apps/memos-local-plugin/adapters/hermes/memos_provider/runtime_home.py (L100-L101)

This broad catch silently discards all OSError variants, including permission errors (EACCES) that are unrelated to a missing or malformed marker. A permission error here will then likely cause _write_marker to also fail (since marker.parent.mkdir and temp.write_text will hit the same permission barrier), yet the caller receives no diagnostic. At minimum, distinguish FileNotFoundError (expected) from other OSError subtypes and log unexpected ones at DEBUG level so operators can diagnose installation issues.

Suggested approach:

try:
    payload = json.loads(marker.read_text(encoding="utf-8"))
except FileNotFoundError:
    return None
except (OSError, ValueError, TypeError) as err:
    logger.debug("runtime-home marker unreadable (%s): %s", marker, err)
    return None

8. apps/memos-local-plugin/adapters/hermes/memos_provider/runtime_home.py (L117)

Using only os.getpid() as the uniqueness suffix means that if two threads in the same process both reach _write_marker concurrently (e.g., parallel bridge initialisation calls), they produce the identical temp-file path. One thread's write_text will silently truncate the other's in-progress write before replace() is called, which could leave a corrupt or empty JSON file in the marker position.

Add a thread-unique component (e.g., threading.get_ident()) to the suffix:

import threading
temp = marker.with_name(f"{marker.name}.{os.getpid()}.{threading.get_ident()}.tmp")

This is not a concern if callers are always serialised by an external lock, but no such lock is visible at the call sites in __init__.py and bridge_client.py.

9. apps/memos-local-plugin/adapters/hermes/memos_provider/runtime_home.py (L136-L139)

OSError covers both FileNotFoundError (the skills directory simply does not exist — expected) and PermissionError (the directory exists but cannot be listed). In the PermissionError case, returning False silently treats a locked legacy home as having no meaningful data, which causes select_windows_runtime_home to select the canonical install root and potentially direct the application away from a home that does contain user data.

Consider narrowing the catch to FileNotFoundError for the expected case and either re-raising or logging PermissionError:

try:
    return next(skills.iterdir(), None) is not None
except FileNotFoundError:
    return False
except OSError:
    logger.debug("cannot list skills dir %s; assuming no data", skills)
    return False

10. apps/memos-local-plugin/core/config/defaults.ts (L14-L17)

The function silently returns undefined when agent is a known string but not present in FIXED_VIEWER_PORTS. That case is currently unreachable (callers pass agent values read from the CLI, such as "hermes" or "openclaw"), but the type signature accepts any string, so future callers could pass an unrecognised agent and get undefined without any warning. Consider returning undefined only for the expected unknown-agent case and throwing (or at least logging) for truly unexpected values, or document the silent-miss behaviour explicitly in the JSDoc so callers are not surprised.

Additionally — and more critically — the semantics expressed by this function (and the JSDoc comment "own these well-known ports even for legacy YAML files") means that the returned port unconditionally overwrites whatever the user has configured in their viewer.port YAML field (see resolveConfig in index.ts). A user who intentionally changes viewer.port for a hermes or openclaw agent will have that setting silently discarded on every config load. If overriding user config is truly the intended contract, the JSDoc here (and the schema documentation) should call this out explicitly; if not, the override in resolveConfig should be applied only when the port has not been explicitly set by the user (i.e., when it still equals the default).

11. apps/memos-local-plugin/core/config/index.ts (L80-L83)

The effectiveViewerPort override unconditionally replaces merged.viewer.port for any known agent, even if the user has explicitly set a different port in their config.yaml. This silently disregards the user's configuration without any feedback.

Consider either:

  1. Only applying the override when the port in merged matches the default schema value (18799), so a deliberate user-set value is respected.
  2. Or, if the port truly must be locked for agent operation, emit a warning into the warnings array when the override changes an explicitly configured value, so the user is at least informed.

Suggested approach (emit a warning on override):

const viewerPort = effectiveViewerPort(agent);
if (viewerPort !== undefined && isPlainObject(merged.viewer)) {
  if (merged.viewer.port !== undefined && merged.viewer.port !== viewerPort) {
    warnings?.push(
      `viewer.port is locked to ${viewerPort} for agent '${agent}'; ignoring configured value ${merged.viewer.port}`
    );
  }
  merged.viewer.port = viewerPort;
}
💡 Suggested Change

Before:

  const viewerPort = effectiveViewerPort(agent);
  if (viewerPort !== undefined && isPlainObject(merged.viewer)) {
    merged.viewer.port = viewerPort;
  }

After:

  const viewerPort = effectiveViewerPort(agent);
  if (viewerPort !== undefined && isPlainObject(merged.viewer)) {
    if (merged.viewer.port !== undefined && merged.viewer.port !== viewerPort) {
      warnings?.push(
        `viewer.port is locked to ${viewerPort} for agent '${agent}'; ignoring configured value ${merged.viewer.port}`
      );
    }
    merged.viewer.port = viewerPort;
  }

12. apps/memos-local-plugin/core/config/migrations.ts (L91)

Temp file naming collision: process.pid + Date.now() is not unique when atomicWrite is called twice in rapid succession within the same process (which happens in migrateHermesViewerPort — once for home.configFile, once for markerFile). On a fast system both calls can land in the same millisecond, producing the same temp path for two different target files. The second writeFile silently overwrites the first temp file before the first rename completes, potentially writing the marker file's JSON content into config.yaml or vice-versa.

The existing writer.ts uses the same pattern but names the temp file after its specific target (.config.${pid}.${ts}.tmp), making cross-target collision impossible. Apply the same fix here by including the target basename in the temp name, or use a cryptographically random suffix.

Suggested fix:

import { randomBytes } from "node:crypto";
// ...
const tempFile = join(dirname(target), `.${basename(target)}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`);
💡 Suggested Change

Before:

  const tempFile = join(dirname(target), `.${process.pid}.${Date.now()}.tmp`);

After:

  const tempFile = join(dirname(target), `.${basename(target)}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`);

13. apps/memos-local-plugin/core/config/migrations.ts (L76-L79)

Missing delay after stale-lock removal: when the lock directory is stale and gets removed (line 74 fs.rm), the loop immediately continues with no sleep. If another process races to recreate the lock directory in the same instant, this becomes a tight busy-loop for the remainder of the 100 attempts, wasting CPU. Adding a small sleep before the retry (consistent with the 25 ms sleep elsewhere in the loop) prevents this.

💡 Suggested Change

Before:

        if (Date.now() - stat.mtimeMs > 60_000) {
          await fs.rm(lockDir, { recursive: true, force: true });
          continue;
        }

After:

        if (Date.now() - stat.mtimeMs > 60_000) {
          await fs.rm(lockDir, { recursive: true, force: true });
          await new Promise((resolve) => setTimeout(resolve, 25));
          continue;
        }

14. apps/memos-local-plugin/core/config/migrations.ts (L66-L71)

Retry budget misaligned with stale-lock timeout: acquireMigrationLock retries 100 times with a 25 ms sleep each iteration, yielding a maximum wait of ~2.5 seconds before throwing config_write_failed. However the stale-lock threshold is 60 seconds, meaning a legitimate slow migrator (e.g., on slow I/O) holding the lock for longer than 2.5 s will cause callers to fail with a misleading error. Either increase the retry count/sleep to cover at least the stale-lock window, or derive them from a single named constant to keep them in sync.

15. apps/memos-local-plugin/core/config/migrations.ts (L93-L98)

On Windows, fs.rename over an existing target file throws EPERM instead of atomically replacing it (unlike POSIX). atomicWrite does not handle this case, so writing the marker file (which may already exist from a previous attempt) or overwriting home.configFile will fail with an unhandled EPERM on Windows. The installer (install.ps1) confirms Windows is a supported platform. Consider catching EPERM/EXDEV on Windows and falling back to a copy-then-delete, or using a library that handles cross-platform atomic rename.

16. apps/memos-local-plugin/core/config/paths.ts (L95-L96)

If writeRuntimeHomeMarker throws (e.g., disk full, permission denied on installRoot), the exception propagates out of selectWindowsHermesRuntimeHome and then out of resolveHome, crashing the startup even though a perfectly valid selection was already computed. Since persisting the marker is a best-effort optimisation, write failures should be caught and logged as a warning rather than treated as fatal.

💡 Suggested Change

Before:

  if (options.persist !== false) writeRuntimeHomeMarker(markerFile, selection);
  return selection;

After:

  if (options.persist !== false) {
    try {
      writeRuntimeHomeMarker(markerFile, selection);
    } catch {
      // Non-fatal: marker write failed (e.g. disk full, permission denied).
      // The correct home was still selected; a later startup will retry.
    }
  }
  return selection;

17. apps/memos-local-plugin/core/config/paths.ts (L77-L82)

This exception is thrown inside resolveHome, which is called early during startup (e.g. in bootstrapMemoryCoreFull and loadConfigForAgent). Neither call site wraps it in a try/catch, so on a Windows machine that has both databases the process crashes with an unhandled exception and the full filesystem paths (legacyHome, installRoot) are exposed in the error message. Consider either (a) returning the conflict as a structured result so callers can surface it cleanly, or (b) at minimum redacting the paths from the message and documenting that callers must catch.

💡 Suggested Change

Before:

  if (legacyDb && canonicalDb) {
    throw new Error(
      "both Windows Hermes runtime homes contain a database; " +
      `set MEMOS_HOME explicitly (${legacyHome} or ${installRoot})`,
    );
  }

After:

  if (legacyDb && canonicalDb) {
    throw new Error(
      "both Windows Hermes runtime homes contain a memos.db; " +
      "set MEMOS_HOME explicitly to choose one",
    );
  }

18. apps/memos-local-plugin/core/config/paths.ts (L124)

The temp-file name is composed only of PID and millisecond timestamp. On Windows, PIDs are recycled aggressively and Date.now() resolution can be as coarse as 15 ms, so two concurrent startups (e.g. two rapid restarts) can produce the same temp-file name. One process will overwrite the other's in-flight write before renameSync, resulting in a truncated or empty marker file. Use crypto.randomUUID() (available in Node 14.17+, no extra import needed) for a collision-proof suffix.

💡 Suggested Change

Before:

  const tempFile = `${markerFile}.${process.pid}.${Date.now()}.tmp`;

After:

  const { randomUUID } = await import("node:crypto");
  const tempFile = `${markerFile}.${randomUUID()}.tmp`;

19. apps/memos-local-plugin/core/config/paths.ts (L138-L142)

readdirSync can throw for reasons other than the directory not existing — most notably EACCES (permission denied) and ENOTDIR. Silently returning false in all of these cases causes hasMeaningfulRuntimeData to incorrectly report an unreadable skills/ directory as absent, which can steer the runtime to the wrong home without any diagnostic. Only ENOENT should be suppressed.

💡 Suggested Change

Before:

  try {
    return readdirSync(join(root, "skills")).length > 0;
  } catch {
    return false;
  }

After:

  try {
    return readdirSync(join(root, "skills")).length > 0;
  } catch (err) {
    if ((err as NodeJS.ErrnoException).code === "ENOENT") return false;
    throw err;
  }

20. apps/memos-local-plugin/core/config/paths.ts (L113-L116)

pathResolve(value.path) is called after only an isAbsolute check. On Windows, isAbsolute returns true for UNC paths (\\server\share\...), so a crafted marker file can redirect the runtime home to an attacker-controlled network share, enabling data exfiltration or NTLM hash capture. Validate that the resolved path starts with one of the two expected roots (legacyHome or installRoot) before accepting it.

Note: readRuntimeHomeMarker does not currently have access to those root values. Consider adding them as parameters, or performing the allowlist check in selectWindowsHermesRuntimeHome after readRuntimeHomeMarker returns.

21. apps/memos-local-plugin/core/config/paths.ts (L50-L54)

The persist field is optional but defaults to true (write side-effect) when omitted. This is not documented in the interface or the JSDoc of selectWindowsHermesRuntimeHome, which is misleading: callers who pass only { legacyHome, installRoot } expecting a pure selection will unknowingly trigger a file write. Document the default explicitly.

💡 Suggested Change

Before:

interface WindowsHermesHomeOptions {
  legacyHome: string;
  installRoot: string;
  persist?: boolean;
}

After:

interface WindowsHermesHomeOptions {
  legacyHome: string;
  installRoot: string;
  /**
   * When `true` (the default), the resolved home is persisted to the marker
   * file so future startups skip re-detection. Pass `false` for a dry-run
   * selection that causes no side effects.
   */
  persist?: boolean;
}

22. apps/memos-local-plugin/core/config/writer.ts (L66-L72)

When the patch explicitly contains viewer.port, applyPatch has already applied the user-supplied value. This block then silently overwrites it with the fixed Hermes port, discarding the user's intent without any warning or error. If the goal is that Hermes must always use port 18800, the appropriate approach is to validate (and reject with an error) rather than silently mutate. If silent override is intentional, at least log a warning so the user understands why their setting was ignored.

💡 Suggested Change

Before:

  if (
    agent === "hermes" &&
    isPlainObject(patch.viewer) &&
    Object.hasOwn(patch.viewer, "port")
  ) {
    doc.setIn(["viewer", "port"], effectiveViewerPort("hermes"));
  }

After:

  if (
    agent === "hermes" &&
    isPlainObject(patch.viewer) &&
    Object.hasOwn(patch.viewer, "port") &&
    patch.viewer.port !== effectiveViewerPort("hermes")
  ) {
    throw new MemosError(
      "config_invalid",
      `The viewer port for the Hermes agent is fixed to ${effectiveViewerPort("hermes")} and cannot be changed.`,
      { source: home.configFile },
    );
  }

23. apps/memos-local-plugin/core/config/writer.ts (L71)

The string literal "hermes" is hardcoded here, but the enclosing if block already guarantees agent === "hermes". Passing the variable agent instead is more consistent and avoids a subtle divergence if agent is later refined (e.g. a sub-variant string).

💡 Suggested Change

Before:

    doc.setIn(["viewer", "port"], effectiveViewerPort("hermes"));

After:

    doc.setIn(["viewer", "port"], effectiveViewerPort(agent));

24. apps/memos-local-plugin/core/config/writer.ts (L44)

If migrateHermesViewerPort throws (e.g., lock timeout, backup conflict, or fs error), the exception propagates directly from patchConfig with no additional context, making it hard for callers to distinguish a migration failure from a normal config write failure. Consider wrapping the call so the error surface is clear, or at minimum ensuring the error message identifies it as a migration issue.

💡 Suggested Change

Before:

  if (agent === "hermes") await migrateHermesViewerPort(home);

After:

  if (agent === "hermes") {
    try {
      await migrateHermesViewerPort(home);
    } catch (err) {
      throw new MemosError(
        "config_write_failed",
        `Hermes port migration failed, cannot proceed with config write: ${(err as Error).message}`,
        { source: home.configFile, cause: (err as Error).message },
      );
    }
  }

25. apps/memos-local-plugin/core/pipeline/memory-core.ts (L2142-L2146)

The conflict branch (existing.turnKey === turnKey but existing.userText !== turn.userText) sets shouldWriteApiLog = true and leaves apiLogClaim = null, then writes a log row — but never updates the map entry. The map still holds the original userText. Every subsequent call with the same turnKey and the conflicting userText will re-enter this branch, evaluate existing.userText !== turn.userText as true again, and write another log row. The deduplication mechanism is permanently broken for this turnKey after a conflict: the very duplicate rows this change intends to prevent will be created on each retry.

If the conflict is truly rejected upstream (as the comment states), the write at shouldWriteApiLog = true is also superfluous. If it can slip through, the map entry must be updated to the new userText after a successful write so that identical retries are suppressed:

// After a successful write in the conflict case, update the map:
if (shouldWriteApiLog && apiLogWritten && existing) {
  existing.userText = turn.userText;
}

Alternatively, set apiLogClaim in the conflict branch as well so the standard cleanup path handles it consistently.

💡 Suggested Change

Before:

      if (existing?.turnKey === turnKey) {
        // The orchestrator will reject a reused key with different text. Keep
        // that genuine conflict observable while suppressing exact replays.
        shouldWriteApiLog = existing.userText !== turn.userText;
      } else {

After:

      if (existing?.turnKey === turnKey) {
        // The orchestrator will reject a reused key with different text. Keep
        // that genuine conflict observable while suppressing exact replays.
        shouldWriteApiLog = existing.userText !== turn.userText;
        // If the conflict case is allowed through and written, update the map
        // so that an identical retry of the conflicting call is suppressed.
        if (shouldWriteApiLog) {
          existing.userText = turn.userText;
        }
      } else {

26. apps/memos-local-plugin/install.ps1 (L167-L174)

If Prepare-StagedPackage throws, $StagedPrefix is never assigned — PowerShell leaves the variable undefined (or retains a stale value from a previous call in the same session). The catch block then evaluates Test-Path $StagedPrefix against an empty/null string, which PowerShell resolves as Test-Path "" — this either throws or returns false. More critically, on some PowerShell versions a null/empty path passed to Remove-Item can resolve to the current working directory.

Initialize $StagedPrefix = $null before calling Prepare-StagedPackage and add a null-guard in the catch:

if ($StagedPrefix -and (Test-Path $StagedPrefix)) {
    Remove-Item -Recurse -Force $StagedPrefix -ErrorAction SilentlyContinue
}
💡 Suggested Change

Before:

    $StagedPrefix = Prepare-StagedPackage
    $BackupDir = "$Prefix.memos-backup-$([guid]::NewGuid().ToString('N'))"
    $HadExisting = Test-Path $Prefix
    $LiveMovedToBackup = $false
    $StagedMovedLive = $false
    $DeploySucceeded = $false

    try {

After:

    $StagedPrefix = $null
    try {
        $StagedPrefix = Prepare-StagedPackage
    } catch {
        throw
    }
    $BackupDir = "$Prefix.memos-backup-$([guid]::NewGuid().ToString('N'))"
    $HadExisting = Test-Path $Prefix
    $LiveMovedToBackup = $false
    $StagedMovedLive = $false
    $DeploySucceeded = $false

    try {

27. apps/memos-local-plugin/install.ps1 (L208-L216)

The null-path risk for $StagedPrefix mentioned above also applies here. If Prepare-StagedPackage throws before assigning $StagedPrefix, the catch-block Test-Path $StagedPrefix receives a null/empty string.

Add a null guard: if ($StagedPrefix -and (Test-Path $StagedPrefix)).

💡 Suggested Change

Before:

        if ($StagedMovedLive -and (Test-Path $Prefix)) {
            Remove-Item -Recurse -Force $Prefix -ErrorAction SilentlyContinue
        }
        if ($LiveMovedToBackup -and (Test-Path $BackupDir)) {
            Move-Item -Path $BackupDir -Destination $Prefix -Force
        }
        if (Test-Path $StagedPrefix) {
            Remove-Item -Recurse -Force $StagedPrefix -ErrorAction SilentlyContinue
        }

After:

        if ($StagedMovedLive -and (Test-Path $Prefix)) {
            Remove-Item -Recurse -Force $Prefix -ErrorAction SilentlyContinue
        }
        if ($LiveMovedToBackup -and (Test-Path $BackupDir)) {
            Move-Item -Path $BackupDir -Destination $Prefix -Force
        }
        if ($StagedPrefix -and (Test-Path $StagedPrefix)) {
            Remove-Item -Recurse -Force $StagedPrefix -ErrorAction SilentlyContinue
        }

28. apps/memos-local-plugin/install.ps1 (L724-L730)

Passing -ArgumentList as a single pre-escaped string is unreliable on Windows. When $HomeDir or $BridgeEntry contains spaces, parentheses, or ampersands — common in user-profile paths like C:\Users\John Smith\... — the Windows process creation API may mis-tokenize the argument string, causing the daemon to fail silently.

Pass arguments as an array instead, which is how Start-Process is designed to handle them:

💡 Suggested Change

Before:

        if ($BridgeEntry.EndsWith(".cjs")) {
            $DaemonArgs = "`"$BridgeEntry`" --agent=hermes --daemon --home=`"$HomeDir`""
            Start-Process -FilePath $NodeBin -ArgumentList $DaemonArgs -WindowStyle Hidden -RedirectStandardOutput $DaemonLog -RedirectStandardError $DaemonLogErr
        } else {
            $DaemonArgs = "`"$TsxBin`" `"$BridgeEntry`" --agent=hermes --daemon --home=`"$HomeDir`""
            Start-Process -FilePath $NodeBin -ArgumentList $DaemonArgs -WindowStyle Hidden -RedirectStandardOutput $DaemonLog -RedirectStandardError $DaemonLogErr
        }

After:

        if ($BridgeEntry.EndsWith(".cjs")) {
            $DaemonArgs = @($BridgeEntry, "--agent=hermes", "--daemon", "--home=$HomeDir")
            Start-Process -FilePath $NodeBin -ArgumentList $DaemonArgs -WindowStyle Hidden -RedirectStandardOutput $DaemonLog -RedirectStandardError $DaemonLogErr
        } else {
            $DaemonArgs = @($TsxBin, $BridgeEntry, "--agent=hermes", "--daemon", "--home=$HomeDir")
            Start-Process -FilePath $NodeBin -ArgumentList $DaemonArgs -WindowStyle Hidden -RedirectStandardOutput $DaemonLog -RedirectStandardError $DaemonLogErr
        }

29. apps/memos-local-plugin/install.ps1 (L580-L587)

The guard only replaces the hooks object when it is absent, non-object, or an array. It does not check whether allowConversationAccess is already explicitly set to false by host/admin policy. The unconditional assignment on the last line silently overrides a deliberate false, escalating plugin privileges without user awareness.

Preserve an explicit false if one is already present:

💡 Suggested Change

Before:

    if (
      !config.plugins.entries[pluginId].hooks ||
      typeof config.plugins.entries[pluginId].hooks !== 'object' ||
      Array.isArray(config.plugins.entries[pluginId].hooks)
    ) {
      config.plugins.entries[pluginId].hooks = {};
    }
    config.plugins.entries[pluginId].hooks.allowConversationAccess = true;

After:

    if (
      !config.plugins.entries[pluginId].hooks ||
      typeof config.plugins.entries[pluginId].hooks !== 'object' ||
      Array.isArray(config.plugins.entries[pluginId].hooks)
    ) {
      config.plugins.entries[pluginId].hooks = {};
    }
    if (config.plugins.entries[pluginId].hooks.allowConversationAccess !== false) {
      config.plugins.entries[pluginId].hooks.allowConversationAccess = true;
    }

30. apps/memos-local-plugin/install.ps1 (L410)

$PID is the current process ID, which is not unique enough for a temp-file suffix. If a previous installer run crashed leaving a stale .tmp file with the same PID (after OS PID reuse), the Move-Item could overwrite it without issue, but [IO.File]::WriteAllText failing mid-write would leave the orphan with no cleanup path. Every other temp path in this script uses [guid]::NewGuid() for uniqueness — this should be consistent.

💡 Suggested Change

Before:

    $TempFile = "$MarkerFile.$PID.tmp"

After:

    $TempFile = "$MarkerFile.$([guid]::NewGuid().ToString('N')).tmp"

31. apps/memos-local-plugin/install.ps1 (L211-L213)

When rollback is triggered on a failed deploy, the backup directory is restored via Move-Item (correct), but there is no message telling the operator where the backup lives if the restore itself fails, or in other failure paths where $BackupDir is left on disk. Adding a Write-Warn after the catch would help users locate orphaned backup directories.

💡 Suggested Change

Before:

        if ($LiveMovedToBackup -and (Test-Path $BackupDir)) {
            Move-Item -Path $BackupDir -Destination $Prefix -Force
        }

After:

        if ($LiveMovedToBackup -and (Test-Path $BackupDir)) {
            Move-Item -Path $BackupDir -Destination $Prefix -Force
        } elseif ($LiveMovedToBackup) {
            Write-Warn "Rollback: backup directory not found at $BackupDir"
        }
        # Inform the operator if the backup is left on disk
        if (-not $StagedMovedLive -and (Test-Path $BackupDir)) {
            Write-Warn "Backup left at: $BackupDir"
        }

32. apps/memos-local-plugin/server/routes/admin.ts (L51-L64)

When core.shutdown() throws, scheduleWindowsShutdownAfterResponse is called immediately, scheduling a process exit even though cleanup has not been attempted and the response signals ok: false, cleared: false. On Windows there is no supervisor to restart the process, so the caller's next retry will find the server gone. The comment in the file header explicitly states "keeps the responding process alive so the route cannot self-destruct before a replacement exists", which contradicts this behaviour.

Consider omitting the shutdown schedule in the error path and only exiting after a successful clear:

💡 Suggested Change

Before:

      try {
        await deps.core.shutdown();
      } catch (err) {
        scheduleWindowsShutdownAfterResponse(ctx.res, options);
        return {
          ok: false,
          cleared: false,
          restarting: false,
          manualRestartRequired: true,
          platform,
          error: `Memory core did not shut down cleanly: ${errorMessage(err)}`,
          message: manualClearRestartMessage(agent, false),
        };
      }

After:

      try {
        await deps.core.shutdown();
      } catch (err) {
        // Do NOT schedule shutdown — leave the process alive so the caller can retry.
        return {
          ok: false,
          cleared: false,
          restarting: false,
          manualRestartRequired: true,
          platform,
          error: `Memory core did not shut down cleanly: ${errorMessage(err)}`,
          message: manualClearRestartMessage(agent, false),
        };
      }

33. apps/memos-local-plugin/server/routes/admin.ts (L173-L178)

fs.access checks whether the calling process has permission to access the path — it does not reliably indicate non-existence. On Windows, a file that was successfully unlinked but is still held open by another handle will make access throw EACCES (not ENOENT), causing the catch block to suppress the error and fall through without pushing to failures. Conversely, on some configurations access may succeed for a deleted-but-still-open file, falsely marking it as a failure.

Use fs.stat and check for ENOENT to determine absence:

💡 Suggested Change

Before:

    try {
      await fs.access(target);
      failures.push(target);
    } catch {
      /* absent as required */
    }

After:

    try {
      await fs.stat(target);
      // File still exists after unlink — held open by another process.
      failures.push(target);
    } catch (statErr) {
      if ((statErr as NodeJS.ErrnoException).code !== "ENOENT") {
        // Unexpected error; treat as failure to be safe.
        failures.push(target);
      }
      /* ENOENT → absent as required */
    }

34. apps/memos-local-plugin/server/routes/admin.ts (L146-L147)

process.pid is embedded in the user-facing instruction string. On Windows there is no supervisor to guarantee process identity is stable: if the process is replaced between the time the response is sent and the user reads the message, the PID could belong to a completely different process. Killing an arbitrary recycled PID could terminate an unrelated application.

Additionally, exposing the internal PID in an unauthenticated (or lightly authenticated) API response is an information disclosure risk in networked deployments.

Consider instructing the user to stop the named process instead, which is unambiguous and PID-independent:

💡 Suggested Change

Before:

            `Configuration saved. Close Hermes, run Stop-Process -Id ${process.pid} ` +
            "in PowerShell to stop Memory Viewer, then start Hermes again.",

After:

            "Configuration saved. Close Hermes, then in PowerShell run " +
            "Get-Process -Name 'node' | Where-Object { $_.MainWindowTitle -eq 'Memory Viewer' } | Stop-Process " +
            "(or close the Memory Viewer window directly), then start Hermes again.",

35. apps/memos-local-plugin/server/routes/admin.ts (L120-L122)

The expression options.lifecycle?.platform ?? process.platform is resolved independently inside the openclaw block and again inside the hermes block of POST /api/v1/admin/restart (lines 121 and 138). The same expression is also used at the top of the clear-data handler. If the platform-detection logic ever changes it must be updated in all three places. Hoist the resolution to a single variable at the top of each route handler, as was already done in the clear-data handler:

💡 Suggested Change

Before:

    if (agent === "openclaw") {
      const platform = options.lifecycle?.platform ?? process.platform;
      if (platform === "win32") {

After:

  routes.set("POST /api/v1/admin/restart", async (_ctx) => {
    const agent = options.agent ?? "unknown";
    const platform = options.lifecycle?.platform ?? process.platform;
    if (agent === "openclaw") {
      if (platform === "win32") {

36. apps/memos-local-plugin/server/routes/admin.ts (L217-L219)

Once finish fires and the shutdown is scheduled, the close and error listeners remain attached to the ServerResponse for the entire 300 ms delay period. While minor per-request, these dangling listeners can cause confusion if the response stream emits additional events after finishing. Use a named handler and clean up the other listeners once one fires:

💡 Suggested Change

Before:

  res.once("finish", () => schedule(300));
  res.once("close", () => schedule(res.writableFinished ? 300 : 1_000));
  res.once("error", () => schedule(1_000));

After:

  const onFinish = (): void => { cleanup(); schedule(300); };
  const onClose = (): void => { cleanup(); schedule(res.writableFinished ? 300 : 1_000); };
  const onError = (): void => { cleanup(); schedule(1_000); };
  const cleanup = (): void => {
    res.off("finish", onFinish);
    res.off("close", onClose);
    res.off("error", onError);
  };
  res.once("finish", onFinish);
  res.once("close", onClose);
  res.once("error", onError);

37. apps/memos-local-plugin/tests/python/test_bridge_client.py (L1464-L1487)

platform_name is iterated but never used in the test body — the subTest label is the only reference. All three loop iterations run identical patches against identical state, so the loop provides no additional coverage and is misleading. The _LOOPBACK_OPENER (a ProxyHandler({}) opener) is process-level state shared across all platforms; the only way to test platform-specific proxy bypass would be to swap out the opener or mock the OS-level proxy resolver. As written, the three subtests are fully redundant.

Either remove the loop and run the assertion once, or add genuine platform-differentiation (e.g., mocking urllib.request.getproxies) so each subtest exercises something different.

38. apps/memos-local-plugin/tests/python/test_bridge_client.py (L1509-L1527)

The HTTPError object is constructed before the try block. If the constructor raises an exception (e.g., due to an unexpected HTTPError API change), error would be unbound when the finally block executes error.close(), converting the original exception into a NameError and masking the real failure.

Move the construction inside the try block, or use contextlib.closing:

try:
    error = urllib.error.HTTPError(...)
    with patch.object(...):
        ...
finally:
    error.close()
💡 Suggested Change

Before:

        error = urllib.error.HTTPError(
            "http://127.0.0.1:18800/test",
            401,
            "Unauthorized",
            {},
            None,
        )
        try:
            with patch.object(
                daemon_manager_mod._LOOPBACK_OPENER,
                "open",
                side_effect=error,
            ):
                self.assertEqual(
                    daemon_manager_mod._probe_json_url("http://127.0.0.1:18800/test"),
                    "occupied",
                )
        finally:
            error.close()

After:

        try:
            error = urllib.error.HTTPError(
                "http://127.0.0.1:18800/test",
                401,
                "Unauthorized",
                {},
                None,
            )
            with patch.object(
                daemon_manager_mod._LOOPBACK_OPENER,
                "open",
                side_effect=error,
            ):
                self.assertEqual(
                    daemon_manager_mod._probe_json_url("http://127.0.0.1:18800/test"),
                    "occupied",
                )
        finally:
            error.close()

39. apps/memos-local-plugin/tests/python/test_bridge_client.py (L1570-L1572)

The tempfile.TemporaryDirectory() context manager is opened but its value is never used or needed — _probe_loopback_port(0) only attempts a socket bind, not any filesystem operation. This creates a misleading impression that the test depends on a temporary directory. Remove the with wrapper.

💡 Suggested Change

Before:

    def test_bind_probe_confirms_a_free_loopback_port(self) -> None:
        with tempfile.TemporaryDirectory():
            self.assertEqual(daemon_manager_mod._probe_loopback_port(0), "free")

After:

    def test_bind_probe_confirms_a_free_loopback_port(self) -> None:
        self.assertEqual(daemon_manager_mod._probe_loopback_port(0), "free")

40. apps/memos-local-plugin/viewer/src/components/RestartOverlay.tsx (L73)

AgentType is locally redeclared here as type AgentType = 'openclaw' | 'hermes', but restart.ts already exports an identical type as RestartAgent. This duplication means the two definitions can silently diverge if a new agent variant is added to the store. Import and reuse the already-exported type instead.

Suggested fix:

import {
  restartState,
  dismissRestartBanner,
  resolveRestartAgent,
  type RestartPhase,
  type RestartAgent,  // add this
} from "../stores/restart";

// Remove the local AgentType alias entirely
type AgentType = RestartAgent;  // or just use RestartAgent directly

41. apps/memos-local-plugin/viewer/src/components/RestartOverlay.tsx (L123-L132)

isTerminalPhase() uses a hardcoded string array with .includes() to classify terminal phases, with no compile-time linkage to the RestartPhase union. If a new terminal phase is added to RestartPhase in the future, this function will silently return false for it — causing the UI to show a spinner and no dismiss button with no type error to warn the developer.

Consider using an explicit record or satisfies pattern to enforce exhaustiveness:

const TERMINAL_PHASES = new Set<RestartPhase>([
  "restartFailed",
  "manualRestartRequired",
  "manualClearRestartRequired",
  "clearFailed",
  "clearResultUnknown",
  "manualCloseRequired",
]);

function isTerminalPhase(phase: RestartPhase): boolean {
  return TERMINAL_PHASES.has(phase);
}

This still doesn't enforce exhaustiveness at compile time, but at minimum avoids re-creating the array on every call. For full exhaustiveness, restructure as a switch with an explicit return per branch matching overlayMessage().

42. apps/memos-local-plugin/viewer/src/components/RestartOverlay.tsx (L110-L117)

Multiple overlayHint() cases use as any to construct dynamic i18n keys (e.g., `restart.clearFailedHint.${agentType}` as any). While agentType is always "openclaw" | "hermes" at runtime, the as any casts bypass TypeScript's key validation entirely across five call sites simultaneously. The i18n translation files confirm all these keys exist, but any future key rename or agent addition will go undetected.

A safer pattern uses explicit key mapping to preserve type safety:

case "clearFailed":
  return agentType === "openclaw"
    ? t("restart.clearFailedHint.openclaw")
    : t("restart.clearFailedHint.hermes");

This is more verbose but keeps the type checker engaged for each key.

43. apps/memos-local-plugin/viewer/src/stores/i18n.ts (L1103-L1104)

This string is missing a trailing 。 (Chinese full stop), which is inconsistent with every other newly added ZH sentence in this block (e.g., restart.manual, restart.clearing, restart.manualHint.hermes, restart.clearComplete, etc. all end with 。). The same issue exists on the updated restart.failedHint.openclaw line in the ZH locale below.

💡 Suggested Change

Before:

  "restart.manualHint.openclaw":
    "请在 PowerShell 中依次执行：openclaw gateway stop；openclaw gateway start",

After:

  "restart.manualHint.openclaw":
    "请在 PowerShell 中依次执行：openclaw gateway stop；openclaw gateway start。",

44. apps/memos-local-plugin/viewer/src/stores/i18n.ts (L1117-L1118)

Same missing trailing 。 as restart.manualHint.openclaw above. The EN counterpart ends with a period (...openclaw gateway start.), and all surrounding ZH strings are terminated with 。.

💡 Suggested Change

Before:

  "restart.failedHint.openclaw":
    "请在 PowerShell 中依次执行：openclaw gateway stop；openclaw gateway start",

After:

  "restart.failedHint.openclaw":
    "请在 PowerShell 中依次执行：openclaw gateway stop；openclaw gateway start。",

45. apps/memos-local-plugin/viewer/src/stores/restart.ts (L171-L172)

The guard condition is inverted. The intent (per the beginClearData contract) is: if the phase is already "clearing", the agent was already locked by beginClearData() — skip re-locking. But !== "clearing" means it calls lockRestartAgent() on every path except the beginClearData path, which is the opposite. Any caller that correctly calls beginClearData() first will have their locked agent silently overwritten here with the current health value, potentially returning a wrong agent if health is already offline at the point triggerCleared runs.

Suggestion: flip the condition.

if (restartState.value.phase !== "clearing") lockRestartAgent();
// should be:
if (restartState.value.phase === "clearing") { /* already locked */ } else { lockRestartAgent(); }
// i.e.:
if (restartState.value.phase !== "clearing") lockRestartAgent();

Wait — re-reading: the guard as written calls lockRestartAgent() when phase is NOT "clearing". That means when beginClearData() was called (phase IS "clearing"), the lock is skipped — which is actually the correct intent. The issue is the opposite: when a direct caller skips beginClearData(), this line will lock from health at an arbitrary point. This is an undocumented assumption that beginClearData() must always precede triggerCleared(). The contract should be enforced explicitly or documented.

Recommendation: add a guard that throws or logs a warning if lockedRestartAgent is null and phase is not "clearing", rather than silently falling back to a potentially stale health read.

💡 Suggested Change

Before:

  if (restartState.value.phase !== "clearing") lockRestartAgent();
  restartState.value = { phase: "restarting" };

After:

  if (restartState.value.phase !== "clearing") {
    // Direct call without beginClearData(): lock now, but health may be stale.
    // Prefer always calling beginClearData() before triggerCleared().
    lockRestartAgent();
  }
  // else: agent was already locked by beginClearData()

46. apps/memos-local-plugin/viewer/src/stores/restart.ts (L177-L184)

The !response.ok check fires before response.manualRestartRequired is checked. Looking at admin.ts, a Windows partial-failure path returns { ok: false, manualRestartRequired: true, ... }. Because !response.ok is checked first, such a response will land on "clearFailed" instead of "manualClearRestartRequired", showing the wrong UI state and losing the manual restart instruction. The manualRestartRequired check should come before the generic !ok check.

💡 Suggested Change

Before:

  if (response && !response.ok) {
    restartState.value = { phase: "clearFailed" };
    return;
  }
  if (response?.manualRestartRequired) {
    restartState.value = { phase: "manualClearRestartRequired" };
    return;
  }

After:

  if (response?.manualCloseRequired) {
    restartState.value = { phase: "manualCloseRequired" };
    return;
  }
  if (response?.manualRestartRequired) {
    restartState.value = { phase: "manualClearRestartRequired" };
    return;
  }
  if (response && !response.ok) {
    restartState.value = { phase: "clearFailed" };
    return;
  }

47. apps/memos-local-plugin/viewer/src/stores/restart.ts (L145-L151)

For the openclaw path, the POST is wrapped in a try/catch that silently swallows errors (to handle the case where the server exits before responding). However, the Windows OpenClaw path returns manualRestartRequired: true synchronously before any shutdown — the server deliberately stays alive to send the response. If a network-level error occurs on that path, the catch discards the response and the function falls through to pollHealthUntilUp(60) instead of showing the manual restart instruction, leaving Windows users polling for 150 s before seeing a "restartFailed" state. Consider differentiating a connection error (server went down) from an HTTP error (server is still up but request failed).

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Memtensor-AI commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (69/69 executed). memos_local_plugin/changed-repo-python: 69/69. Duration: 2s [advisory, non-gating] AI-generated tests on branch test/auto-gen-e245d1faac77ccd5-20260806224217: 62/67 passed, 5 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/plugin-windows-runtime-lifecycle

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现，等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 6, 2026
@syzsunshine219
syzsunshine219 merged commit 8d310a7 into main Aug 7, 2026
18 checks passed
@syzsunshine219
syzsunshine219 deleted the fix/plugin-windows-runtime-lifecycle branch August 7, 2026 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hijzy hijzy Awaiting requested review from hijzy
@whipser030 whipser030 Awaiting requested review from whipser030

Assignees

@hijzy hijzy

Labels

area:plugin OpenClaw & Hermes status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现，等待认领或派发

Projects

None yet

Milestone

No milestone

4 participants

@Hun-ger @Memtensor-AI @hijzy @syzsunshine219