Skip to content

feat(plugin): add DeepSeek Harness memory adapter - #2254

Merged
syzsunshine219 merged 5 commits into
mainfrom
agent-mem-dsh
Aug 16, 2026
Merged

feat(plugin): add DeepSeek Harness memory adapter#2254
syzsunshine219 merged 5 commits into
mainfrom
agent-mem-dsh

Conversation

@MatthewZhuang

Copy link
Copy Markdown
Collaborator

Description

Adds DeepSeek Harness support to @memtensor/memos-local-plugin as an out-of-tree Cordis bundle while preserving the existing OpenClaw and Hermes integrations.

Key changes:

  • Adds the DSH adapter, six memory tools, profile-aware storage, and package metadata.
  • Performs one automatic recall for every accepted non-empty direct-user turn, with the user query ordered before the labeled memory context.
  • Bounds DSH recall to a 3-second deadline, returns the mechanical safeCutoff on cancellable timeout/provider failure/malformed filter output, and disables malformed-JSON retries for DSH.
  • Moves relation classification, intent classification, episode routing, capture, summaries, embeddings, L2/L3, and skill work off the foreground turn path.
  • Reuses the active DSH provider/model route without copying API credentials into MemOS configuration.
  • Runs the Memory Viewer in the DSH process on loopback port 18801 and closes it with the profile across graceful and forced shutdown paths.
  • Extends the one-command installer to reconcile pnpm's reviewed native build-script allowlist and verify DSH bundle composition.
  • Adds DSH-specific documentation and regression coverage; OpenClaw and Hermes defaults remain unchanged.
  • Preserves the latest main Hermes Windows restart behavior while keeping DSH's manual profile-restart handoff immediate.

Related Issue (Required): N/A — this is a new integration proposal without an existing tracking issue.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Tested on macOS with Node.js 22.23.1 and Python 3.11.7.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)
cd apps/memos-local-plugin
npm run lint
npm test
npm run build:package
npm run check:hermes-version
python3.11 -m unittest discover -s tests/python -p "test_*.py"
bash -n install.sh
npm pack

Results:

  • Node: 175 test files passed; 1,441 tests passed and 2 skipped.
  • Python/Hermes provider: 123 tests passed.
  • Package audit: 836 archive entries scanned; no forbidden paths or credential findings.
  • Manual smoke test: clean DSH profile installation from 2.0.16-beta.1, Viewer startup, recall/tool use, and profile shutdown behavior verified.

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 | 我已添加测试以证明我的修复有效或功能正常
  • Documentation is included in this repository; no separate MemOS-Docs change is required
  • I have linked the issue to this PR (not applicable; no tracking issue)
  • Suggested reviewers: @hijzy and @whipser030

Reviewer Checklist

  • Confirm the DSH adapter remains isolated from OpenClaw/Hermes foreground semantics
  • Made sure Checks passed
  • Tests have been provided

Release follow-up

After this PR is merged, publish @memtensor/memos-local-plugin@2.0.16 with npm dist-tag latest through the repository's standalone local-plugin publisher. The workflow will synchronize package.json, package-lock.json, and the Hermes manifest for the release tag; pnpm-lock.yaml does not require a version-only refresh.

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

Copy link
Copy Markdown
Collaborator

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Environment preparation failed before any gating tests executed. Failed scopes: memos_local_plugin
Branch: agent-mem-dsh

@pittosporum-seu

Copy link
Copy Markdown

Thanks for putting this together — a first-party DSH adapter is exactly what the ecosystem needs. We've been running a parallel lightweight implementation (dsh-memos, a standalone DSH bundle that talks to the local viewer HTTP API) and hit a few things that are directly relevant to this PR. Sharing them in case they help:

