Skip to content

fix: parse autoRecallTimeoutMs in parsePluginConfig#466

Merged
rwmjhb merged 2 commits intoCortexReach:masterfrom
jlin53882:fix/autoRecallTimeoutMs-parse
Apr 7, 2026
Merged

fix: parse autoRecallTimeoutMs in parsePluginConfig#466
rwmjhb merged 2 commits intoCortexReach:masterfrom
jlin53882:fix/autoRecallTimeoutMs-parse

Conversation

@jlin53882
Copy link
Copy Markdown
Contributor

@jlin53882 jlin53882 commented Apr 2, 2026

Problem

autoRecallTimeoutMs is declared in the PluginConfig interface (line 105), and used at line 2261:

const AUTO_RECALL_TIMEOUT_MS = parsePositiveInt(config.autoRecallTimeoutMs) ?? 3_000;

However, it is completely missing from parsePluginConfig return object, so the user-supplied value is never parsed, always falls back to the hardcoded default of 3000ms, and the config validator rejects the setting with:

Config invalid
plugins.entries.memory-lancedb-pro.config: invalid config: must NOT have additional properties

Fix

  1. Schema (this PR): Add autoRecallTimeoutMs to openclaw.plugin.json configSchema.properties. Required because additionalProperties: false means the field must be listed in the schema to be accepted.

  2. Code (upstream): Add autoRecallTimeoutMs: parsePositiveInt(cfg.autoRecallTimeoutMs) ?? 3000 to the parsePluginConfig return object.

Verification

  • Schema: openclaw.plugin.jsonconfigSchema.properties.autoRecallTimeoutMs now exists
  • Config validator accepts autoRecallTimeoutMs without additionalProperties error
  • Line 105: autoRecallTimeoutMs?: number; - declared
  • Line 2261: parsePositiveInt(config.autoRecallTimeoutMs) ?? 5_000 - used

Changelog

  • Bug fix: autoRecallTimeoutMs now listed in plugin config schema
  • Users can now configure autoRecallTimeoutMs without validation errors

Environment

  • Plugin version: v1.1.0-beta.10

Closes #467

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@jlin53882
Copy link
Copy Markdown
Contributor Author

Closes #467

Copy link
Copy Markdown
Collaborator

@AliceLJY AliceLJY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Clean bug fix.

autoRecallTimeoutMs was declared in the PluginConfig interface and consumed at runtime, but never actually parsed in parsePluginConfig — meaning user config was silently ignored and always fell back to the 3000ms default. The schema addition in openclaw.plugin.json is also necessary since additionalProperties: false would reject the field otherwise.

Changes are minimal and correct. Approving and assigning to @rwmjhb for final merge.

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented Apr 3, 2026

This PR fixes the missing config field, but it also changes the effective default autoRecallTimeoutMs from 5000ms back to 3000ms for users who leave it unset. If 5000ms from #314 is still intended, parsePluginConfig should not default to 3000 here.

@jlin53882
Copy link
Copy Markdown
Contributor Author

Good point — addressed in the latest commit (fbcd5c8). Both parsePluginConfig and openclaw.plugin.json now use 5000ms as the default, consistent with the intent of Issue #314. Users can still override via config if needed.

Copy link
Copy Markdown

app3apps commented Apr 5, 2026

Hi @jlin53882, this PR is currently showing merge conflicts with master (mergeStateStatus: DIRTY, mergeable: CONFLICTING), so it can’t be merged as-is. Could you please rebase onto the latest master (or merge master into this branch), resolve the conflicts, and push an update? Once that’s done, we can take another look. Thanks.

@jlin53882 jlin53882 force-pushed the fix/autoRecallTimeoutMs-parse branch from fbcd5c8 to 7c212f6 Compare April 5, 2026 12:20
@jlin53882
Copy link
Copy Markdown
Contributor Author

✅ Done — Conflict resolved + default updated

What was fixed

  1. Rebased onto latest master (adb621e), resolved openclaw.plugin.json conflict
  2. parsePluginConfig now includes autoRecallTimeoutMs (defaults to 5000ms)
  3. Schema default: 5000 (matches discussion outcome)
  4. UI schema synced

Final PR status

  • mergeStateStatus: CLEAN
  • mergeable: MERGEABLE
  • AliceLJY ✅ APPROVED

Changes

File Change
index.ts parsePluginConfig adds autoRecallTimeoutMs, default 5000ms
openclaw.plugin.json schema + uiSchema add the field, default 5000

Ready to merge.

Copy link
Copy Markdown
Collaborator

@rwmjhb rwmjhb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix: parse autoRecallTimeoutMs in parsePluginConfig

高价值修复——config validation 拒绝用户设置的 timeout 值并静默回退到硬编码默认值,这是真实 bug。

Must Fix

  1. Duplicate key: configSchema.propertiesautoRecallTimeoutMs 出现了两次,后者会覆盖前者。部分 JSON parser 会报错。

  2. 默认值不一致: parsePluginConfig 默认 5000ms,但 runtime consumer 用 ?? 3000。两层 fallback 会导致实际默认值取决于 parsePositiveInt 对未设值的返回行为,请确认只有一处生效。

Notes

  • Build failure 是 stale_base=true,rebase 后应该能解决。
  • 修掉重复 key + 确认默认值链后可以合并。

jlin53882 added a commit to jlin53882/memory-lancedb-pro that referenced this pull request Apr 5, 2026
Must fix per PR CortexReach#466 review by rwmjhb:
- Remove duplicate autoRecallTimeoutMs key in configSchema.properties
  (had two entries: maximum 30000 and maximum 60000; kept the broader one)
- uiHints was already correct with label field
@jlin53882 jlin53882 force-pushed the fix/autoRecallTimeoutMs-parse branch from 7c212f6 to b7ba938 Compare April 5, 2026 14:13
@jlin53882 jlin53882 force-pushed the fix/autoRecallTimeoutMs-parse branch from b7ba938 to d926298 Compare April 5, 2026 14:20
@jlin53882
Copy link
Copy Markdown
Contributor Author

✅ Review Must-Fix 已處理(d926298)

rwmjhb 提出的 Must-Fix

1. Duplicate key in configSchema.properties
→ 已確認:upstream/master 已有正確的單一 autoRecallTimeoutMs 條目(maximum: 60000, default: 5000),JSON 無 duplicate key ✅

2. 預設值不一致(parsePluginConfig ?? 3000 vs runtime ?? 5000
→ 已確認:兩者皆為 5000ms

  • parsePluginConfig: ?? 5000
  • Runtime consumer: ?? 5_000
  • Schema default: 5000

PR 最終狀態

  • mergeStateStatus: UNSTABLE ⚠️(CI unrelated 失敗,非本 PR 造成)
  • mergeable: MERGEABLE
  • 所有 Must-Fix 已解決,可以合併

@rwmjhb rwmjhb merged commit 7ea8174 into CortexReach:master Apr 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: autoRecallTimeoutMs declared in PluginConfig interface but missing from parsePluginConfig return

4 participants