Skip to content

Fix #2474: [Bug] memos-local-plugin: config validator reports schema-declared 'reasoning' k - #2475

Open
Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.36from
Memtensor-AI:bugfix/autodev-2474-20261008202913915
Open

Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.36from
Memtensor-AI:bugfix/autodev-2474-20261008202913915

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixes #2474 — the memos-local-plugin config validator falsely reported schema-declared reasoning fields (llm.reasoning, skillEvolver.reasoning, l3Llm.reasoning) as unknown config keys on every boot. The root cause was pruneUnknown in apps/memos-local-plugin/core/config/index.ts using DEFAULT_CONFIG as the "known keys" oracle, which misses any Type.Optional(...) field without a concrete default — the same defect class as #2247/#2248.

The fix rewrites pruneUnknown to walk the TypeBox ConfigSchema directly: it uses the properties map of type: "object" nodes to decide what is known, and treats patternProperties (TypeBox Type.Record) as a free-form map so llm.headers, logging.channels, and similar free-form sections continue to accept arbitrary child keys. The existing FREE_FORM_CONFIG_PATHS allowlist is preserved for backward compatibility. The warning text, pass-through semantics, and the downstream pipeline (deepMerge → Value.Default → Value.Errors) are all unchanged.

Verification: a new tests/unit/config/schema-driven-unknown-keys.test.ts adds 9 regression cases (reasoning no-warning across all three LLM slots, typos still warn, Record-based free-form maps unchanged, nested unknowns inside a known Optional block still warn, resolved config retains the reasoning block byte-for-byte). Full plugin unit suite 1599 pass + 1 skipped, integration suite 6/6, tsc -p tsconfig.json --noEmit clean.

Related Issue (Required): Fixes #2474

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

@whipser030, @hijzy please review this PR.

Reviewer Checklist

…2474)

`pruneUnknown` used DEFAULT_CONFIG as the "known keys" oracle, so every
schema-declared `Type.Optional(...)` field without a default value was
flagged on every boot:

    config.warning message="unknown config key 'llm.reasoning' (…)"
    config.warning message="unknown config key 'skillEvolver.reasoning' (…)"
    config.warning message="unknown config key 'l3Llm.reasoning' (…)"

`reasoning` is declared in LlmSchema / SkillEvolverSchema and the value
was applied end-to-end — only the warning was wrong. MemTensor#2248 patched the
same class of defect for `llm.maxTokens` / `llm.headers` by seeding
defaults, but every future optional field would reintroduce the bug.

Walk the TypeBox schema instead: use the `properties` map of
`type: "object"` nodes to decide what is known, and treat
`patternProperties` (TypeBox `Type.Record`) as a free-form map so
`llm.headers` / `logging.channels` continue to accept arbitrary child
keys. `FREE_FORM_CONFIG_PATHS` stays as an explicit allowlist for
backward compatibility. The `unknown config key …` warning text, the
pass-through semantics, and the downstream pipeline
(deepMerge/Value.Default/Value.Errors) are all preserved.

Tests: full plugin unit suite 1599 pass + 1 skipped (0 fail), the new
`schema-driven-unknown-keys.test.ts` adds 9 cases covering reasoning
(no warning), typos (still warn), free-form Record maps, and nested
unknowns inside a known Optional block. Integration suite 6/6 green.
`tsc -p tsconfig.json --noEmit` clean.

Fixes MemTensor#2474
@Memtensor-AI Memtensor-AI added ai:generated Generated or modified by AI | 由 AI 生成或修改 area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Oct 8, 2026
@Memtensor-AI

Memtensor-AI commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2475
Task: 85e5733f87f42cdc
Base: dev-v2.0.36
Head: bugfix/autodev-2474-20261008202913915
Head SHA: a94eb697c7a00ca8cbb307be2de153d1440183a7

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


1. apps/memos-local-plugin/core/config/index.ts (L400-L404)

Finding #2 is partially resolved: all boolean guards now use KindGuard.IsObject / KindGuard.IsRecord correctly. However, the one residual inline cast (node as { properties?: unknown }) still remains. Since KindGuard.IsObject(node) is a TypeBox type predicate that narrows node to TObject, importing TObject from @sinclair/typebox would let TypeScript resolve .properties directly without the extra cast — which is exactly what finding #2 requested.

Suggested fix:

  1. Add TObject (and optionally TSchema) to the import on line 14.
  2. Change the parameter type of schemaObjectProperties from unknown to TSchema | unknown or directly reference TObject after the guard.
💡 Suggested Change

Before:

function schemaObjectProperties(node: unknown): Record<string, unknown> | null {
  if (!KindGuard.IsObject(node)) return null;
  const props = (node as { properties?: unknown }).properties;
  return isPlainObject(props) ? (props as Record<string, unknown>) : null;
}

After:

import { KindGuard, Type, type TObject } from "@sinclair/typebox";

// …

function schemaObjectProperties(node: unknown): Record<string, unknown> | null {
  if (!KindGuard.IsObject(node)) return null;
  // After KindGuard.IsObject, `node` is narrowed to TObject;
  // no inline cast needed once TObject is imported.
  const props: TObject["properties"] = node.properties;
  return isPlainObject(props) ? (props as Record<string, unknown>) : null;
}

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🔧 Open Code Review requested Agent fix

Open Code Review found 2 issue(s). I have resumed the development Agent to fix them.

  • Task: 85e5733f87f42cdc
  • Fix attempt: 1/2
  • Finding delta: 0 repeated / 2 new / 0 likely resolved

The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed.

…Guard

Addresses the two OCR findings on MemTensor#2475:

1. `pruneUnknown` used to silently fall through when `known` was null —
   e.g. if a future schema refactor introduced a `Type.Union`,
   `Type.Intersect`, or `Type.Any` at a recursed-into node, every child key
   would be passed through verbatim with no warning. The guard
   `if (known && !(k in known))` short-circuited on falsy `known` and
   nothing emitted a diagnostic. Now the `known === null && !freeFormHere`
   case takes an early return with an explicit warning so the bypass is
   visible rather than silent. `Type.Record` nodes still skip the check
   via `freeFormHere`, so the current schema's behaviour is unchanged.

2. The three helpers `schemaObjectProperties`, `schemaAcceptsAnyKey`,
   `isObjectSchema` all repeated the same
   `(node as { type?: unknown }).type === "object"` inline cast. Replaced
   with TypeBox's own `KindGuard.IsObject` / `KindGuard.IsRecord` so the
   introspection stays aligned with TypeBox's internal representation.

Tests: tests/unit/config/ (109 tests) still pass; `npm run lint` is clean.
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Error details
All test phases passed (1 + 12 tests, exit 0 in both). No failures to analyze. [advisory, non-gating] AI-generated tests on branch test/auto-gen-85e5733f87f42cdc-20261009045918: 68/68 passed — these do NOT affect the PR verdict; review the branch manually.

Branch: bugfix/autodev-2474-20261008202913915

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated Generated or modified by AI | 由 AI 生成或修改 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.

2 participants