Findings from our implementation

  1. Namespace is sticky on the HTTP surfacememory/search only searches the daemon's active namespace (whatever --agent= was set to); traces written under another ownerAgentKind are unreachable through the viewer API, and the delete endpoints are namespace-scoped too (they return deleted:false for rows outside the active namespace). If the DSH adapter uses the JSON-RPC turn pipeline (turn.start carries namespace handling), this should be fine — worth a test asserting DSH-written traces are recallable from the DSH side and isolated from OpenClaw/Hermes pools.

  2. Imported traces carry no vectorsimportBundle writes vec_summary/vec_action as NULL, so imported rows are invisible to tier-2 retrieval until POST /api/v1/embeddings/rebuild runs (local Xenova/all-MiniLM-L6-v2 embedder). If the adapter ever falls back to the import path (recovery, migration), it must trigger a rebuild or the memories silently never recall.

  3. Viewer port choice is right — running the DSH-process Viewer on 18801 avoids the Hermes viewer's 18800. We ran into exactly that collision; keeping per-agent viewer ports distinct is the correct call.

  4. Windows/pnpm install details for the one-command installer — pnpm ≥10/11 allowBuilds in pnpm-workspace.yaml is a map (pkg: true), and pnpm silently normalizes a list form into placeholder entries (set this to true or false) rather than rejecting it — easy to misdiagnose. Git/file: installs with prepare scripts need explicit allowBuilds entries, and the exact key pnpm prints must be used. Also note: link:-style local installs break ESM resolution for bare @deepseek-ai/* imports on Node (symlink realpath resolution), so a packed file: install is the reliable path on Windows.

  5. Write-quality gating — since DSH has no MemOS reward pipeline in front of writes, "write every turn" produces raw noise in the shared bank. We ended up with a three-way mode switch (auto / model-decides / off). If the adapter's automatic per-turn recall is paired with an easy write policy toggle, that covers both power users and privacy-sensitive ones.

Happy to run any integration checks against our local setup (Hermes + DSH side by side) if that helps validate the multi-agent coexistence story.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2254
Task: 4d1ca7bd54e788b2
Base: main
Head: agent-mem-dsh

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


1. apps/memos-local-plugin/adapters/deepseek-harness/deadline.ts (L20-L25)

The onAbort! non-null assertion is used inside finish() before onAbort is assigned on the very next line. Although this works at runtime because finish is only ever called after onAbort is assigned, the closure over the let variable is ordered such that the TypeScript compiler's control-flow analysis cannot prove it. Reordering the code to assign onAbort first (before defining finish) eliminates the need for the assertion and makes the dependency explicit:

onAbort = () => finish(
  options.signal.reason ?? new DOMException("DSH operation aborted", "AbortError"),
);
const finish = (error: unknown): void => {
  if (timer !== undefined) clearTimeout(timer);
  options.signal.removeEventListener("abort", onAbort!);
  reject(error);
};

Alternatively, capture onAbort in a local const inside the executor so both variables are in scope before either is used.

💡 Suggested Change

Before:

    const finish = (error: unknown): void => {
      if (timer !== undefined) clearTimeout(timer);
      options.signal.removeEventListener("abort", onAbort!);
      reject(error);
    };
    onAbort = () => finish(

After:

    onAbort = () => finish(
      options.signal.reason ?? new DOMException("DSH operation aborted", "AbortError"),
    );
    const capturedOnAbort = onAbort;
    const finish = (error: unknown): void => {
      if (timer !== undefined) clearTimeout(timer);
      options.signal.removeEventListener("abort", capturedOnAbort);
      reject(error);
    };

2. apps/memos-local-plugin/adapters/deepseek-harness/deadline.ts (L39-L42)

The finally block unconditionally attempts to clear the timer and remove the abort listener even when the cutoff promise was the one that rejected (i.e., timeout or abort fired first). In that case, finish() already called clearTimeout and removeEventListener before the finally runs, resulting in redundant — though harmless — double calls. More importantly, when the abort-listener path wins the race, options.signal.removeEventListener is called twice with the same onAbort reference. This is safe per the spec but suggests that cleanup ownership is split between two places. Consider consolidating cleanup exclusively in the finally block and removing it from finish(), making one canonical cleanup site:

const finish = (error: unknown): void => {
  reject(error); // cleanup happens in finally
};

This also makes the code easier to reason about when auditing resource lifetimes.


3. apps/memos-local-plugin/adapters/deepseek-harness/host-llm.ts (L321-L325)

Nested ternary expression is prohibited per the review checklist. When failure.code does not match either 'RATE_LIMIT' or 'TIMEOUT', the inner ternary runs as the false branch of the outer one, which is hard to read and error-prone.

Refactor to an if/else chain or a lookup map:

💡 Suggested Change

Before:

  const code = failure.code === "RATE_LIMIT"
    ? ERROR_CODES.LLM_RATE_LIMITED
    : failure.code === "TIMEOUT"
      ? ERROR_CODES.LLM_TIMEOUT
      : ERROR_CODES.LLM_UNAVAILABLE;

After:

  let code: typeof ERROR_CODES[keyof typeof ERROR_CODES];
  if (failure.code === "RATE_LIMIT") {
    code = ERROR_CODES.LLM_RATE_LIMITED;
  } else if (failure.code === "TIMEOUT") {
    code = ERROR_CODES.LLM_TIMEOUT;
  } else {
    code = ERROR_CODES.LLM_UNAVAILABLE;
  }

4. apps/memos-local-plugin/adapters/deepseek-harness/host-llm.ts (L298-L302)

The default branch of assertSuccessfulFinish discards the actual finish.kind value, making production failures from unknown/new DSH finish reasons opaque and difficult to diagnose. Include finish.kind in the error details so the unrecognized value is surfaced in logs.

💡 Suggested Change

Before:

    default:
      throw outputError(
        "DeepSeek Harness host LLM returned an unsupported finish reason",
        "UNSUPPORTED_FINISH",
      );

After:

    default:
      throw outputError(
        `DeepSeek Harness host LLM returned an unsupported finish reason: ${String((finish as { kind: string }).kind)}`,
        "UNSUPPORTED_FINISH",
      );

5. apps/memos-local-plugin/adapters/deepseek-harness/host-llm.ts (L31)

UNSUPPORTED_REASONING_EFFORT is a plain string literal compared against error.code from the DSH harness library. If the library changes this error code string in a future release, the comparison silently fails — the fallback prepareCall is skipped and the original error propagates unexpectedly. Add a comment referencing the DSH source of this magic string, or import the constant from the library if it is exported.

💡 Suggested Change

Before:

const UNSUPPORTED_REASONING_EFFORT = "UNSUPPORTED_REASONING_EFFORT";

After:

// Error code emitted by @deepseek-ai/dsh-llm when the adapter does not
// support the supplied ReasoningEffortId. Keep in sync with the library.
const UNSUPPORTED_REASONING_EFFORT = "UNSUPPORTED_REASONING_EFFORT";

6. apps/memos-local-plugin/adapters/deepseek-harness/index.ts (L465-L466)

The disposal callback uses bridge!.dispose() with a non-null assertion, but bridge can legitimately be undefined if an exception occurs after core.init() but before bridge = createDeepSeekHarnessBridge(...) — for example, inside the viewer startup block — and failOnStartupError is false. In that scenario the catch block catches the error, correctly calls core.shutdown(), and returns the no-op disposal function. However, if through any other code path execution reaches this return callback with bridge still undefined, bridge!.dispose() will throw a runtime TypeError during teardown, leaving core un-shutdown. The catch block already handles the null case with else if (core) await core.shutdown(), but the success-path disposal has no equivalent fallback.

Suggestion: Replace with a null-safe call and add a core fallback:

if (bridge) await bridge.dispose();
else if (core) await core.shutdown();

7. apps/memos-local-plugin/adapters/deepseek-harness/index.ts (L146-L150)

IPv6 loopback (::1) is not handled. node:net's isIP("::1") returns 6, so the condition fails and the function returns false, causing the plugin to throw "DSH Viewer bind host must be loopback" on any IPv6-only or dual-stack system where bindHost defaults to or is configured as ::1. This would break IPv6 environments silently at startup.

Suggestion: Add an IPv6 loopback check:

return normalized === "localhost" ||
  normalized === "::1" ||
  (isIP(normalized) === 4 && normalized.startsWith("127."));

8. apps/memos-local-plugin/adapters/deepseek-harness/index.ts (L295-L302)

This is a nested ternary expression, which is prohibited by the code review checklist. The outer ternary has a second ternary in its false-branch, making the logic harder to read and reason about at a glance.

Suggestion: Replace with an if/else block or a helper:

let continuationMessage: string;
if (config.failOnStartupError) {
  continuationMessage = "failing plugin startup";
} else if (isAddressInUse(error)) {
  continuationMessage = "continuing with memory enabled while Viewer retries in the background";
} else {
  continuationMessage = "continuing with memory enabled and Viewer unavailable";
}
ctx.logger.warn(`memos-local-memory: ${detail}; ${continuationMessage}`);

9. apps/memos-local-plugin/adapters/deepseek-harness/index.ts (L384-L388)

The magic number 114 used as the system-prompt section order is unexplained. There is no named constant, no comment explaining its relationship to other registered sections, and no obvious mnemonic. This makes it difficult for future maintainers to reason about ordering conflicts when new prompt sections are added.

Suggestion: Introduce a named constant with an explanatory comment:

/** Order for the memos memory guidance section; positioned after core agent instructions (≤100) and before tool-call policy sections (≥120). */
const MEMOS_SYSTEM_PROMPT_ORDER = 114;

10. apps/memos-local-plugin/adapters/deepseek-harness/bridge.ts (L514-L526)

The settled variable is declared with let on line 516 but is first read inside the .finally() closure on line 521, before it is actually assigned on line 520. This works at runtime because JavaScript closures capture the binding (not the value), and .finally() always executes asynchronously after settled is assigned — but it is a subtle read-before-assignment pattern that is brittle and confusing.

Consider restructuring to make the assignment order unambiguous:

private enqueue(sessionId: string, task: () => Promise<void>): void {
  const previous = this.pendingBySession.get(sessionId) ?? Promise.resolve();
  const current = previous.then(task).catch((error) => {
    this.warn(`MemOS background write failed for DSH session ${sessionId}`, error);
  });
  const settled = current.finally(() => {
    if (this.pendingBySession.get(sessionId) === settled) {
      this.pendingBySession.delete(sessionId);
    }
  });
  this.pendingBySession.set(sessionId, settled);
}

Using const and declaring settled directly from .finally() makes the temporal relationship explicit and prevents accidental re-assignment.

💡 Suggested Change

Before:

  private enqueue(sessionId: string, task: () => Promise<void>): void {
    const previous = this.pendingBySession.get(sessionId) ?? Promise.resolve();
    let settled: Promise<void>;
    const current = previous.then(task).catch((error) => {
      this.warn(`MemOS background write failed for DSH session ${sessionId}`, error);
    });
    settled = current.finally(() => {
      if (this.pendingBySession.get(sessionId) === settled) {
        this.pendingBySession.delete(sessionId);
      }
    });
    this.pendingBySession.set(sessionId, settled);
  }

After:

  private enqueue(sessionId: string, task: () => Promise<void>): void {
    const previous = this.pendingBySession.get(sessionId) ?? Promise.resolve();
    const current = previous.then(task).catch((error) => {
      this.warn(`MemOS background write failed for DSH session ${sessionId}`, error);
    });
    const settled = current.finally(() => {
      if (this.pendingBySession.get(sessionId) === settled) {
        this.pendingBySession.delete(sessionId);
      }
    });
    this.pendingBySession.set(sessionId, settled);
  }

