Skip to content

Fix #1543: fix: MEMOS不支持JSON5格式的openclaw.json#2048

Open
Memtensor-AI wants to merge 1 commit into
MemTensor:mainfrom
Memtensor-AI:bugfix/autodev-1543
Open

Fix #1543: fix: MEMOS不支持JSON5格式的openclaw.json#2048
Memtensor-AI wants to merge 1 commit into
MemTensor:mainfrom
Memtensor-AI:bugfix/autodev-1543

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixes MemOS issue #1543: the memos-local-openclaw-plugin used raw JSON.parse when reading the user's openclaw.json, so any config with // line comments, single-quoted strings or trailing commas caused SyntaxError: Expected double-quoted property name at startup, silently disabling the tools.allow patch step and blocking plugin tools.

The fix introduces a small shared helper apps/memos-local-openclaw/src/shared/openclaw-config.ts that parses openclaw.json with json5@^2.2.3 (strict JSON is a subset, so nothing regresses), and routes all runtime + script read sites through it: index.ts (tools.allow patch), src/shared/llm-call.ts, src/ingest/providers/index.ts, src/viewer/server.ts, plus the four scripts/*.ts maintenance scripts. The regex-based tools.allow insertion is left untouched for the common double-quoted case; when the regex cannot safely locate the insertion point, we now log a clear warn asking the user to add "group:plugins" manually instead of silently doing nothing.

Test evidence: npx tsc --noEmit clean; new file tests/openclaw-json5.test.ts adds 11 cases covering line/block comments, single-quoted keys/values, trailing commas, unquoted identifier keys, and end-to-end loadOpenClawFallbackConfig on a JSON5 config — 11/11 pass. Existing tests/openclaw-fallback.test.ts (16/16), tests/viewer-config.test.ts, tests/config.test.ts, tests/integration.test.ts (26/26) all still pass. The 5 unrelated failures in the full vitest run (task-processor, accuracy, skill-auto-install, update-install) touch none of the modified files; they are pre-existing environmental flakes (native binding / process signals / embedding accuracy).

Related Issue (Required): Fixes #1543

Type of change

Please delete options that are not relevant.

  • 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?

Automated tests are pending.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

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)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

@MatthewZhuang, @CarltonXiang, @syzsunshine219, @World-controller please review this PR.

Reviewer Checklist

The memos-local-openclaw plugin failed to patch tools.allow at startup
whenever the user's openclaw.json contained // comments, single-quoted
strings or trailing commas, because every read site used JSON.parse.

Add a tiny shared helper (src/shared/openclaw-config.ts) that parses with
json5 (strict JSON is a subset, so nothing regresses) and route all
runtime + script read sites through it. The regex-based tools.allow
insertion is unchanged for the common double-quoted case; when the
regex cannot safely locate the insertion point we now warn asking the
user to add "group:plugins" manually instead of silently doing nothing.

Adds json5@^2.2.3 to plugin deps and 11 vitest cases covering line and
block comments, single-quoted keys/values, trailing commas, unquoted
identifier keys, and end-to-end loadOpenClawFallbackConfig on a JSON5
config.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2048
Task: fde1c36c0dd31bb1
Base: dev-20260624-v2.0.22
Head: bugfix/autodev-1543

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


1. apps/memos-local-openclaw/scripts/run-accuracy-test.ts (L42)

Avoid as any cast on unknown return value.

parseOpenClawConfig intentionally returns unknown so TypeScript forces callers to narrow the type before use. Casting it straight to any silences all type-checking on the subsequent property chain and defeats that safety net.

Additionally, the readOpenClawConfig helper already wraps both the fs.readFileSync call and parseOpenClawConfig, so this call-site duplicates that logic.

Consider using readOpenClawConfig with a typed assertion or interface instead:

interface OpenClawConfig {
  plugins?: {
    entries?: Record<string, { config?: Record<string, unknown> }>;
  };
}
const raw = readOpenClawConfig(cfgPath) as OpenClawConfig;
return raw?.plugins?.entries?.["memos-local-openclaw-plugin"]?.config ?? {};

This keeps the JSON5 tolerance, eliminates the duplicated read logic, and preserves type safety.

💡 Suggested Change

Before:

  const raw = parseOpenClawConfig(fs.readFileSync(cfgPath, "utf-8")) as any;

After:

  const raw = readOpenClawConfig(cfgPath) as OpenClawConfig;

2. apps/memos-local-openclaw/index.ts (L363)

as any cast discards type safety unnecessarily.

parseOpenClawConfig already returns unknown, which is the correct safe return type. Casting straight to any here bypasses all TypeScript checks on everything downstream (e.g., cfg?.tools?.allow is never verified).

Consider defining a typed interface for the config shape and casting to that, or at least keep it as unknown and use a type guard:

interface OpenClawConfig {
  tools?: { allow?: string[] };
}

Then:

const cfg = parseOpenClawConfig(raw) as OpenClawConfig;
💡 Suggested Change

Before:

        const cfg = parseOpenClawConfig(raw) as any;

After:

        const cfg = parseOpenClawConfig(raw) as { tools?: { allow?: string[] } };

3. apps/memos-local-openclaw/index.ts (L366-L370)

Regex patch is broken for JSON5 single-quoted strings.

After successfully parsing the file with JSON5 (which accepts 'group:something'), the patch regex is built with JSON.stringify(allow[allow.length - 1]), which always produces a double-quoted string (e.g., "my-entry"). If the raw file uses single quotes (valid JSON5), the regex will never match, the patched !== raw guard will be false, and the warn branch will fire on every plugin init — even though the file was parsed correctly.

Since the file is already read as raw text and the parsed value is known, consider writing back a fully-serialized JSON (not regex-based) result, or at minimum also try matching the single-quoted variant:

const lastEntryDoubleQuoted = JSON.stringify(allow[allow.length - 1]);
const lastEntrySingleQuoted = `'${allow[allow.length - 1]}'`;
const pattern = new RegExp(
  `(${lastEntryDoubleQuoted}|${lastEntrySingleQuoted})(\\s*\\])`
);

Or, more robustly, re-serialize the entire config to standard JSON after adding the entry.


4. apps/memos-local-openclaw/scripts/run-accuracy-test.ts (L42)

Avoid as any cast on unknown return value.

parseOpenClawConfig intentionally returns unknown so TypeScript forces callers to narrow the type before use. Casting it straight to any silences all type-checking on the subsequent property chain and defeats that safety net.

Additionally, the readOpenClawConfig helper already wraps both the fs.readFileSync call and parseOpenClawConfig, so this call-site duplicates that logic.

Consider using readOpenClawConfig with a typed interface instead:

interface OpenClawConfig {
  plugins?: {
    entries?: Record<string, { config?: Record<string, unknown> }>;
  };
}
const raw = readOpenClawConfig(cfgPath) as OpenClawConfig;
return raw?.plugins?.entries?.["memos-local-openclaw-plugin"]?.config ?? {};

This keeps the JSON5 tolerance, eliminates the duplicated read logic, and preserves type safety.


5. apps/memos-local-openclaw/scripts/refresh-summaries.ts (L56)

The parseOpenClawConfig helper intentionally returns unknown to enforce proper type narrowing, but immediately casting it to any defeats that type safety entirely. Any subsequent property access (e.g., config.plugins?.entries?.[...]) becomes unchecked and can silently produce undefined chains rather than surfacing type errors at compile time.

Consider defining a typed interface (or Zod/io-ts schema) for the config shape and performing a proper type narrowing or assertion, rather than using as any. For example:

interface OpenClawConfig {
  plugins?: {
    entries?: Record<string, { config?: unknown }>;
    configs?: Record<string, { config?: unknown }>;
  };
}

const config = parseOpenClawConfig(fs.readFileSync(configPath, "utf-8")) as OpenClawConfig;

Also, the readOpenClawConfig helper in openclaw-config.ts already wraps fs.readFileSync + parseOpenClawConfig in one call, so the inline fs.readFileSync here is redundant boilerplate that could be simplified to:

const config = readOpenClawConfig(configPath) as OpenClawConfig;

6. apps/memos-local-openclaw/scripts/refresh-skill.ts (L28)

The as any cast suppresses TypeScript's type safety on the parsed config object without any explanatory comment. Per the review checklist, any usage must be accompanied by a comment explaining why it is necessary.

Additionally, openclaw-config.ts already exports a readOpenClawConfig helper that combines fs.readFileSync + parseOpenClawConfig into a single call. Using it here (and in the other identical call sites) would reduce repetition — though it would still need the as any cast with an explanation.

Suggestion: add a brief comment and consider using readOpenClawConfig instead.

💡 Suggested Change

Before:

  const oc = parseOpenClawConfig(fs.readFileSync(`${stateDir}/openclaw.json`, "utf-8")) as any;

After:

  // Cast to `any` so we can traverse the loosely-typed config structure with
  // optional chaining; the actual values are validated before use below.
  const oc = parseOpenClawConfig(fs.readFileSync(`${stateDir}/openclaw.json`, "utf-8")) as Record<string, any>;

7. apps/memos-local-openclaw/scripts/refresh-skill.ts (L28)

The as any cast suppresses TypeScript's type safety on the parsed config object without any explanatory comment. Per the review checklist, any usage must be accompanied by a comment explaining why it is necessary.

Additionally, openclaw-config.ts already exports a readOpenClawConfig helper that combines fs.readFileSync + parseOpenClawConfig in a single call. The same two-step pattern (parseOpenClawConfig(fs.readFileSync(...))) is repeated across all 6 updated call sites — none of them use readOpenClawConfig, making that helper dead code.

Suggested fix:

  1. Use readOpenClawConfig(path) instead of parseOpenClawConfig(fs.readFileSync(path, "utf-8")) at every call site to avoid duplication and make the dedicated helper useful.
  2. Add a brief inline comment at the as any cast explaining the reason, e.g.:
// Cast to `any` to traverse the loosely-typed config tree with optional
// chaining; actual values are validated/narrowed before use.
const oc = readOpenClawConfig(`${stateDir}/openclaw.json`) as Record<string, any>;

8. apps/memos-local-openclaw/scripts/refresh-skill.ts (L28)

Two issues with this line:

  1. as any without explanation: Per the review checklist, any usage must be accompanied by a comment explaining why it is necessary. A narrower cast like as Record<string, any> (with a brief comment) would be clearer.

  2. readOpenClawConfig helper is dead code: openclaw-config.ts exports a readOpenClawConfig convenience function that combines fs.readFileSync + parseOpenClawConfig in one call. Yet all 6 updated call sites (this file + refresh-summaries.ts, run-accuracy-test.ts, test-agent-isolation.ts, llm-call.ts, server.ts, index.ts) repeat the two-step pattern manually, leaving readOpenClawConfig unused.

Suggested fix — import and use the dedicated helper, and add an explanatory comment:

// Cast to `any` to traverse the loosely-typed config tree with optional
// chaining; actual values are validated/narrowed before use.
const oc = readOpenClawConfig(`${stateDir}/openclaw.json`) as Record<string, any>;

9. apps/memos-local-openclaw/scripts/test-agent-isolation.ts (L65)

Dead code / unused export: readOpenClawConfig is exported from openclaw-config.ts as a convenience wrapper that reads and parses the config in one call, but every call site still manually invokes fs.readFileSync(cfgPath, "utf-8") before calling parseOpenClawConfig. The helper is therefore never referenced anywhere in the codebase.

Suggestion: Replace all parseOpenClawConfig(fs.readFileSync(cfgPath, "utf-8")) usages with readOpenClawConfig(cfgPath) to reduce duplication and actually use the provided helper, or remove readOpenClawConfig from the export if it is intentionally unused.

💡 Suggested Change

Before:

  const raw = parseOpenClawConfig(fs.readFileSync(cfgPath, "utf-8")) as any;

After:

  const raw = readOpenClawConfig(cfgPath) as any;

10. apps/memos-local-openclaw/scripts/test-agent-isolation.ts (L65)

Avoid as any: parseOpenClawConfig intentionally returns unknown to enforce type-safe access, but casting immediately to any with as any defeats that protection — TypeScript will no longer catch typos or shape mismatches on any downstream property access (e.g., raw?.plugins?.entries?.[...]).

Suggestion: Either define a typed interface for the config shape and cast to that (as OpenClawConfigFile), or access the needed nested property through a typed accessor. Using as unknown as OpenClawConfigFile with a defined type is safer and conveys intent. At minimum, add a comment explaining why any is acceptable here if the untyped approach is kept intentionally.


11. apps/memos-local-openclaw/src/shared/llm-call.ts (L58)

The as any cast lacks an explanatory comment, which violates the project's TypeScript type checklist rule: "Avoid using any type; if necessary, provide a comment explaining the reason."

Note: JSON.parse previously returned an implicit any, but making it an explicit as any cast with no justification is a step backward in type safety. Consider either adding a comment or defining a minimal interface for the raw config shape:

interface OpenClawRawConfig {
  agents?: { defaults?: { model?: { primary?: string } } };
}
const raw = parseOpenClawConfig(fs.readFileSync(cfgPath, "utf-8")) as OpenClawRawConfig;

12. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L19-L22)

Dead Code: readOpenClawConfig is never used.

Reviewing all the other changed files in this PR (index.ts, llm-call.ts, providers/index.ts, server.ts, and all scripts), every call site imports and calls parseOpenClawConfig directly — reading the file via fs.readFileSync inline. readOpenClawConfig is exported here but imported nowhere.

This is dead code that adds maintenance surface with no benefit. Consider one of:

  1. Remove it if there are no other consumers outside this PR.
  2. Migrate call sites to use it, replacing the repeated parseOpenClawConfig(fs.readFileSync(path, "utf-8")) pattern — which was the apparent intent of this helper.

13. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L19-L22)

Dead Code: readOpenClawConfig is never used.

Reviewing all the other changed files in this PR (index.ts, llm-call.ts, providers/index.ts, server.ts, and all scripts), every call site imports and calls parseOpenClawConfig directly — reading the file via fs.readFileSync inline. readOpenClawConfig is exported here but imported nowhere.

This is dead code that adds maintenance surface with no benefit. Consider one of:

  1. Remove it if there are no other consumers outside this PR.
  2. Migrate call sites to use it, replacing the repeated parseOpenClawConfig(fs.readFileSync(path, "utf-8")) pattern — which was the apparent intent of this helper.

14. apps/memos-local-openclaw/src/viewer/server.ts (L653)

TypeScript any type usage without explanation comment: The as any cast suppresses TypeScript's type safety. Per the review checklist, if any is necessary, a comment explaining the reason should be provided inline.

The return type of parseOpenClawConfig is already unknown (as designed), so the cast to any is used here to allow property access via optional chaining (raw?.plugins?.entries). A safer approach would be to cast to a specific type or at least add an inline comment explaining why any is needed here.

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2048
Task: fde1c36c0dd31bb1
Base: dev-20260624-v2.0.22
Head: bugfix/autodev-1543

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


1. apps/memos-local-openclaw/index.ts (L368)

"group:plugins" is hardcoded in three separate places within the same logical block (the guard condition, the replacement string, and the verification check). A single typo in any one of them would silently break the feature — the guard could pass while the wrong string is injected, or the verification would never match. Consider extracting it into a named constant:

const PLUGINS_GROUP_ENTRY = "group:plugins";

Then use PLUGINS_GROUP_ENTRY in all three places.


2. apps/memos-local-openclaw/index.ts (L375)

The regex ("'${escapedValue}["'])(\s*\]) will silently fail for one of the most common JSON5 patterns — a trailing comma on the last array entry, e.g.:

"allow": [
  "memos:search",
  "memos:recall",   // trailing comma — perfectly valid JSON5
]

Here the raw text contains "memos:recall",], but \s*\] does not allow for the , between the value and ], so patched === raw and the warn path always fires for users who write idiomatic JSON5. Since the PR's stated motivation is tolerating JSON5 style, this is a gap.

Fix: make the trailing comma optional in the second capture group:

new RegExp(`(["']${escapedValue}["'])(,?\\s*\\])`),
//                                      ^^  optional trailing comma

The replacement $1,\n "group:plugins"$2 still emits a comma before the new entry regardless of whether $2 starts with ,.

💡 Suggested Change

Before:

            new RegExp(`(["']${escapedValue}["'])(\\s*\\])`),

After:

            new RegExp(`(["']${escapedValue}["'])(,?\\s*\\])`,

3. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L49-L51)

Parse errors from JSON5.parse carry no file path context. A syntax error will surface as e.g. JSON5: invalid character '/' at 3:5 with no indication of which file triggered it. Since users routinely hand-edit these files (see issue #1543 referenced in the comment), this will be a common and hard-to-diagnose failure.

Suggested fix: wrap read and parse in separate try/catch blocks so callers get distinct, path-contextual messages:

export function readOpenClawConfig(configPath: string): unknown {
  let raw: string;
  try {
    raw = fs.readFileSync(configPath, "utf-8");
  } catch (err) {
    throw new Error(
      `Failed to read openclaw config at "${configPath}": ${(err as Error).message}`
    );
  }
  try {
    return parseOpenClawConfig(raw);
  } catch (err) {
    throw new Error(
      `Failed to parse openclaw config at "${configPath}": ${(err as Error).message}`
    );
  }
}

4. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L7-L8)

"narrow" is the wrong term here. as OpenClawConfig is a type assertion (a compile-time cast with no runtime check), not TypeScript type narrowing (which uses runtime predicates like typeof/instanceof). This wording could mislead future contributors into thinking the cast provides a runtime safety guarantee.

Minimal fix — update the wording:

 * callers can assert the result as OpenClawConfig instead of `as any`,
 * gaining IDE autocomplete with no runtime validation.

Preferred fix — add a lightweight type guard:

export function isOpenClawConfig(v: unknown): v is OpenClawConfig {
  return typeof v === "object" && v !== null;
}
💡 Suggested Change

Before:

* plugin so callers can narrow with a single `as OpenClawConfig` instead of
* `as any`, while still leaving deeper fields loosely typed (they come from

After:

 * we only type the branches used inside the
+ * plugin so callers can assert the result as OpenClawConfig instead of
+ * `as any` (type assertion only — no runtime validation), while still leaving deeper fields loosely typed (they come from

5. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L48-L49)

fs.readFileSync blocks the Node.js event loop for the duration of the disk read. This is acceptable for one-time startup use, but undocumented — a future caller inside a request handler or a loop would unknowingly block the event loop.

Suggested fix — add a JSDoc annotation:

/**
 * Read + parse an openclaw.json file with JSON5 tolerance.
 *
 * **Synchronous** — intended for one-time startup use only.
 * For use inside request handlers or hot paths, prefer
 * `fs.promises.readFile` + `parseOpenClawConfig` with `await`.
 */

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2048
Task: fde1c36c0dd31bb1
Base: dev-20260624-v2.0.22
Head: bugfix/autodev-1543

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


