fix(plugin): consolidate local runtime hardening

[Hun-ger]

Description

This PR consolidates the local-plugin reliability fixes developed and validated on fix-local-plugin-260824.

Highlights

• hardens Hermes bridge process detection, PID/status reconciliation, installer rollback, Windows-native provider behavior, and UTF-8 tool output;
• makes secret environment fallback explicit and wires first-class maxTokens / headers configuration through the L3 and Skill Evolver clients;
• moves idle-skill archival off the foreground path with bounded, atomic lifecycle work;
• recovers useful crystallizer drafts when LLM output omits derivable fields;
• bounds startup-recovery shutdown and adds the newest-trace index with collision-safe migration handling;
• resolves DeepSeek Harness auxiliary reasoning capability from exact model metadata, with per-route TTL caching, concurrent lookup coalescing, adapter-update invalidation, and registration-bound race fallback.

Contribution history is preserved from #2192, #2209, #2246, #2248, #2252, #2253, #2256, and #2284. The capability-driven implementation supersedes the negative probe cache proposed in #2285.

No new runtime dependencies are introduced.

Related Issue (Required): Fixes #2278

Reviewers: @hijzy @whipser030

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Refactor (does not change functionality, e.g. code style improvements, linting)
• Documentation update

How Has This Been Tested?

• Unit Test
  • npm run test:unit -- --reporter=dot: 179 files passed; 1,549 tests passed and 2 skipped.
  • npm exec vitest run tests/unit/adapters/deepseek-harness-*.test.ts -- --reporter=dot: 65/65 passed.
  • python3 apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py: 36/36 passed.
• Test Script Or Test Steps
  • npm run lint
  • npm run build:package
  • npm run check:hermes-version
  • Ruff 0.11.8 check and format verification for both changed Python files.
  • Packed commit d76c78aa and installed the identical tarball into DSH 0.1.0-rc.6 web profiles on macOS and Windows.
  • Verified SHA-256 61e805f862acbe0873581e22cac4592ab0a9ff4d23d607a5f9fac5df93e80253 on both hosts.
  • Verified unsupported-reasoning routes issue zero rejected off probes across repeated/concurrent calls, supported routes retain off, adapter invalidation refreshes the capability, and both installed profiles start with healthy Host LLM bridges.
• Pipeline Automated API Test
  • GitHub CI will run after PR creation.

Note: make format could not start because Poetry is not installed on the validation host. The equivalent Ruff 0.11.8 checks for the changed Python files passed.

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 (if applicable) | N/A: no external documentation change is required.
• 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

• closes [Bug] memos-local-plugin: --daemon entry never starts the status heartbeat — health reports "stale" forever while RPC works #2278
• Made sure Checks passed locally; GitHub CI will run on creation
• Tests have been provided

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2286
Task: 2bc64e976fa083c0
Base: main
Head: fix-local-plugin-260824

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

⚠️ 1 warning(s) occurred during review.

---

1. apps/memos-local-plugin/adapters/deepseek-harness/host-llm.ts (L328-L344)

The .catch callback closes over entry, but entry is assigned after the promise chain is constructed. If resolveModelInfo rejects synchronously (e.g. in unit tests with mock implementations), the .catch fires before entry is written, so cache.entries.get(key) === entry compares against undefined and never removes the stale entry — the rejected promise remains cached indefinitely for this key until TTL expiry.

Fix: assign entry before the async chain, using Promise.resolve().then(...) to guarantee async settlement, or restructure so the entry is written before any .catch can fire:

const entry: CapabilityCacheEntry = {
  expiresAt: Date.now() + MODEL_CAPABILITY_TTL_MS,
  value: Promise.resolve()
    .then(() => llm.resolveModelInfo(route.provider, route.model, signal))
    .then((info): AuxiliaryReasoningCapability => (
      info.reasoning?.efforts.some((e) => e.id === NO_REASONING_EFFORT) ? "off" : "plain"
    ))
    .catch((error: unknown) => {
      if (cache.entries.get(key) === entry) cache.entries.delete(key);
      throw error;
    }),
};
cache.entries.set(key, entry);
return entry.value;

---

2. apps/memos-local-plugin/adapters/deepseek-harness/host-llm.ts (L329-L338)