11. apps/memos-local-plugin/adapters/deepseek-harness/bridge.ts (L452-L464)

In dispose(), the orphan cleanup loop calls this.core.closeSession(sessionId) for any memory sessions remaining in memorySessionOwners. However, closeSession() (called earlier via Promise.all) already drains flush() and calls releaseMemorySession() for each session it owns, which in turn deletes entries from memorySessionOwners and calls core.closeSession. If a session was successfully released in the earlier closeSession call, its entry in memorySessionOwners is already deleted, so the orphan loop only covers genuinely untracked sessions — this is correct.

However, there is still a subtle issue: owner is obtained via .values().next().value which can return undefined if the owners Set is somehow empty (e.g. an empty Set was left in the map). While withSessionLlmRoute(undefined, ...) is handled gracefully by the undefined branch, the invariant that memorySessionOwners never holds empty Sets is not enforced. If it does, core.closeSession is still called without a valid LLM route, which may fail silently.

Consider adding a guard or an assertion to make the invariant explicit:

💡 Suggested Change

Before:

      for (const sessionId of [...this.memorySessionOwners.keys()]) {
        const owner = this.memorySessionOwners.get(sessionId)?.values().next().value;
        try {
          await this.withSessionLlmRoute(
            owner,
            () => this.core.closeSession(sessionId),
          );
        } catch (error) {
          this.warn(`MemOS closeSession failed for ${sessionId}`, error);
        } finally {
          this.memorySessionOwners.delete(sessionId);
        }
      }

After:

      for (const sessionId of [...this.memorySessionOwners.keys()]) {
        const owners = this.memorySessionOwners.get(sessionId);
        // owners should never be empty here, but guard defensively.
        if (!owners || owners.size === 0) {
          this.memorySessionOwners.delete(sessionId);
          continue;
        }
        const owner = owners.values().next().value as DshSessionLike;
        try {
          await this.withSessionLlmRoute(
            owner,
            () => this.core.closeSession(sessionId),
          );
        } catch (error) {
          this.warn(`MemOS closeSession failed for ${sessionId}`, error);
        } finally {
          this.memorySessionOwners.delete(sessionId);
        }
      }

12. apps/memos-local-plugin/adapters/deepseek-harness/bridge.ts (L615-L639)

In recordCompletedToolOutcomes, this.withLlmRoute(state, () => { this.core.recordToolOutcome(...) }) wraps a fire-and-forget call inside withLlmRoute. While recordToolOutcome is confirmed to return void (not a Promise) and does not need to be awaited, the try/catch surrounding this call will NOT catch any synchronous exceptions thrown inside withLlmRoute's runWithLlmRoute callback (e.g., from routes.run() in DeepSeekHarnessLlmRouteContext), because withLlmRoute returns T synchronously and exceptions propagate up through this.withLlmRoute(...) to the try/catch — so this part is fine.

However, recordCompletedToolOutcomes itself is a synchronous method called in the middle of the async captureTurn body. If this.core.recordToolOutcome(...) throws synchronously inside the withLlmRoute wrapper, the exception is caught by the inner try/catch. But if withLlmRoute or runWithLlmRoute throws (e.g. on invalid route), that exception propagates to the outer try/catch in captureTurn and terminates the entire capture — including the subsequent onTurnEnd call. Consider whether a failed recordToolOutcome should abort the whole capture or be isolated more aggressively.

💡 Suggested Change

Before:

  private recordCompletedToolOutcomes(
    state: TurnState,
    sessionId: SessionId,
    episodeId: EpisodeId,
  ): void {
    for (const tool of state.toolCalls) {
      const endedAt = tool.endedAt;
      if (endedAt === undefined) continue;
      try {
        this.withLlmRoute(state, () => {
          this.core.recordToolOutcome({
            sessionId,
            episodeId,
            tool: tool.name,
            success: tool.errorCode === undefined,
            errorCode: tool.errorCode,
            durationMs: Math.max(0, endedAt - (tool.startedAt ?? endedAt)),
            ts: endedAt,
          });
        });
      } catch (error) {
        this.warn(`MemOS recordToolOutcome failed for ${tool.name}`, error);
      }
    }
  }

After:

  private recordCompletedToolOutcomes(
    state: TurnState,
    sessionId: SessionId,
    episodeId: EpisodeId,
  ): void {
    for (const tool of state.toolCalls) {
      const endedAt = tool.endedAt;
      if (endedAt === undefined) continue;
      try {
        // recordToolOutcome is synchronous (void). Wrap with route context so
        // any routing setup/teardown errors are isolated per-tool and do not
        // abort the enclosing captureTurn (and its critical onTurnEnd call).
        this.withLlmRoute(state, () => {
          this.core.recordToolOutcome({
            sessionId,
            episodeId,
            tool: tool.name,
            success: tool.errorCode === undefined,
            errorCode: tool.errorCode,
            durationMs: Math.max(0, endedAt - (tool.startedAt ?? endedAt)),
            ts: endedAt,
          });
        });
      } catch (error) {
        this.warn(`MemOS recordToolOutcome failed for ${tool.name}`, error);
      }
    }
  }

13. apps/memos-local-plugin/adapters/deepseek-harness/bridge.ts (L402-L416)

The flush(sessionId) loop has no termination guard. If a consumer keeps enqueuing new tasks for sessionId faster than they complete (e.g., due to a rapidly-firing event stream), this loop will spin indefinitely, causing closeSession() and dispose() to hang forever with no timeout or iteration cap.

While in practice this scenario is unlikely during normal shutdown, it would be safer to document the assumption that no new tasks are enqueued after closeSession/dispose is called, or to add a bounded-retry escape hatch.


14. apps/memos-local-plugin/adapters/deepseek-harness/bridge.ts (L360-L364)

In the 'turn/end' event handler, extractDeepSeekHarnessLlmRoute is called with a freshly synthesized agent-like object { id: session.id, session }. This duplicates the LLM route extraction that was already performed in beforeStep (where the route is saved via rememberLlmRoute). The synthesized call reads session.requestHeader?.() a second time which has side-effect potential (it's a function call on an external type).

Since beforeStep already persists the extracted route to lastLlmRouteBySession, consider reusing it here via this.lastLlmRouteBySession.get(session) instead of re-invoking requestHeader. If a route update is needed at turn-end (e.g., the model changed mid-turn), a comment explaining why re-extraction is preferred over the cached value would clarify the intent.


15. apps/memos-local-plugin/adapters/deepseek-harness/bridge.ts (L669-L672)

There is a trailing blank line before the closing brace of handleToolResult. Minor style issue that should be cleaned up.

💡 Suggested Change

Before:

    tool.errorCode = errorCode;
    tool.endedAt = event.time;

  }

After:

    tool.errorCode = errorCode;
    tool.endedAt = event.time;
  }

16. apps/memos-local-plugin/agent-contract/dto.ts (L116-L117)

The llmFilterMalformedRetries field is typed as plain number, which admits negative values, zero, and non-integers (e.g. 0.5). This is a retry count, so its meaningful domain is non-negative integers. A caller accidentally passing -1 or 1.5 would silently reach the core with an invalid value. Consider either:

  1. Adding a JSDoc constraint (@minimum 0, @type integer) consistent with other config fields in the schema, or
  2. Narrowing the type to something like number & { __brand: 'NonNegInt' } if the codebase uses branded types for this purpose.