1. apps/memos-local-openclaw/scripts/test-agent-isolation.ts (L64)

The result of readOpenClawConfig is cast to OpenClawConfig with as, but readOpenClawConfig performs no runtime validation — it only parses the file as JSON5. If the on-disk config is missing or has an unexpected shape (e.g., plugins.entries uses a different key), the cast silently succeeds and the code later falls back to {} via ?? {}, which is fine. However, subsequent call sites that consume deeper fields without optional chaining could still throw at runtime with an opaque message.

Consider adding a lightweight schema validation (e.g., zod) inside readOpenClawConfig, or at minimum document here that the as assertion is intentional and safe only because every field access below uses optional chaining.


2. apps/memos-local-openclaw/index.ts (L380)

The regex uses independent ["'] character classes on both sides, allowing mismatched delimiters like "value' or 'value". Use a backreference to pin the closing quote to the opening one.

Suggested fix:

new RegExp(`(["'])${escapedValue}\\1(,?\\s*\\])`),
`$1${lastValue}$1,\n      "${PLUGINS_GROUP_ENTRY}"$2`,

Note that capture group numbering shifts: $1 becomes the quote character and $2 the trailing bracket match.

💡 Suggested Change

Before:

new RegExp(`(["']${escapedValue}["'])(,?\\s*\\])`),

After:

new RegExp(`(["'])${escapedValue}\\1(,?\\s*\\])`),
            `$1${lastValue}$1,\n      "${PLUGINS_GROUP_ENTRY}"$2`,

3. apps/memos-local-openclaw/index.ts (L370)

parseOpenClawConfig is declared to return unknown, yet the result is immediately cast to the non-nullable OpenClawConfig. This hides the fact that the function can theoretically return null or an unexpected shape (e.g., if the JSON5 file contained a top-level null). The very next line already uses optional chaining (cfg?.tools), which signals the developer knows the value could be absent.

Prefer a nullable cast to keep TypeScript's safety net intact:

const cfg = parseOpenClawConfig(raw) as OpenClawConfig | null;

This way TypeScript will continue to enforce the ?. guards at all future call sites.

💡 Suggested Change

Before:

        const cfg = parseOpenClawConfig(raw) as OpenClawConfig;

After:

        const cfg = parseOpenClawConfig(raw) as OpenClawConfig | null;

4. apps/memos-local-openclaw/src/viewer/server.ts (L653)

readOpenClawConfig intentionally returns unknown (type assertion only — no runtime validation, per the JSDoc). Casting the result to OpenClawConfig with as OpenClawConfig silently bypasses TypeScript's type safety — the compiler won't catch an incorrect or partially-missing config shape. The JSDoc already acknowledges this is a deliberate tradeoff, but callers should at least not double-down on it by re-asserting at the call site after already typing it as unknown. The cast is necessary here to use the result, but consider narrowing the cast to only what's needed (e.g. as Pick<OpenClawConfig, 'plugins'>) or adding a runtime guard for the specific field before use.


5. apps/memos-local-openclaw/src/viewer/server.ts (L657)

pluginCfg is typed as Record<string, unknown>, so taskAutoFinalizeHours is unknown at runtime. The as number cast satisfies the compiler but provides zero runtime safety — the value could be null, a string like "24", or a boolean, all of which would silently produce NaN or 0 instead of a valid timeout. Replace the !== undefined check with a typeof === "number" guard, which both validates at runtime and narrows the type automatically without needing the cast.

💡 Suggested Change

Before:

        if (pluginCfg.taskAutoFinalizeHours !== undefined) return (pluginCfg.taskAutoFinalizeHours as number) * 60 * 60 * 1000;

After:

        if (typeof pluginCfg.taskAutoFinalizeHours === "number") return pluginCfg.taskAutoFinalizeHours * 60 * 60 * 1000;

6. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L64-L76)