resolveAuxiliaryReasoningCapability is declared async but returns an un-awaited promise (return value). An async function wraps its return in another Promise, so the actual type is Promise<Promise<AuxiliaryReasoningCapability>> — TypeScript flattens this transparently, but it is misleading style and can confuse future readers or stricter lint rules. Prefer return await value; inside the async function, or drop the async keyword entirely since no await is used in this function body.

---

3. apps/memos-local-plugin/bridge/status.ts (L177-L183)

The heartbeat timer calls markConnected() unconditionally on every tick, regardless of the current connection state. If a caller invokes markDisconnected(message) without first calling stop() on the heartbeat handle, the next tick (≤5 s) will silently overwrite the 'disconnected' status back to 'connected', masking the outage.

The writer's hermes-process.ts diff only changes the pgrep pattern — it does not show a lifecycle guard that prevents this race. Consider checking the status before writing inside the interval callback.

💡 Suggested Change

Before:

      const timer = setInterval(markConnected, heartbeatMs);
      timer.unref?.();
      return {
        stop() {
          clearInterval(timer);
        },
      };

After:

      const timer = setInterval(() => {
        if (status.status !== "disconnected") markConnected();
      }, heartbeatMs);
      timer.unref?.();
      return {
        stop() {
          clearInterval(timer);
        },
      };

---

4. apps/memos-local-plugin/bridge/status.ts (L97-L98)

Uses != (loose inequality) instead of !== (strict inequality). The checklist prohibits ==/!=; use !== null here.

💡 Suggested Change

Before:

        status.lastOkAt != null &&
        observedAt - status.lastOkAt <= staleMs;

After:

        status.lastOkAt !== null &&
        observedAt - status.lastOkAt <= staleMs;

---

5. apps/memos-local-plugin/bridge/status.ts (L106-L107)

Same loose != violation. Additionally, heartbeatStale evaluates to true whenever the last written status was 'connected' with a non-null lastOkAt — but this branch is reached only after freshConnected was already false. The two scenarios ("heartbeat went stale" vs. "process just started with no prior file") produce different heartbeatStale values, which is correct, but naming the variable heartbeatStale when it also captures "was previously connected" makes the intent ambiguous for future readers.

💡 Suggested Change

Before:

      const heartbeatStale =
        status?.status === "connected" && status.lastOkAt != null;

After:

      const hadConnectedHeartbeat =
        status?.status === "connected" && status.lastOkAt !== null;

---

6. apps/memos-local-plugin/bridge/status.ts (L149-L151)

All I/O errors in writeStatus are silently swallowed. A persistent permission error or full disk will go completely undetected: the writer's in-memory status stays current, but the file on disk is never updated. The Viewer daemon (a separate process) reads only the file, so it will be stuck on the last successful write indefinitely with no indication anything is wrong.

Suggestion: log once on first failure without rethrowing:

let writeErrorLogged = false;
...
} catch (err) {
if (!writeErrorLogged) {
console.error("[bridge/status] Failed to write status file:", err);
writeErrorLogged = true;
}
}

---

7. apps/memos-local-plugin/bridge/status.ts (L69-L74)

lastOkAt is used as a fallback value for an error timestamp. When no error has ever occurred but a successful heartbeat has, errorAt() returns the time of the last success as if it were the time of the last error. Operators or dashboards reading lastErrorAt from the resulting snapshot will see a misleading timestamp that predates the actual problem onset.

💡 Suggested Change

Before:

function errorAt(
  status: BridgeStatusSnapshot | null,
  observedAt: number,
): number {
  return status?.lastErrorAt ?? status?.lastOkAt ?? observedAt;
}

After:

function errorAt(
  status: BridgeStatusSnapshot | null,
  observedAt: number,
): number {
  return status?.lastErrorAt ?? observedAt;
}

---

8. apps/memos-local-plugin/bridge/status.ts (L5-L6)

The 4× ratio between heartbeat interval and stale threshold (allowing up to 3 missed heartbeats) is a deliberate safety margin, but it is not documented. If either constant is changed independently in the future, the margin could silently be violated without any warning. A brief comment capturing the invariant would prevent this.

💡 Suggested Change