At minimum, a brief inline comment documenting the expected range (e.g. // ≥ 0; non-integer values are floored by the core) prevents misuse.

💡 Suggested Change

Before:

  llmFilterMalformedRetries?: number;
}

After:

  /**
   * Optional per-request override for malformed JSON retries in the
   * retrieval relevance filter. Adapters that omit it retain the core
   * default.
   *
   * Must be a non-negative integer; fractional or negative values are
   * not meaningful and the core will clamp/reject them.
   */
  llmFilterMalformedRetries?: number; // ≥ 0, integer
}

17. apps/memos-local-plugin/agent-contract/dto.ts (L573)

Extract<RetrievalReason, "turn_start" | "tool_driven"> is a clever way to stay in sync with the parent union, but it is fragile in one direction: if either "turn_start" or "tool_driven" is ever removed from RetrievalReason, Extract silently drops the missing member (it resolves to the narrower remaining type, or never) rather than producing a compile-time error at the definition site. A caller passing the removed value would only get an error at their call site, which could be far from this definition.

If forward-compatibility guarantees make it unlikely these values will be removed, this is acceptable. Otherwise, prefer an explicit union literal "turn_start" | "tool_driven" and add a helper type-check assertion elsewhere to guard the sync, or document the risk.

💡 Suggested Change

Before:

  reason?: Extract<RetrievalReason, "turn_start" | "tool_driven">;

After:

  // If either literal is removed from RetrievalReason, Extract silently
  // drops it; prefer an explicit union to make the breakage visible here.
  reason?: "turn_start" | "tool_driven";

18. apps/memos-local-plugin/adapters/deepseek-harness/tools.ts (L110-L135)

When a timeout occurs, timedOut is set to true but the returned text field is still formatHits([]) which produces "No relevant memories found." — identical to a genuine empty result. The LLM has no way to distinguish a degraded/timed-out response from a legitimate absence of memories, and may incorrectly report that no prior context exists. The timeout indicator is only visible in the structured timedOut field, which the LLM likely does not read.

Suggestion: include an explicit warning in the text field when timedOut is true, e.g.:

text: timedOut
  ? `Memory search timed out after ${searchTimeoutMs}ms. Results may be incomplete.`
  : formatHits(hits),

19. apps/memos-local-plugin/adapters/deepseek-harness/tools.ts (L168-L171)

When kind is not "trace" or "policy", the code falls through unconditionally to attempt a world_model lookup — even if an unsupported or unexpected string was passed (e.g., "skill" or ""). The DSH schema declares an enum ["trace", "policy", "world_model"], but if that constraint is not strictly enforced by the framework before execute is called, an invalid kind will silently produce a misleading notFound("<invalid_kind>", id) response.

Add an explicit guard after the policy branch:

if (kind !== "world_model") {
  return notFound(kind, id); // or throw a validation error
}

20. apps/memos-local-plugin/adapters/deepseek-harness/tools.ts (L422-L427)

observeAbort only checks the signal before and after the operation, it does not interrupt operation() mid-flight. If exec.signal is aborted during a long-running operation (e.g., listWorldModels, timeline, getSkill), the abort is only detected after the operation fully completes — wasting resources and delaying cancellation.

Note that memos_search is partially protected because waitForDeepSeekHarnessDeadline races against the signal independently. However, all other tools (memos_get, memos_timeline, memos_environment, memos_skill_list, memos_skill_get) rely solely on observeAbort and have no such racing mechanism. For these, consider passing the signal down to the core methods or using a Promise.race with an abort-rejection promise.


21. apps/memos-local-plugin/adapters/deepseek-harness/tools.ts (L391)

exec.callId is cast with String(...) unconditionally. If exec.callId is undefined or null at runtime (e.g., when the DSH framework omits it for certain call types), this produces the literal strings "undefined" or "null", which will be stored as the toolCallId in skill usage records. This can corrupt usage analytics or deduplication logic that relies on unique call IDs.

Suggestion: guard the conversion and only pass it when a real value is present:

toolCallId: exec.callId != null ? String(exec.callId) : undefined,

22. apps/memos-local-plugin/adapters/deepseek-harness/tools.ts (L418-L420)

If any individual dispose() call throws, the remaining disposers in the loop are skipped, leaving their resources unreleased. This is especially problematic in the catch block of registerDeepSeekHarnessTools, where partial cleanup after a registration failure could leave tool registrations active.

Suggestion: wrap each call in a try/catch to ensure all disposers run:

function disposeAll(disposers: Array<() => void>): void {
  for (const dispose of disposers.splice(0).reverse()) {
    try { dispose(); } catch { /* ignore disposal errors */ }
  }
}

23. apps/memos-local-plugin/core/embedding/providers/local.ts (L48-L58)

Minor redundancy: the listener is registered with { once: true }, which causes the runtime to auto-remove it after the first abort event fires. The explicit signal.removeEventListener("abort", onAbort) calls inside the .then(...) resolve/reject handlers are therefore no-ops when the abort path has already executed, and harmless duplicates otherwise.

If keeping both is intentional for defensive clarity, a brief comment explaining the dual-cleanup would help future readers. Alternatively, drop { once: true } and rely solely on the manual removeEventListener calls — that makes the cleanup contract explicit and consistent across all three code paths.

💡 Suggested Change

Before:

    signal.addEventListener("abort", onAbort, { once: true });
    promise.then(
      (value) => {
        signal.removeEventListener("abort", onAbort);
        resolve(value);
      },
      (err) => {
        signal.removeEventListener("abort", onAbort);
        reject(err);
      },
    );

After:

    // Use manual removeEventListener on all three exit paths so the cleanup
    // contract is consistent and immediately obvious without { once: true }.
    signal.addEventListener("abort", onAbort);
    promise.then(
      (value) => {
        signal.removeEventListener("abort", onAbort);
        resolve(value);
      },
      (err) => {
        signal.removeEventListener("abort", onAbort);
        reject(err);
      },
    );

24. apps/memos-local-plugin/core/feedback/subscriber.ts (L89-L90)

AsyncLocalStorage.snapshot() is a static method that was introduced in Node.js 20.13.0 (and Node.js 22.3.0 for non-experimental use). The engines field in package.json declares "node": ">=20.0.0", which means any Node.js 20.x runtime below 20.13.0 will throw a TypeError: AsyncLocalStorage.snapshot is not a function at runtime on the first enqueue() call. This can silently break the entire feedback subscriber on supported (but older 20.x) installs.

Consider either:

  1. Tightening the engine constraint to >=20.13.0, or
  2. Providing a polyfill/fallback (e.g., const snapshot = typeof AsyncLocalStorage.snapshot === 'function' ? AsyncLocalStorage.snapshot() : (fn: () => Promise<void>) => fn()) so older 20.x nodes degrade gracefully.
💡 Suggested Change

Before:

const runInEnqueueContext = AsyncLocalStorage.snapshot();
    queue.push(() => runInEnqueueContext(job));

After:

// AsyncLocalStorage.snapshot() requires Node >=20.13.0 / >=22.3.0.
// Guard or tighten the engine requirement in package.json to >=20.13.0.
const snapshotFn = typeof AsyncLocalStorage.snapshot === "function"
  ? AsyncLocalStorage.snapshot()
  : (fn: () => Promise<void>) => fn();
queue.push(() => snapshotFn(job));

25. apps/memos-local-plugin/core/pipeline/types.ts (L230-L231)