Unsafe error cast — .message can be undefined

Both catch blocks cast the thrown value directly to Error via (err as Error).message. In JavaScript any value can be thrown (throw "some string", throw 42, throw { code: 'ENOENT' }), so if fs.readFileSync or JSON5.parse throws a non-Error, .message is undefined and the user sees:

Failed to read openclaw config at "...": undefined

This is the exact opposite of the improvement described in issue #1543 — the user gets less information than before.

Suggested fix: Use a safe helper that falls back to String(err):

function toMessage(err: unknown): string {
  return err instanceof Error ? err.message : String(err);
}

Then replace both occurrences:

throw new Error(
  `Failed to read openclaw config at "${configPath}": ${toMessage(err)}`,
);

This handles Error instances, plain strings, and anything else gracefully.


7. apps/memos-local-openclaw/src/viewer/server.ts (L653)

readOpenClawConfig intentionally returns unknown (type assertion only — no runtime validation, per its JSDoc). Casting the result to OpenClawConfig here with as OpenClawConfig silently bypasses TypeScript's type safety — the compiler won't catch an incorrect or partially missing config shape at runtime. Consider replacing the cast with a typeof === 'number' guard on the specific field you actually use (see the comment on taskAutoFinalizeHours below), which eliminates the need for this top-level assertion entirely.