Before:

export const BRIDGE_STATUS_HEARTBEAT_MS = 5_000;
export const BRIDGE_STATUS_STALE_MS = 20_000;

After:

export const BRIDGE_STATUS_HEARTBEAT_MS = 5_000;
// Stale threshold allows up to 3 missed heartbeats (4× heartbeat interval).
export const BRIDGE_STATUS_STALE_MS = 20_000;

---

9. apps/memos-local-plugin/core/config/defaults.ts (L62)

The maxTokens default for llm is 1024 while l3Llm and skillEvolver both default to 4096. If intentional (e.g. short retrieval calls vs. synthesis), a brief comment should explain the difference to prevent a future maintainer from silently "fixing" the asymmetry.

---

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

The arithmetic makes the unit clear but the value 30 encodes a business policy ("archive skills idle for 30 days") with no explanation. Add a short inline comment — e.g. // 30 days — so readers tuning this value understand its intent rather than treating it as an arbitrary constant.

---

11. apps/memos-local-plugin/core/config/schema.ts (L120-L125)

The maxTokens and headers fields added here are byte-for-byte identical to those added to SkillEvolverSchema below. If this codebase supports composing TypeBox schemas (e.g., a shared LlmBaseSchema partial with Type.Intersect or Type.Composite), extract these two fields to avoid divergence drift — any future constraint change must currently be applied in two places.

---

12. apps/memos-local-plugin/core/config/schema.ts (L122)

The { default: {} } is passed to Type.Record, but this field is Type.Optional (the key can be entirely absent). In TypeBox/AJV with useDefaults, a nested default only fires when the field is present but the value is missing — not when the field is omitted. If the intent is "default to an empty object when the key is absent", the default must be placed on the outer Type.Optional wrapper or handled in the defaults layer, not inside Type.Record.

💡 Suggested Change

Before:

  headers: Type.Optional(Type.Record(Type.String(), Type.String(), { default: {} })),

After:

  headers: Type.Optional(Type.Record(Type.String(), Type.String()), { default: {} }),

---

13. apps/memos-local-plugin/core/config/schema.ts (L119-L120)

The comment ties the minimum value of 100 to a single model (deepseek-v4-flash). As other providers/models are added this constraint will become misleading without the comment being updated. Model-specific rationale belongs in the PR description or changelog; the schema comment should describe the field generically.

---

14. apps/memos-local-plugin/bridge/status.ts (L73)

errorAt() falls back to status.lastOkAt when lastErrorAt is null. This reports the timestamp of the last successful heartbeat as if it were the time the error occurred. Any dashboard or log entry reading lastErrorAt will show a timestamp that predates the actual problem onset.

Suggestion: drop the lastOkAt fallback and use observedAt directly:

return status?.lastErrorAt ?? observedAt;

---

15. apps/memos-local-plugin/core/config/index.ts (L235-L237)

resolveOpenCodeApiKey checks cleaned.llm.provider before deepMerge runs, so a user who omits provider from their YAML (relying on the default "openai_compatible") will have cleaned.llm.provider === undefined, causing the check to return early and silently skip the OpenCode API key fallback. No warning is emitted. resolveSecretEnv — and thus this function — is called on cleaned before deepMerge(DEFAULT_CONFIG, cleaned) is applied, so no default values are present yet.

Fix: move resolveSecretEnv(cleaned, warnings) to run after deepMerge, passing merged instead of cleaned.

---

16. apps/memos-local-plugin/core/config/index.ts (L331-L337)

FREE_FORM_CONFIG_PATHS.includes(path) checks the parent path passed into the current pruneUnknown call, not the full dotted path of the child key k being processed. When iterating the llm section, path is "llm" and k might be "headers" — but the check tests "llm" against entries like "llm.headers", so it never matches. Conversely, if a parent path were somehow listed, all of its children would incorrectly be passed through as free-form. The check should be made against the child's full dotted path.

Fix:

const childPath = path ? `${path}.${k}` : k;
if (FREE_FORM_CONFIG_PATHS.includes(childPath)) {
  out[k] = v;
  continue;
}
out[k] = pruneUnknown(v, (defaults as Record<string, unknown>)[k], childPath, warnings);