The return type () => void is an anonymous function type with no semantic label. It's unclear at the call site that calling the returned function releases the foreground lock. Consider using a named type alias (e.g., type ReleaseForeground = () => void) or at minimum adding an inline name to the return type comment. Relatedly, if a caller acquires a foreground lock via enterForeground() but forgets to call the returned release (e.g., due to an unhandled exception path), the foreground state will leak permanently since there is no RAII-style automatic cleanup for it. Consider documenting that the release must be called in a finally block, or model the return as a Disposable (using TypeScript 5's Symbol.dispose pattern) to make this contract enforceable.

💡 Suggested Change

Before:

  /** Mark an adapter-owned retrieval as foreground until the returned release runs. */
  enterForeground(): () => void;

After:

  /** Mark an adapter-owned retrieval as foreground until the returned release runs.
   *  The returned release **must** be called in a `finally` block to prevent leaking
   *  the foreground lock.
   */
  enterForeground(): ReleaseForeground;

26. apps/memos-local-plugin/core/retrieval/llm-filter.ts (L223-L230)

The aborted-signal check is performed after computing remainingMs and timeoutMs, but those values are only needed later (for the timer). More importantly, if timeoutMs computes to 0 (i.e. deadlineAt is already in the past) but deps.signal is not yet aborted, the function still enters the Promise.race path and sets a setTimeout(..., 0) — meaning it rejects only on the next event-loop tick rather than immediately. This is a minor latency concern but can be addressed by adding an explicit expired-deadline fast-path:

if (remainingMs === 0) {
  throw new DOMException("retrieval filter deadline exceeded", "TimeoutError");
}

This keeps the expired-deadline rejection synchronous and consistent with the already-aborted-signal fast-path above it.

💡 Suggested Change

Before:

  const remainingMs = Math.max(0, (deps.deadlineAt ?? Date.now()) - Date.now());
  const timeoutMs = Math.max(
    0,
    Math.min(remainingMs, deps.timeoutMs ?? remainingMs),
  );
  if (deps.signal?.aborted) {
    throw deps.signal.reason ?? new DOMException("retrieval deadline exceeded", "TimeoutError");
  }

After:

  const remainingMs = Math.max(0, (deps.deadlineAt ?? Date.now()) - Date.now());
  const timeoutMs = Math.max(
    0,
    Math.min(remainingMs, deps.timeoutMs ?? remainingMs),
  );
  if (deps.signal?.aborted) {
    throw deps.signal.reason ?? new DOMException("retrieval deadline exceeded", "TimeoutError");
  }
  if (remainingMs === 0) {
    throw new DOMException("retrieval filter deadline exceeded", "TimeoutError");
  }

27. apps/memos-local-plugin/core/retrieval/llm-filter.ts (L237-L247)

Inside finish, onAbort! is used with a non-null assertion. At the point finish is called from the timer callback, onAbort has already been assigned (the assignment happens before setTimeout), so this is safe at runtime. However, using a non-null assertion here is fragile — if the order of assignments is ever changed, this becomes a silent removeEventListener("abort", undefined) call which would silently fail. Consider using an explicit guard or restructuring the closure to avoid the assertion:

deps.signal?.removeEventListener("abort", onAbort ?? (() => {}));
// or
if (onAbort) deps.signal?.removeEventListener("abort", onAbort);
💡 Suggested Change

Before:

    deps.signal?.removeEventListener("abort", onAbort!);
      reject(error);
    };
    onAbort = () => finish(
      deps.signal?.reason ?? new DOMException("retrieval deadline exceeded", "TimeoutError"),
    );
    deps.signal?.addEventListener("abort", onAbort, { once: true });
    timer = setTimeout(
      () => finish(new DOMException("retrieval filter deadline exceeded", "TimeoutError")),
      timeoutMs,
    );

After:

    const finish = (error: unknown): void => {
      if (timer !== undefined) clearTimeout(timer);
      if (onAbort) deps.signal?.removeEventListener("abort", onAbort);
      reject(error);
    };

28. apps/memos-local-plugin/core/pipeline/orchestrator.ts (L1267-L1273)

The finally block checks deadline?.signal.aborted to decide whether to emit a deadline-exceeded warning, but this condition misfires in two ways:

  1. False positive: When requestSignal is AbortSignal.any([deadline.signal, externalSignal]) and both signals fire (e.g., the external signal fires first but the deadline also fires before the finally block runs), deadline.signal.aborted will be true even though the root cause was the external caller — producing a misleading warning.

  2. False negative / no distinction: The log message says deadline_exceeded but there is no check that distinguishes it from the external signal aborting. A reader debugging production logs cannot tell whether the request was cancelled externally or genuinely timed out.

Consider adding an explicit check:

if (deadline?.signal.aborted && !externalSignal?.aborted) {
  log.warn("turn.recall.deadline_exceeded", { ... });
} else if (externalSignal?.aborted) {
  log.debug("turn.recall.cancelled", { ... });
}
💡 Suggested Change

Before:

      if (deadline?.signal.aborted) {
        log.warn("turn.recall.deadline_exceeded", {
          sessionId: input.sessionId,
          deadlineAt: input.deadlineAt,
          elapsedMs: Date.now() - startedAt,
        });
      }

After:

      if (deadline?.signal.aborted && !externalSignal?.aborted) {
        log.warn("turn.recall.deadline_exceeded", {
          sessionId: input.sessionId,
          deadlineAt: input.deadlineAt,
          elapsedMs: Date.now() - startedAt,
        });
      } else if (externalSignal?.aborted) {
        log.debug("turn.recall.cancelled", {
          sessionId: input.sessionId,
          elapsedMs: Date.now() - startedAt,
        });
      }

29. apps/memos-local-plugin/core/pipeline/orchestrator.ts (L1341-L1344)

prepareTurn calls prepareTurnInternal with no signal, making it non-cancellable. prepareTurnInternal performs several async operations including ensureSession, openEpisodeIfNeeded (relation classification), and intentForCurrentTurn. If the caller disconnects or is abandoned, all of these will run to completion with no way to interrupt them, potentially causing resource leaks or orphaned writes. Consider accepting and forwarding an AbortSignal:

async function prepareTurn(
  input: TurnInputDTO,
  signal?: AbortSignal,
): Promise<{ sessionId: SessionId; episodeId: EpisodeId }>

This matches the pattern used by prepareTurnInternal and onTurnStartForeground and is also reflected in PipelineHandle.prepareTurn which currently exposes no signal.

💡 Suggested Change