8. apps/memos-local-openclaw/src/viewer/server.ts (L657)

pluginCfg values are unknown at runtime (sourced from unvalidated user JSON). The as number cast satisfies the compiler but provides zero runtime safety — the value could be null, a string "24", or a boolean, all of which silently produce NaN or 0 instead of a valid timeout. Replace the !== undefined check with a typeof === "number" guard, which validates at runtime and narrows the type automatically without needing the cast.

💡 Suggested Change

Before:

        if (pluginCfg.taskAutoFinalizeHours !== undefined) return (pluginCfg.taskAutoFinalizeHours as number) * 60 * 60 * 1000;

After:

        if (typeof pluginCfg.taskAutoFinalizeHours === "number") return pluginCfg.taskAutoFinalizeHours * 60 * 60 * 1000;

9. apps/memos-local-openclaw/src/viewer/server.ts (L653)

readOpenClawConfig intentionally returns unknown (type assertion only — no runtime validation, per its JSDoc). Casting the result with as OpenClawConfig here silently bypasses TypeScript's type safety — the compiler will not catch an incorrect or partially missing config shape at runtime. Consider removing this top-level assertion and instead using a typeof === 'number' guard directly on the specific field accessed (see the taskAutoFinalizeHours line below), which eliminates the need for this cast entirely.