---

17. apps/memos-local-plugin/core/config/index.ts (L171-L179)

The traversalOk flag is dead code. When the loop breaks due to !isPlainObject(cursor), the break fires before cursor is reassigned, so cursor still holds the non-plain-object value that triggered the guard. The post-loop check !isPlainObject(cursor) is therefore already true in that case, and !traversalOk adds nothing. The comment's concern about cursor pointing at the wrong node is not reachable given the loop structure.

Fix: remove traversalOk and simplify the guard to if (!isPlainObject(cursor)) continue;

💡 Suggested Change

Before:

    let traversalOk = true;
    for (let i = 0; i < keys.length - 1; i++) {
      if (!isPlainObject(cursor)) {
        traversalOk = false;
        break;
      }
      cursor = (cursor as Record<string, unknown>)[keys[i]!];
    }
    if (!traversalOk || !isPlainObject(cursor)) continue;

After:

    for (let i = 0; i < keys.length - 1; i++) {
      if (!isPlainObject(cursor)) break;
      cursor = (cursor as Record<string, unknown>)[keys[i]!];
    }
    if (!isPlainObject(cursor)) continue;

---

18. apps/memos-local-plugin/core/config/index.ts (L249-L253)

resolveSecretEnv (and this function) runs before Typebox schema validation in resolveConfig. When new URL(endpoint) throws for a malformed endpoint, the catch block is silent — no warning is pushed. The user receives a schema error about the bad endpoint, but no signal that llm.apiKey also wasn't resolved. The two failures are causally linked (bad endpoint → key not resolved → auth fails at runtime), so the missing warning compounds the debugging difficulty.

Fix: push a warning from the catch block indicating that OpenCode key resolution was skipped due to the malformed endpoint value.

💡 Suggested Change

Before:

  } catch {
    // Schema validation reports malformed endpoints later. Secret resolution
    // must not broaden fallback scope just because parsing failed here.
  }
  return undefined;

After:

  } catch {
    warnings?.push(
      `config: 'llm.apiKey' OpenCode fallback skipped — 'llm.endpoint' value is not a valid URL`
    );
  }
  return undefined;

---

19. apps/memos-local-plugin/core/pipeline/memory-core.ts (L1998-L2004)

Non-timeout errors thrown by startupRecoveryPromise are silently swallowed. The original code had a comment /* already logged inside the recovery promise */, but any error that is not the timeout sentinel (e.g. an unexpected rejection bubbling out of withTimeout) falls through the if block without being logged or re-thrown. This masks real failures at shutdown. Either add an else branch that logs the unexpected error, or re-throw it.

💡 Suggested Change

Before:

      } catch (err) {
        if (
          err instanceof Error &&
          err.message === "startup_recovery_shutdown_timeout"
        ) {
          startupRecoveryTimedOut = true;
          startupRecoveryCancelled = true;

After:

      } catch (err) {
        if (
          err instanceof Error &&
          err.message === "startup_recovery_shutdown_timeout"
        ) {
          startupRecoveryTimedOut = true;
          startupRecoveryCancelled = true;
          log.warn("startup_recovery.shutdown_timeout", {
            timeoutMs: startupRecoveryShutdownGraceMs,
            action: "cancel_and_shutdown_pipeline",
          });
        } else {
          /* already logged inside the recovery promise */
          log.warn("startup_recovery.unexpected_error", {
            err: err instanceof Error ? err.message : String(err),
          });
        }
      }

---

20. apps/memos-local-plugin/core/pipeline/memory-core.ts (L5076-L5079)

handle.db.tx() return value is not awaited or checked. If tx() is async (or returns a value that signals failure), ignoring it could mean the transaction silently failed while the code below still emits a skill.status.changed event as if the writes succeeded. Verify whether db.tx() is synchronous and throws on failure; if it can return a rejected promise, it must be awaited.

💡 Suggested Change

Before:

    handle.db.tx(() => {
      handle.repos.skills.setStatus(id, "active", now);
      handle.repos.skills.recordUse(id, now);
    });

After:

    await handle.db.tx(() => {
      handle.repos.skills.setStatus(id, "active", now);
      handle.repos.skills.recordUse(id, now);
    });

---

21. apps/memos-local-plugin/core/pipeline/orchestrator.ts (L1668-L1676)

Double foregroundResources.shutdown() when flushGraceMs === 0. When the caller passes flushGraceMs: 0, (1) foregroundResources.shutdown(reason) fires immediately in the guard block, then (2) settlesWithin(flushPromise, 0) returns false right away (the flush microtask cannot settle in zero real milliseconds), so the if (!completed) branch fires and calls foregroundResources.shutdown(reason) a second time. If shutdown is not idempotent this silently corrupts state or triggers double-teardown side effects.

Suggested fix: skip the inner call when the early-abort path was already taken:

if (flushGraceMs === 0) {
  foregroundResources.shutdown(reason);
}
const flushPromise = flush();
try {
  const completed = await settlesWithin(flushPromise, flushGraceMs);
  if (!completed) {
    log.warn("pipeline.flush_timeout", { reason, timeoutMs: flushGraceMs });
    if (flushGraceMs > 0) foregroundResources.shutdown(reason); // avoid double call
    ...
  }

---

22. apps/memos-local-plugin/core/pipeline/orchestrator.ts (L1666-L1667)

Magic number defaults should be named constants. The values 15_000 and 4_000 are coupled to Hermes' 20 s shutdown ceiling (noted in the adjacent comment). Inlining them makes silent drift easy — changing one without the other can blow past the outer ceiling. Extract to module-level constants:

const DEFAULT_FLUSH_GRACE_MS = 15_000;
const DEFAULT_ABORT_WAIT_MS  = 4_000;

---

23. apps/memos-local-plugin/core/skill/lifecycle-worker.ts (L68-L74)

runNow() and flush() both expose the raw promise from beginRun(), so rejections propagate to callers with no log output. trigger() wraps the same promise in a .catch() that logs the error, but callers of runNow()/flush() will silently swallow failures unless they handle the rejection themselves. Either document that callers are responsible, or add a consistent .catch() log path here.

---

24. apps/memos-local-plugin/core/skill/lifecycle-worker.ts (L48-L49)

When a lifecycle run is still active (running is truthy) and the interval fires, trigger() silently skips that tick with no retry scheduled. A run lasting longer than intervalMs will effectively double the wait until the next cycle. This behaviour should at minimum be documented with a comment; if it matters for correctness, consider scheduling a deferred retry when the active run finishes.

---

25. apps/memos-local-plugin/core/skill/lifecycle-worker.ts (L41-L43)

The .then()/.finally() chain here is intentional — beginRun() must return the same Promise object it assigns to running so the identity check (running === current) in finally works correctly. A short comment explaining this invariant would prevent future readers from refactoring it to async/await and silently breaking the deduplication logic.

---

26. apps/memos-local-plugin/core/skill/lifecycle-worker.ts (L63)

The double cast is the only TypeScript-safe way to call Node.js's unref() without importing @types/node. A one-line comment (e.g. // Node.js: prevent this timer from keeping the process alive) would clarify intent for reviewers unfamiliar with the pattern.

---

27. apps/memos-local-plugin/core/skill/crystallize.ts (L741-L751)

defaultDraftValidator silently mutates its argument. A function named "validator" with a void return type is expected to be a pure predicate — callers and tests can observe unexpected changes to draft.summary. The repair logic already lives in prepareDraft's repairedFields path, so this is also duplicated. Remove the mutation here and rely solely on the prepareDraft repair path; restore the original throw new Error("skill.crystallize.invalid: missing summary") behaviour.

💡 Suggested Change

Before:

  if (!draft.summary) {
    // Auto-generate a summary from the richest available field. Use `||` not
    // `??`: LLM JSON emits empty strings, and `??` only falls through on
    // null/undefined.
    const autoSummary =
      draft.steps?.[0]?.body ||
      draft.steps?.[0]?.title ||
      draft.displayTitle ||
      draft.name;
    draft.summary = autoSummary.slice(0, 200);
  }

After:

  if (!draft.summary) {
    throw new Error("skill.crystallize.invalid: missing summary");
  }

---

28. apps/memos-local-plugin/core/skill/crystallize.ts (L424-L425)

Identity-equality guard validate !== defaultDraftValidator is fragile. If a caller wraps defaultDraftValidator in an adapter (e.g. (d) => defaultDraftValidator(d)), both run, which currently double-mutates draft.summary and could double-throw in edge cases. Consider removing the deduplication guard and documenting that callers should not re-pass defaultDraftValidator as the custom validator.

💡 Suggested Change

Before:

    defaultDraftValidator(draft);
    if (validate && validate !== defaultDraftValidator) validate(draft);

After:

    defaultDraftValidator(draft);
    if (validate) validate(draft);

---

29. apps/memos-local-plugin/core/skill/crystallize.ts (L489-L494)

stepUsesAlias produces false positives: a step object that has both canonical fields (title, body) and extra alias fields (name, description, etc.) is flagged even though no alias normalisation was needed. The "step-aliases" log entry would fire spuriously. Add a check that the canonical fields are absent before declaring an alias was used.

💡 Suggested Change

Before:

function stepUsesAlias(value: unknown): boolean {
  if (typeof value === "string") return true;
  if (!isRecord(value)) return false;
  return value.name !== undefined || value.description !== undefined ||
    value.content !== undefined || value.instruction !== undefined;
}

After:

function stepUsesAlias(value: unknown): boolean {
  if (typeof value === "string") return true;
  if (!isRecord(value)) return false;
  const hasCanonical = cleanOptionalText(value.title) || cleanOptionalMarkdown(value.body);
  if (hasCanonical) return false;
  return value.name !== undefined || value.description !== undefined ||
    value.content !== undefined || value.instruction !== undefined;
}

---

30. apps/memos-local-plugin/core/skill/crystallize.ts (L167-L168)

malformedRetries: 0 is a silent behaviour change if this field previously defaulted to a non-zero value. It is unclear whether this is intentional (retries are now handled by crystallizeDraft's own catch block) or an accidental disabling of retries. Add a brief inline comment explaining the intent.

💡 Suggested Change

Before:

        schemaHint: SKILL_DRAFT_SCHEMA_HINT,
        malformedRetries: 0,

After:

        schemaHint: SKILL_DRAFT_SCHEMA_HINT,
        malformedRetries: 0, // retries handled by crystallizeDraft's own catch block

---

31. apps/memos-local-plugin/core/skill/subscriber.ts (L243)

archiveNextIdleBatch() is called bare with no try/catch. If the repository layer throws (e.g., SQLITE_BUSY or a schema mismatch), the entire lifecycleTick() rejects, potentially silencing the error or crashing the subscriber. Wrap the call in a try/catch that logs and breaks out of the loop gracefully.

💡 Suggested Change

Before:

      const archivedSkills = deps.repos.skills.archiveNextIdleBatch({

After:

      let archivedSkills: ReturnType<typeof deps.repos.skills.archiveNextIdleBatch>;
      try {
        archivedSkills = deps.repos.skills.archiveNextIdleBatch({

---

32. apps/memos-local-plugin/core/skill/subscriber.ts (L278-L286)

The if/else conflates two unrelated concerns: 'should I warn?' and 'should I yield?'. On the final iteration the else branch is skipped because the while-condition immediately becomes false — correct intent, but hard to reason about. Separate the concerns: unconditionally yield between productive batches, and warn once after the loop if the limit was reached.

💡 Suggested Change

Before:

      if (batchesProcessed === IDLE_ARCHIVE_MAX_BATCHES_PER_TICK) {
        log.warn("skill.idle_archive_batch_limit_reached", {
          batchCount: batchesProcessed,
          archivedCount: archivedTotal,
          batchSize: IDLE_ARCHIVE_BATCH_LIMIT,
        });
      } else {
        await yieldToEventLoop();
      }

After:

      await yieldToEventLoop();
    }
    if (batchesProcessed >= IDLE_ARCHIVE_MAX_BATCHES_PER_TICK && archivedTotal > 0) {
      log.warn("skill.idle_archive_batch_limit_reached", {
        batchCount: batchesProcessed,
        archivedCount: archivedTotal,
        batchSize: IDLE_ARCHIVE_BATCH_LIMIT,
      });

---

33. apps/memos-local-plugin/core/skill/subscriber.ts (L40)

The JSDoc hardcodes 'ten' in prose, so it silently goes stale if the constant value changes. Either drop the comment (the name IDLE_ARCHIVE_MAX_BATCHES_PER_TICK is self-documenting) or make it value-agnostic and explain the why.

💡 Suggested Change

Before:

/** Bound one lifecycle pass to ten repository-sized archival batches. */

After:

/** Cap archival batches per lifecycle tick to avoid starving the event loop. */

---

34. apps/memos-local-plugin/core/storage/migrations/018-traces-ts-index.sql (L26)

No ANALYZE after index creation. SQLite's query planner uses statistics from sqlite_stat1; on an existing large traces table (the described ~30k row / ~235 MB case) these stats may be absent or stale. Without fresh statistics the planner may not choose the new index on the very first query after upgrade — exactly the boot-time latestTimestamp() call inside runMigrations → ensureHubSharingSearchColumns path. Adding ANALYZE traces; immediately after the CREATE INDEX statement ensures the planner sees accurate cardinality data from the start.

---

35. apps/memos-local-plugin/core/storage/repos/traces.ts (L109-L111)

The first generic argument types the bound parameters. Since this query has no parameters, unknown is inaccurate — it signals "parameter type is unknown" rather than "no parameters". The repo's own style uses concrete types (e.g. { id: string } in the adjacent selectById). Prefer [] or Record<string, never> to match better-sqlite3 convention for zero-param statements.

💡 Suggested Change

Before:

  const selectLatestTimestamp = db.prepare<unknown, { ts: number }>(
    `SELECT ts FROM traces ORDER BY ts DESC, id DESC LIMIT 1`,
  );

After:

  const selectLatestTimestamp = db.prepare<[], { ts: number }>(
    `SELECT ts FROM traces ORDER BY ts DESC, id DESC LIMIT 1`,
  );

---

36. apps/memos-local-plugin/tests/unit/skill/_helpers.ts (L43)

This magic number duplicates the production default from core/config/defaults.ts (also modified in this PR). If the canonical default changes, this test helper silently stays at 30 days and tests may no longer exercise the real default. Import the constant from defaults.ts instead of repeating the literal.

💡 Suggested Change

Before:

    idleArchiveMs: 30 * 24 * 60 * 60 * 1000,

After:

    idleArchiveMs: DEFAULT_SKILL_CONFIG.idleArchiveMs,

---

37. apps/memos-local-plugin/install.ps1 (L67-L72)

The throw here produces a raw PowerShell ErrorRecord with a full stack trace when unhandled, burying the human-readable message. The pattern is also repeated in Install-OpenClaw where Stop-Die was replaced with a bare throw. Consider emitting a coloured Write-Host before throwing, or a small helper (Throw-WithMessage) so the user sees a clear error without a stack dump.

---

38. apps/memos-local-plugin/install.ps1 (L510-L515)

State keys are bare string literals ("inactive", "needs_recovery", "final_failed"). PS 5.1 has no enum type; a single-character typo in any assignment or in the finally comparison silently evaluates the wrong branch. The final_failed branch is load-bearing — a typo there causes a spurious second gateway start on failure. Define constants at the top of the function and reference only those:

$GW_INACTIVE       = 'inactive'
$GW_NEEDS_RECOVERY = 'needs_recovery'
$GW_FINAL_FAILED   = 'final_failed'

---

39. apps/memos-local-plugin/install.ps1 (L666-L673)

The inner catch correctly prevents a secondary recovery exception from masking the original install error. However, if the recovery block grows (e.g. a log flush is added after this try/catch), an exception from new code would escape the finally and silently replace the original error. Document the invariant in a comment, or wrap the entire finally body in an outer try/catch:

} finally {
    try {
        if ($GatewayRecoveryState -eq "needs_recovery") { … }
    } catch {
        Write-Warn "Unexpected error in recovery cleanup: $($_.Exception.Message)"
    }
}

---

40. apps/memos-local-plugin/install.ps1 (L522-L524)

The original Stop-Die presumably printed a formatted, coloured message before exiting. The bare throw here triggers the finally-block recovery correctly, but the user-facing error is now buried in a PS ErrorRecord stack trace. Emit a Write-Host/Write-Error before throwing so the message remains visible regardless of how the caller handles the exception.

---

41. apps/memos-local-plugin/install.ps1 (L684-L692)

The two critical recovery paths — (a) deploy fails after gateway stop → recovery start called exactly once, and (b) final gateway start fails → recovery does NOT start again (final_failed) — have no automated test coverage. A regression in either path leaves the user with a silently stopped gateway. Add Pester tests mocking Invoke-OpenClawGatewayChecked and Deploy-Tarball covering the happy path, the deploy-failure recovery path, and the final-start-failure path.

---

42. apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py (L1072-L1074)

If SHARED_BRIDGE_REGISTRY.close_all() raises, self._mode_patch.stop() is never reached and the MEMOS_HERMES_BRIDGE_MODE env-var patch leaks into every subsequent test in the process.

Fix: register the stop via addCleanup in setUp immediately after start() — addCleanup is guaranteed to run even when tearDown raises.

def setUp(self) -> None:
    memos_provider.SHARED_BRIDGE_REGISTRY.close_all()
    self._mode_patch = patch.dict("os.environ", {"MEMOS_HERMES_BRIDGE_MODE": "legacy"})
    self._mode_patch.start()
    self.addCleanup(self._mode_patch.stop)  # guaranteed even if tearDown raises

def tearDown(self) -> None:
    memos_provider.SHARED_BRIDGE_REGISTRY.close_all()
    # _mode_patch.stop() handled by addCleanup above

---

43. apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py (L1137-L1139)

The trace and world_model variants of this test both guard with self.assertTrue(parsed["found"]) before probing nested fields. The policy variant skips that guard and jumps straight to parsed["body"].

If the provider returns {"found": False, ...} for the policy kind (e.g. a routing bug silently returns the not-found path), parsed["body"] will either be absent (raising KeyError) or empty — but the raw-string assertIn on _CH_POLICY_TITLE may still pass if the title leaked into an error message. This makes the policy branch a weaker regression guard than the other two.

Add self.assertTrue(parsed["found"]) before the assertIn on parsed["body"].

💡 Suggested Change

Before:

        self.assertIn(ChineseToolResultBridge._CH_POLICY_TITLE, raw)
        parsed = json.loads(raw)
        self.assertIn(ChineseToolResultBridge._CH_POLICY_BODY, parsed["body"])

After:

        self.assertIn(ChineseToolResultBridge._CH_POLICY_TITLE, raw)
        parsed = json.loads(raw)
        self.assertTrue(parsed["found"])
        self.assertIn(ChineseToolResultBridge._CH_POLICY_BODY, parsed["body"])

---

44. apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py (L1205-L1207)

The list variant checks parsed structure (parsed["worldModels"][0]["title"]), proving encoding end-to-end. The query variant only checks the raw string with assertIn and then parsed["queried"]. If the world-model titles inside the hits were serialized with ensure_ascii=True while another field still contained the Chinese literal verbatim, the raw-string check would pass while the actual regression went undetected.

Add a structural assertion on the parsed hits. The bridge's memory.search response for this path returns snippet containing _CH_WORLD_TITLE, so:

self.assertTrue(parsed["queried"])
self.assertIn(
    ChineseToolResultBridge._CH_WORLD_TITLE,
    parsed["hits"][0]["snippet"],
)

💡 Suggested Change

Before:

        self.assertIn(ChineseToolResultBridge._CH_WORLD_TITLE, raw)
        parsed = json.loads(raw)
        self.assertTrue(parsed["queried"])

After:

        self.assertIn(ChineseToolResultBridge._CH_WORLD_TITLE, raw)
        parsed = json.loads(raw)
        self.assertTrue(parsed["queried"])
        self.assertIn(
            ChineseToolResultBridge._CH_WORLD_TITLE,
            parsed["hits"][0]["snippet"],
        )

---

🧹 Filtered 10 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 3, duplicate: 7).

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (36/36 executed). memos_local_plugin/changed-repo-python: 36/36. Duration: 4s [advisory, non-gating] AI-generated tests on branch test/auto-gen-2bc64e976fa083c0-20260826213933: 17/44 passed, 27 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix-local-plugin-260824