Before:

  async function prepareTurn(
    input: TurnInputDTO,
  ): Promise<{ sessionId: SessionId; episodeId: EpisodeId }> {
    const prepared = await prepareTurnInternal(input);

After:

  async function prepareTurn(
    input: TurnInputDTO,
    signal?: AbortSignal,
  ): Promise<{ sessionId: SessionId; episodeId: EpisodeId }> {
    const prepared = await prepareTurnInternal(input, signal);

30. apps/memos-local-plugin/core/pipeline/orchestrator.ts (L1337-L1341)

prepareTurnInternal unconditionally runs intentForCurrentTurn and scheduleInjection (building a retrievePlan) before returning. When called from prepareTurn, both the computed retrievePlan and the intent classification result are discarded — the work is repeated by the next onTurnStartForeground or recallTurn call. Beyond the wasted CPU/LLM cost, if openEpisodeIfNeeded performs any writes (e.g., opening a new episode), calling prepareTurn followed by onTurnStart could produce duplicate episode-open side effects depending on idempotency guarantees.

If prepareTurn truly only needs session and episode IDs, it should stop before the intent/retrieve-plan computation, or prepareTurnInternal should be split into routing-only and full-plan variants.


31. apps/memos-local-plugin/core/pipeline/orchestrator.ts (L1850)

enterForeground is now part of the public PipelineHandle API. It returns a leaveForeground callback that must be called to release the foreground resource counter. If a caller forgets the release (especially on error paths), the resource will leak and could permanently block background operations that wait on the foreground lock.

There is no documentation on the handle interface, no TypeScript disposable pattern (Symbol.dispose), and no RAII wrapper to help callers handle this correctly. At minimum, the PipelineHandle JSDoc for enterForeground should make the required cleanup explicit, and ideally callers should wrap it in a try/finally.


32. apps/memos-local-plugin/core/pipeline/memory-core.ts (L2912-L2914)

AbortSignal.any was introduced in Node.js 20.3.0, but package.json declares "node": ">=20.0.0". On Node 20.0.0–20.2.x this line throws TypeError: AbortSignal.any is not a function, crashing every searchMemory call that receives both a deadline and an external signal. Either raise the engine floor to >=20.3.0, or implement a compat shim:

const merge = (a: AbortSignal, b: AbortSignal): AbortSignal => {
  if (typeof AbortSignal.any === 'function') return AbortSignal.any([a, b]);
  const ctrl = new AbortController();
  const abort = () => ctrl.abort();
  a.addEventListener('abort', abort, { once: true });
  b.addEventListener('abort', abort, { once: true });
  return ctrl.signal;
};

33. apps/memos-local-plugin/core/pipeline/memory-core.ts (L908-L916)

Clamping to Math.max(1, ...) means that when fewer than 1 ms remain before the deadline, the LLM filter is invoked with a 1 ms timeout, which is effectively guaranteed to fire and reject via setTimeout(..., 1). This produces a confusing timeout error in logs and degrades retrieval quality right at the deadline boundary. It would be safer to skip the filter entirely when remaining time falls below a minimum viable threshold (e.g., 50 ms):

const remaining = input.deadlineAt - Date.now();
const MIN_VIABLE_FILTER_MS = 50;
timeoutMs: remaining < MIN_VIABLE_FILTER_MS
  ? 0  // skip filter; caller in llmFilterCandidates already guards on llm==null
  : Math.min(DEADLINE_FILTER_SAFE_CUTOFF_MS, remaining),

Alternatively return early from finalFilterMergedHits when insufficient budget remains.


34. apps/memos-local-plugin/core/pipeline/memory-core.ts (L2907-L2908)

ts and startedAt are assigned on consecutive lines via separate Date.now() calls, giving them slightly different values in slow environments. They appear to serve different semantic roles (ts is the retrieval request timestamp passed downstream; startedAt is the logging duration baseline), but they should share the same captured instant. Assign once and reuse:

const ts = Date.now();
const startedAt = ts;

35. apps/memos-local-plugin/core/pipeline/memory-core.ts (L2943-L2946)

The __memosDeferLlmFilterToCaller flag is passed as an untyped string key inside contextHints: Record<string, unknown>. There is no compile-time enforcement that producers and consumers agree on this key name or its type. A typo or rename on either side silently disables the deferred-filter optimization with no type error or test failure. Consider promoting it to a dedicated typed field on TurnInputDTO or contextHints, or at minimum extract it as a named constant:

const DEFER_LLM_FILTER_HINT = "__memosDeferLlmFilterToCaller" as const;
// ...
...(hubHits.length > 0 ? { [DEFER_LLM_FILTER_HINT]: true } : {}),

36. apps/memos-local-plugin/pnpm-lock.yaml (L201-L203)

The YAML key @deepseek-ai/dsh-typert-protocol appears as a peer dependency key for both dsh-agent and dsh-session. The word "typert" looks like it may be a misspelling of "typed" (i.e., dsh-typed-protocol). While this is auto-generated from the upstream package's published metadata and may be intentional, please verify this is the correct canonical package name on the registry — if it is indeed a typo in the upstream package, importing from or depending on the wrong name will cause silent resolution failures at runtime.


37. apps/memos-local-plugin/pnpm-lock.yaml (L11-L13)

@huggingface/transformers has been bumped from 3.8.1 to 4.2.0 — a major version upgrade — and the specifier is now pinned to an exact version (4.2.0) without a range operator (no ^ or ~). Major version upgrades can introduce breaking API changes. Pinning to an exact version prevents automatic patch/security fixes. Consider using ^4.2.0 to allow compatible minor and patch updates, and verify that all usages in the codebase (e.g., core/embedding/providers/local.ts) are compatible with the 4.x API.


38. apps/memos-local-plugin/pnpm-lock.yaml (L1539-L1541)

uuid@10.0.0 is explicitly flagged as deprecated in the lockfile: "uuid@10 and below is no longer supported". Since package.json declares "uuid": "^10.0.0" and this package generates identifiers in memory/session-critical paths, consider upgrading to uuid@11+ to stay on a supported version and receive future security patches.


39. apps/memos-local-plugin/install.sh (L1217-L1219)

mktemp failure is not guarded: if it fails (e.g. disk full), add_log is set to an empty string and the subsequent grep -Fq "ERR_PNPM_IGNORED_BUILDS" "" will silently pass or misfire, allowing the build-script approval gate to be bypassed. Additionally, add_log is not registered with the EXIT trap (cleanup_install_temp_dirs), so if the script is interrupted by SIGINT/SIGTERM between mktemp and one of the explicit rm -f "${add_log}" calls, the temp file leaks.

Suggestion:

  1. Guard mktemp with || die "..." as done for other temp dirs.
  2. Either add add_log to the cleanup trap, or at minimum declare it at function scope and always call rm -f "${add_log}" in a finally-style structure (e.g. after exiting the if block unconditionally).
💡 Suggested Change

Before:

  add_log="$(mktemp)"
  run_dsh_plugin_without_onnx_cuda "${dsh_bin}" plugin --profile "${DSH_PROFILE}" add "${spec}" 2>&1 \
    | tee "${add_log}" || add_status="${PIPESTATUS[0]}"

After:

  add_log="$(mktemp)" || die "Unable to create a temporary log file for DSH install."
  run_dsh_plugin_without_onnx_cuda "${dsh_bin}" plugin --profile "${DSH_PROFILE}" add "${spec}" 2>&1 \
    | tee "${add_log}" || add_status="${PIPESTATUS[0]}"

40. apps/memos-local-plugin/install.sh (L1114-L1120)

ensure_dsh_pnpm accepts any pnpm version already on PATH without checking its major version. The ERR_PNPM_IGNORED_BUILDS error code and the approve-builds sub-command are features of pnpm v11. If the user has an older pnpm (e.g. v8 or v9), the initial dsh plugin add will fail with a different error that doesn't match the grep -Fq "ERR_PNPM_IGNORED_BUILDS" check, causing the error path to be treated as a fatal generic failure and skipping the approval gate entirely.

Suggestion: Extract the major version and die if it is below the required minimum (11 in this case).

💡 Suggested Change

Before:

  if pnpm_bin="$(command -v pnpm 2>/dev/null)"; then
    if ! pnpm_version="$(pnpm --version 2>/dev/null)" || [[ -z "${pnpm_version}" ]]; then
      die "pnpm exists at ${pnpm_bin}, but it cannot run. Repair that installation and re-run."
    fi
    success "pnpm ${pnpm_version}"
    return 0
  fi

After:

  if pnpm_bin="$(command -v pnpm 2>/dev/null)"; then
    if ! pnpm_version="$(pnpm --version 2>/dev/null)" || [[ -z "${pnpm_version}" ]]; then
      die "pnpm exists at ${pnpm_bin}, but it cannot run. Repair that installation and re-run."
    fi
    local pnpm_major_version="${pnpm_version%%.*}"
    if (( pnpm_major_version < 11 )); then
      die "DSH install requires pnpm >=11 (found ${pnpm_version}). Upgrade with: npm install -g pnpm@${DSH_PNPM_VERSION}"
    fi
    success "pnpm ${pnpm_version}"
    return 0
  fi

41. apps/memos-local-plugin/install.sh (L240-L244)

find_dsh_cli has inconsistent output behaviour between its two branches:

  • The command -v dsh branch: command -v prints the path to stdout and returns 0, then && return 0 exits — the path is correctly emitted.
  • The [[ -x ... ]] branch: the fallback path is explicitly echo-ed to stdout — also correct.

However, when the function is called for the HAS_DSH detection (find_dsh_cli >/dev/null 2>&1), both stdout and stderr are discarded, which is intentional. The real problem surfaces when called from the interactive picker display:

printf "    ${GREEN}●${NC}  DSH        ${DIM}$(find_dsh_cli)${NC}\n"

If dsh is on PATH, command -v dsh prints the path and the && return 0 fires, so the substitution captures the path. But if only the fallback ~/.local/bin/dsh exists, the command -v branch exits with non-zero (printing nothing useful), and execution falls through to the [[ -x ]] branch which echos the path — correct, but relies on order-of-evaluation semantics that are easy to break when refactoring. The two branches should follow the same pattern (both using command -v style or both using echo).


42. apps/memos-local-plugin/install.sh (L114-L121)

The --profile validation regex ^[A-Za-z0-9._-]+$ allows names that start with a hyphen (e.g. --profile -x). Such a value, when passed directly to dsh plugin --profile "-x" add ..., will be misinterpreted as a CLI flag by the dsh binary, leading to unexpected behavior or errors that will be hard to diagnose.

Suggestion: Tighten the regex so the first character must be alphanumeric: ^[A-Za-z0-9][A-Za-z0-9._-]*$

💡 Suggested Change

Before:

    --profile)
      DSH_PROFILE="${2:-}"
      [[ -n "${DSH_PROFILE}" ]] || die "--profile requires a DSH profile name"
      [[ "${DSH_PROFILE}" =~ ^[A-Za-z0-9._-]+$ ]] \
        || die "--profile may contain only letters, numbers, '.', '_' and '-'"
      DSH_PROFILE_EXPLICIT="true"
      shift 2
      ;;

After:

    --profile)
      DSH_PROFILE="${2:-}"
      [[ -n "${DSH_PROFILE}" ]] || die "--profile requires a DSH profile name"
      [[ "${DSH_PROFILE}" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] \
        || die "--profile must start with a letter or digit and may contain only letters, numbers, '.', '_' and '-'"
      DSH_PROFILE_EXPLICIT="true"
      shift 2
      ;;

43. apps/memos-local-plugin/install.sh (L1177)

ort.listSupportedBackends() is not part of the public onnxruntime-node API. This call will throw TypeError: ort.listSupportedBackends is not a function, which means the verification always fails with a cryptic error rather than the structured message, making it very hard to diagnose. The real indicator that the CPU binding is healthy is that the package loads and an InferenceSession can be created (or a simpler capability probe). Consider replacing with an API that actually exists, such as checking ort.env or attempting a session create with a dummy model, or simply verifying the native .node binary is present and loadable.

💡 Suggested Change

Before:

    node -e 'const ort = require("onnxruntime-node"); const cpu = ort.listSupportedBackends().find((backend) => backend.name === "cpu"); if (!cpu || cpu.bundled !== true) throw new Error("onnxruntime-node CPU backend is unavailable");'

After:

    node -e 'const ort = require("onnxruntime-node"); if (!ort || typeof ort.InferenceSession === "undefined") throw new Error("onnxruntime-node CPU backend is unavailable");'

44. apps/memos-local-plugin/install.sh (L1397-L1401)

There is a duplicate ${NC} (color reset) at the end of the Memory Viewer line. The format string contains ${NC} after the URL and again after (after DSH restart), while the surrounding plain text (after DSH restart) is never colored. Compare with the hermes branch which correctly places a single ${NC} after the URL. The extra reset is harmless at runtime but is inconsistent and looks like a copy-paste error.

💡 Suggested Change

Before:

    dsh)
      printf "  ${BOLD}Quick links:${NC}\n"
      printf "    ${DIM}○  Memory Viewer   ${CYAN}http://127.0.0.1:${DSH_PORT}${NC}  (after DSH restart)${NC}\n"
      printf "    ${GREEN}●${NC}  DSH Web UI      ${CYAN}http://127.0.0.1:3080${NC}\n"
      ;;