10. apps/memos-local-openclaw/src/viewer/server.ts (L657)

pluginCfg values are unknown at runtime (sourced from unvalidated user JSON). The as number cast satisfies the compiler but provides zero runtime safety — the value could be null, a string "24", or a boolean, all of which would silently produce NaN or 0 instead of a valid timeout. Replace the !== undefined check with a typeof === "number" guard, which validates at runtime and narrows the type automatically without needing the cast.

💡 Suggested Change

Before:

        if (pluginCfg.taskAutoFinalizeHours !== undefined) return (pluginCfg.taskAutoFinalizeHours as number) * 60 * 60 * 1000;

After:

        if (typeof pluginCfg.taskAutoFinalizeHours === "number") return pluginCfg.taskAutoFinalizeHours * 60 * 60 * 1000;

11. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L66-L77)

Unsafe error cast — .message can be undefined

Both catch blocks cast the thrown value directly to Error via (err as Error).message. In JavaScript, anything can be thrown (a plain string, a number, an object without .message), so if fs.readFileSync or JSON5.parse throws a non-Error, .message is undefined and the user sees:

Failed to read openclaw config at "...": undefined

This defeats the purpose of the error-enrichment described in the JSDoc (and issue #1543).

Suggested fix: use a safe helper or an inline guard:

const msg = err instanceof Error ? err.message : String(err);
throw new Error(`Failed to read openclaw config at "${configPath}": ${msg}`);

Apply the same pattern to both catch blocks.

💡 Suggested Change

Before:

  } catch (err) {
    throw new Error(
      `Failed to read openclaw config at "${configPath}": ${(err as Error).message}`,
    );
  }
  try {
    return parseOpenClawConfig(raw);
  } catch (err) {
    throw new Error(
      `Failed to parse openclaw config at "${configPath}": ${(err as Error).message}`,
    );
  }

After:

  } catch (err) {
    const msg = err instanceof Error ? err.message : String(err);
    throw new Error(
      `Failed to read openclaw config at "${configPath}": ${msg}`,
    );
  }
  try {
    return parseOpenClawConfig(raw);
  } catch (err) {
    const msg = err instanceof Error ? err.message : String(err);
    throw new Error(
      `Failed to parse openclaw config at "${configPath}": ${msg}`,
    );
  }

12. apps/memos-local-openclaw/src/viewer/server.ts (L653)

readOpenClawConfig intentionally returns unknown (type assertion only — no runtime validation, per its JSDoc). Casting the result with as OpenClawConfig here silently bypasses TypeScript's type safety — the compiler will not catch an incorrect or partially missing config shape at runtime. Consider removing this top-level assertion and instead using a typeof === 'number' guard directly on the specific field accessed (see the taskAutoFinalizeHours line below), which eliminates the need for this cast entirely.


13. apps/memos-local-openclaw/src/viewer/server.ts (L657)

pluginCfg values are unknown at runtime (sourced from unvalidated user JSON). The as number cast satisfies the compiler but provides zero runtime safety — the value could be null, a string "24", or a boolean, all of which would silently produce NaN or 0 instead of a valid timeout. Replace the !== undefined check with a typeof === "number" guard, which validates at runtime and narrows the type automatically without needing the cast.

💡 Suggested Change

Before:

        if (pluginCfg.taskAutoFinalizeHours !== undefined) return (pluginCfg.taskAutoFinalizeHours as number) * 60 * 60 * 1000;

After:

        if (typeof pluginCfg.taskAutoFinalizeHours === "number") return pluginCfg.taskAutoFinalizeHours * 60 * 60 * 1000;

14. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L68)

Unsafe error cast — .message can be undefined

Both catch blocks cast the thrown value directly to Error with (err as Error).message. In JavaScript, any value can be thrown (a plain string, a number, an object without .message), so when fs.readFileSync or JSON5.parse throws a non-Error, .message is undefined and the user sees:

Failed to read openclaw config at "...": undefined

This defeats the intent of the JSDoc comment (and issue #1543) which aims to give users actionable error output.

Suggested fix: use a type-guard before accessing .message:

} catch (err) {
  const msg = err instanceof Error ? err.message : String(err);
  throw new Error(
    `Failed to read openclaw config at "${configPath}": ${msg}`,
  );
}

Apply the same pattern to both catch blocks.

💡 Suggested Change

Before:

      `Failed to read openclaw config at "${configPath}": ${(err as Error).message}`,

After:

      `Failed to read openclaw config at "${configPath}": ${err instanceof Error ? err.message : String(err)}`,

15. apps/memos-local-openclaw/src/shared/openclaw-config.ts (L75)

Same issue as the first catch block — (err as Error).message will produce undefined in the error message if JSON5.parse throws a non-Error value. Use the same safe pattern:

  const msg = err instanceof Error ? err.message : String(err);
  throw new Error(`Failed to parse openclaw config at "${configPath}": ${msg}`);
💡 Suggested Change

Before:

      `Failed to parse openclaw config at "${configPath}": ${(err as Error).message}`,

After:

      `Failed to parse openclaw config at "${configPath}": ${err instanceof Error ? err.message : String(err)}`,

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

⚠️ Open Code Review automatic OCR fix limit reached

Open Code Review still found 15 issue(s), but the automatic OCR fix loop has reached 2/2 attempts.

I stopped auto-fixing this PR to avoid an infinite loop. Please review the latest OCR comment and fix or dismiss the remaining findings manually.

PR: #2048

@syzsunshine219 syzsunshine219 added the needs-audit Requires manual audit before merge label Jul 3, 2026
@syzsunshine219 syzsunshine219 changed the base branch from dev-20260624-v2.0.22 to main July 3, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-generated bug Something isn't working | 功能异常 needs-audit Requires manual audit before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: MEMOS不支持JSON5格式的openclaw.json

5 participants