After:

    dsh)
      printf "  ${BOLD}Quick links:${NC}\n"
      printf "    ${DIM}○  Memory Viewer   ${CYAN}http://127.0.0.1:${DSH_PORT}${NC}  ${DIM}(after DSH restart)${NC}\n"
      printf "    ${GREEN}●${NC}  DSH Web UI      ${CYAN}http://127.0.0.1:3080${NC}\n"
      ;;

45. apps/memos-local-plugin/server/routes/admin.ts (L186-L195)

The deepseek-harness restart response omits the platform field that all other manualRestartRequired: true responses in this route include (see the openclaw/win32 response at line 135–143 and the hermes/win32 response at line 165–176). If any client-side code branches on platform when manualRestartRequired is true, it will receive undefined here instead of the expected platform string. For consistency and defensive safety, consider including it:

const platform = options.lifecycle?.platform ?? process.platform;
return {
  ok: true,
  restarting: false,
  manualRestartRequired: true,
  platform,
  message: ...,
};

46. apps/memos-local-plugin/server/http.ts (L38-L40)

The local AgentName type (line 38) is "openclaw" | "hermes", but ServerOptions["agent"] now also accepts "deepseek-harness" (per the updated types.ts). This mismatch means:

  1. AGENT_DEFAULT_PORTS: Record<AgentName, number> silently excludes "deepseek-harness", which is intentional but undocumented.
  2. Any future code inside this file that assigns options.agent to an AgentName-typed variable will produce a TypeScript error without explanation.

Consider either updating the local type to match or adding a comment clarifying that AgentName here intentionally only covers agents with a legacy URL prefix and a well-known redirect port, while deepseek-harness is handled differently.

💡 Suggested Change

Before:

type AgentName = "openclaw" | "hermes";
type AgentPrefix = AgentName | "memos";
const AGENT_PREFIXES: readonly AgentPrefix[] = ["openclaw", "hermes", "memos"];

After:

/** Agents that participate in legacy URL-prefix backwards-compat and peer-port redirects.
 * deepseek-harness is excluded intentionally: it has no legacy prefix and no static peer port.
 */
type AgentName = "openclaw" | "hermes";
type AgentPrefix = AgentName | "memos";
const AGENT_PREFIXES: readonly AgentPrefix[] = ["openclaw", "hermes", "memos"];

47. apps/memos-local-plugin/server/routes/migrate.ts (L95-L100)

Type mismatch: ScanResult.agent is typed as LegacyAgent ("openclaw" | "hermes"), but options.agent can be "deepseek-harness" or the fallback "unknown" — neither is a valid LegacyAgent. Although TypeScript won't catch this here because the handler returns unknown, it violates the ScanResult contract and will produce unexpected values for consumers that parse and type-assert the response body.

Suggestion: widen the agent field in ScanResult (or a distinct error variant) to also accept string, or cast explicitly with a comment:

    return {
      found: false,
      agent: (options.agent ?? "unknown") as string,  // not a LegacyAgent
      path: "",
      error: "No legacy memory database is defined for this agent.",
    };

Alternatively, introduce a narrow discriminated-union response type:

type ScanResponse = ScanResult | { found: false; agent: string; path: string; error: string };

48. apps/memos-local-plugin/viewer/src/components/Header.tsx (L202-L208)

Nested ternary expressions are prohibited per the code quality checklist. This inline nested ternary is also duplicating agent-to-logo mapping logic that already exists in the AgentLogo component (which was updated in this same PR to handle "deepseek-harness"). Instead of duplicating and nesting, you should extract a helper or, better yet, reuse the existing AgentLogo component that already encapsulates this logic.

Example refactor:

// Replace the <img> block with the AgentLogo component:
<AgentLogo agent={h.agent} size={28} />

This eliminates the nested ternary and avoids duplicating the agent-to-logo mapping.

💡 Suggested Change

Before:

src={
                h.agent === "hermes"
                  ? "hermes-logo.svg"
                  : h.agent === "openclaw"
                    ? "openclaw-logo.svg"
                    : "memos-logo.svg"
              }

After:

<AgentLogo agent={h.agent} size={28} />

49. apps/memos-local-plugin/viewer/src/components/AgentLogo.tsx (L37-L38)

The deepseek-harness branch reuses /memos-logo.svg, which is the MemOS application's own brand mark. This creates a confusing visual identity: the DeepSeek Harness adapter appears to carry the MemOS logo instead of a DeepSeek-related mark.

Consider one of the following:

  1. Ship a dedicated /deepseek-harness-logo.svg (e.g. the DeepSeek logo already present in the repo at apps/openwork-memos-integration/apps/desktop/public/assets/ai-logos/deepseek.svg) placed into viewer/public/, and reference /deepseek-harness-logo.svg here — consistent with how hermes uses /hermes-logo.svg.
  2. If intentional (branding requirement), add an inline comment explaining why the generic MemOS logo is used for this adapter, so future maintainers don't mistake it for a copy-paste error.

50. apps/memos-local-plugin/viewer/src/components/AgentLogo.tsx (L34-L36)

The file-level JSDoc comment at the top of AgentLogo.tsx describes the openclaw and hermes agents but has not been updated to mention the new deepseek-harness entry. Consider adding a bullet similar to the existing ones so future readers know which static asset is used and where it comes from.


51. apps/memos-local-plugin/viewer/src/components/RestartOverlay.tsx (L76)

The local AgentType is now identical to the exported RestartAgent type defined in ../stores/restart. Duplicating the union means a future agent variant must be added in two places. Replace the local alias with a re-use of the canonical type:

import { , type RestartAgent } from "../stores/restart";

Then remove the local AgentType declaration and replace all usages with RestartAgent.

💡 Suggested Change

Before:

type AgentType = "openclaw" | "hermes" | "deepseek-harness";

After:

// Remove this local alias — import RestartAgent from "../stores/restart" instead.
// type AgentType = "openclaw" | "hermes" | "deepseek-harness";

52. apps/memos-local-plugin/viewer/src/stores/restart.ts (L164-L165)

The early return only guards the manualRestartRequired === true branch. When the DeepSeek Harness server responds with manualRestartRequired: false (a plausible future code path), the deepseek-harness agent will fall through to pollHealthUntilReplaced, which is designed for Hermes' daemon-replacement model and will spin for up to 120 seconds polling an in-process server that never goes down. Consider hoisting the agent guard to cover the entire non-openclaw block, or adding an equivalent guard before pollHealthUntilReplaced:

if (agent === "deepseek-harness") return;
const replaced = await pollHealthUntilReplaced(response.instanceId);

→ add a symmetric guard after the if (response.manualRestartRequired) block:

if (agent === "deepseek-harness") return;   // in-process; no daemon replacement
const ok = await pollHealthUntilUp(60);
💡 Suggested Change

Before:

        if (agent === "deepseek-harness") return;
        const replaced = await pollHealthUntilReplaced(response.instanceId);

After:

        if (agent === "deepseek-harness") return;
        const replaced = await pollHealthUntilReplaced(response.instanceId);
        if (replaced) {
          window.location.href =
            window.location.pathname + "?_t=" + Date.now();
          return;
        }
        restartState.value = { phase: "restartFailed" };
        throw new Error("restart did not complete");
      }
    } catch {
      restartState.value = { phase: "restartFailed" };
      throw new Error("restart failed");
    }

    if (agent === "deepseek-harness") return; // in-process; no daemon replacement needed
    const ok = await pollHealthUntilUp(60);

53. apps/memos-local-plugin/viewer/src/views/SettingsView.tsx (L1006-L1008)

The new deepseek-harness agent type was added to ServerOptions.agent (in server/types.ts) but is not included in this guard condition. As a result, users running the deepseek-harness agent will never see DangerZoneSection, even though the section is otherwise applicable and available. Whenever a new agent is added, this allowlist must be updated manually — this is a fragile pattern.

Consider inverting the logic to an explicit deny-list, or better yet, deriving the visibility from a dedicated field in the HealthPayload (e.g. health.value?.supportsDangerZone) that the server controls, so the client doesn't need to hard-code agent names at all.

💡 Suggested Change

Before:

      {(health.value?.agent === "openclaw" || health.value?.agent === "hermes") && (
        <DangerZoneSection />
      )}

After:

      {(health.value?.agent === "openclaw" || health.value?.agent === "hermes" || health.value?.agent === "deepseek-harness") && (
        <DangerZoneSection />
      )}

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Environment preparation failed before any gating tests executed. Failed scopes: memos_local_plugin
Branch: agent-mem-dsh

@syzsunshine219
syzsunshine219 merged commit 65a8713 into main Aug 16, 2026
17 of 18 checks passed
@syzsunshine219
syzsunshine219 deleted the agent-mem-dsh branch August 16, 2026 04:27
syzsunshine219 added a commit that referenced this pull request Aug 16, 2026
## Description

Fix the DSH Viewer static-root resolution exposed by the 2.0.16 release
dry run. In a clean checkout, the test job runs before `prepack` builds
`viewer/dist`; the previous existence-based fallback therefore selected
`apps/viewer/dist` instead of the plugin-owned Viewer directory.

The resolver now identifies the package root through its stable
`package.json` marker, so both source (`adapters/deepseek-harness`) and
packed (`dist/adapters/deepseek-harness`) layouts resolve to
`<package>/viewer/dist` even before Viewer assets are built. A
regression test covers both clean layouts, including a package root
itself named `dist`.

Failed dry-run evidence:
https://github.com/MemTensor/MemOS/actions/runs/31927133607

Related Issue (Required): Follow-up fix for #2254

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)

## How Has This Been Tested?

- [x] Unit Test: `npx vitest run
tests/unit/adapters/deepseek-harness-viewer.test.ts` (11 passed)
- [x] Test Script Or Test Steps: `npm run lint`
- [x] Test Script Or Test Steps: `npx vitest run --silent` (175 files
passed; 1448 passed, 2 skipped)
- [x] Test Script Or Test Steps: `npm pack --pack-destination
tmp/dsh-viewer-static-root-pack` and verified the tarball contains the
compiled DSH adapter plus `viewer/dist/index.html`

## Checklist

- [x] I have performed a self-review of my own code | 我已自行检查了自己的代码
- [x] I have commented my code in hard-to-understand areas |
我已在难以理解的地方对代码进行了注释
- [x] I have added tests that prove my fix is effective or that my
feature works | 我已添加测试以证明我的修复有效或功能正常
- [x] Documentation update is not applicable for this packaging-path fix
- [x] I have linked the originating PR and failed release run

## Reviewer Checklist

- [ ] Made sure Checks passed
- [x] Tests have been provided
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants