fix: add isInvalidAgentIdFormat guard to runMemoryReflection (Issue #686) by jlin53882 · Pull Request #687 · CortexReach/memory-lancedb-pro

jlin53882 · 2026-04-22T08:49:49Z

Closing in favor of PR #722

This PR (#687) became a mix of multiple PRs (#516 scope creep, serialCooldownMs, excludeAgents, reflection-store.ts mass reformat, CI manifest changes, etc.) — as noted in the maintainer review.

A clean replacement has been opened at PR #722: #722

PR #722 contains only the minimal Issue #686 fix:

isInvalidAgentIdFormat() guard on runMemoryReflection command hook (1 guard site, matching the issue scope)
Production export for testability (14 tests, 0 skipped)
No schema changes, no CI manifest changes, no unrelated feature additions

…ebase) This rebases the following fixes from PR CortexReach#516 onto upstream/master (0988a46): F2 (excludeAgents runtime reading): - Add isAgentOrSessionExcluded() helper supporting exact/wildcard/temp:* patterns - Add memoryReflection.excludeAgents to PluginConfig and openclaw.plugin.json schema - Add excludeAgents check in runMemoryReflection command hook F3 (wildcard pattern fix): - Replace config.autoRecallExcludeAgents.includes(agentId) with isAgentOrSessionExcluded() in before_prompt_build hook - Supports pi-, temp:*, and exact match patterns F5 (serialCooldownMs configurable): - Add serialCooldownMs?: number to PluginConfig.memoryReflection - Serial guard now reads cooldown from cfg.memoryReflection.serialCooldownMs - Default: 120000ms (2 min), set to 0 to disable Schema additions (openclaw.plugin.json): - memoryReflection.serialCooldownMs (integer, min: 0) - memoryReflection.excludeAgents (string array) - autoRecallExcludeAgents (string array, top-level) EF1 (backtick fix already present in upstream 0988a46)

…eview fixes)

…eclaredAgents validation - Add isChatIdBasedAgentId() helper: pure-digit IDs (e.g. "657229412030480397") are almost always chat_id extractions and cause 60s auto-recall timeout - Add isInvalidAgentIdFormat() with three-layer guard: empty check → numeric check → declaredAgents Set lookup (authoritative, from openclaw.json) - Add declaredAgents Set (IIFE) populated from cfg.agents.list in config return - Add guard to all 6 hook sites: auto-recall entry, recallWork inner, auto-capture (agent_end), reflection inheritance, reflection derived+error, before_reset

新增 test/agentid-validation.test.mjs，覆蓋 Issue CortexReach#492 的修復邏輯：測試內容： 1. Layer 1（空值檢查） - undefined / null / "" → invalid 2. Layer 2（純數字 = chat_id） - "657229412030480397" → invalid（這就是導致 60s timeout 的元兇） - "dc-channel--1476858065914695741" → NOT invalid（有字母前綴，正確） - "tg-group--5108601505" → NOT invalid 3. Layer 3（declaredAgents Set） - "main" 在清單中 → valid - 不在清單中的隨機字串 → invalid - declaredAgents 為空時 → 不主動阻擋 4. Regex 迴歸測試 - 13 個邊界案例全部驗證通過同時更新 ci-test-manifest.mjs，將新測試加入 core-regression 測試群組。根因對照： Issue CortexReach#492 的根本原因是 numeric chat_id（如 657229412030480397）被當成 agentId 傳入 LanceDB，導致 retriever.test() timeout。本測試確保： - 純數字 ID（Layer 2）被正確攔截 - 有效的 agent ID（dc-channel-- / tg-group--）不受影響 - declaredAgents Set 白名單邏輯正確

…2/F5 review fix)

…low CI)

…ortexReach#686)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1b3bd46f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-22T08:54:09Z

        })(),
        errorReminderMaxEntries: parsePositiveInt(memoryReflectionRaw.errorReminderMaxEntries) ?? DEFAULT_REFLECTION_ERROR_REMINDER_MAX_ENTRIES,
        dedupeErrorSignals: memoryReflectionRaw.dedupeErrorSignals !== false,
+        serialCooldownMs: parsePositiveInt(memoryReflectionRaw.serialCooldownMs) ?? DEFAULT_SERIAL_GUARD_COOLDOWN_MS,


Allow zero serial cooldown to disable reflection throttle

memoryReflection.serialCooldownMs is documented as “set to 0 to disable,” but this parser uses parsePositiveInt, which rejects 0 and falls back to the default 120000 ms. In configs that explicitly set serialCooldownMs: 0, reflection still remains throttled for two minutes, so command-hook reflections are unexpectedly skipped instead of running every trigger.

Useful? React with 👍 / 👎.

jlin53882 · 2026-04-24T16:55:44Z

CI 失敗分析（3 個 Job 均無關聯 PR #687）

失敗摘要

Job	錯誤檔案	根因	是否關聯 PR #687
core-regression	`test/smart-extractor-branches.mjs:497`	測試斷言失敗	❌ 檔案無變動
storage-and-schema	`smart-extractor.ts:444` - `this.store.bulkStore is not a function`	mock 缺少 `bulkStore`	❌ 測試 mock 既有問題
packaging-and-workflow	`scripts/verify-ci-test-manifest.mjs:60`	manifest 未列入 `test/import-markdown/`	❌ CI 設定既有問題

關鍵觀察

三個 Job 的 Test step 都失敗在同一個 merge commit (011d344841633aa) 上，這是 PR #687 的 parent commit（merge base），並非 PR 自己引入的變更。PR #687 只修改了 index.ts，與上述三個測試檔案完全無交集。

建議

建議開獨立 issue 追蹤這三個 upstream 既有問題，或在 upstream/master 上驗證這些失敗是否已存在後再合併 PR #687。

Conflict resolution for PR CortexReach#687: - scripts/ci-test-manifest.mjs: merge bulkStore (Issue CortexReach#665, PR-re-added) + agentId validation (Issue CortexReach#492, PR CortexReach#687) - scripts/verify-ci-test-manifest.mjs: merge both — bulkStore from master + agentId from PR branch CI manifest now includes all baseline tests consistently across both files.

Guard against parsePositiveInt rejecting 0 (P2 review comment). Before: serialCooldownMs: 0 silently fell back to 120000ms instead of disabling the throttle, contradicting documented behavior. After: intercept 0 explicitly (covers both number 0 and string "0") before calling parsePositiveInt. Guard condition is always false when cooldownMs=0, so throttle is correctly bypassed.

jlin53882 · 2026-04-28T09:11:33Z

P2 Fix: Allow zero serial cooldown to disable reflection throttle

Fixed in commit ed6eafd.

Root cause: parsePositiveInt() rejects 0 (strictly positive only), so serialCooldownMs: 0 was silently treated as "unset" and fell back to 120000ms, contradicting the documented "set to 0 to disable" behavior.

Fix: Intercept 0 explicitly before calling parsePositiveInt. Uses Number(raw) === 0 to cover both number 0 and string "0" (e.g. from env var expansion). When cooldownMs = 0, the guard condition (Date.now() - lastRun) < cooldownMs is always false, so throttle is correctly bypassed.

serialCooldownMs: (() => {
  const raw = memoryReflectionRaw.serialCooldownMs;
  if (raw == null) return DEFAULT_SERIAL_GUARD_COOLDOWN_MS;
  if (Number(raw) === 0) return 0;  // 0 = disabled
  return parsePositiveInt(raw) ?? DEFAULT_SERIAL_GUARD_COOLDOWN_MS;
})(),

This keeps parsePositiveInt unchanged for all other callers.

rwmjhb

Thanks for adding the missing runMemoryReflection guard. The narrow fix is useful, but this PR is not ready in its current form.

Blocking issues:

The PR appears to re-apply most or all of #516 plus unrelated serialCooldownMs and CI manifest changes. Please rebase/split so this PR contains only the #686 runMemoryReflection guard delta on top of its intended parent.
The unit tests for isInvalidAgentIdFormat are silently skipped because the helper is not exported from index.ts, so the advertised guard tests are not exercising the production implementation.
Typecheck/build was skipped in the review pipeline, and the diff appears to add/read declaredAgents without the corresponding PluginConfig interface support.
declaredAgents appears to be built from plugin config rather than the OpenClaw root config, which can make the new validation layer ineffective in normal installs.
openclaw.plugin.json now has a duplicate autoRecallExcludeAgents schema entry, which can override/drop the earlier default.

Please split/rebase first, then add a test that reaches the actual production hook path instead of a skipped/helper-only path.

jlin53882 · 2026-04-29T16:42:29Z

Closing in favor of PR #722

This PR (#687) became a mix of multiple PRs (#516 scope creep, serialCooldownMs, excludeAgents, reflection-store.ts mass reformat, CI manifest changes, etc.) — as noted in the maintainer review.

A clean replacement has been opened at PR #722: #722

PR #722 contains only the minimal Issue #686 fix:

isInvalidAgentIdFormat() guard on runMemoryReflection command hook (1 guard site, matching the issue scope)
Production export for testability (14 tests, 0 skipped)
No schema changes, no CI manifest changes, no unrelated feature additions

This PR should be closed.

…ortexReach#686) Minimal fix addressing Issue CortexReach#686 without the scope creep of PR CortexReach#687. Changes: - Add isInvalidAgentIdFormat() guard (Layer 1: empty/undefined, Layer 2: pure numeric = chat_id) to runMemoryReflection command hook, consistent with other hook sites. - Export isInvalidAgentIdFormat for testability. - Add integration test (agentid-validation-686.test.mjs) that directly imports the production export — NOT skipped/helper-only. Layer 3 (declaredAgents Set validation) is intentionally omitted from this fix. The plugin config does not carry agents.list from openclaw.json root config; implementing Layer 3 requires proper api.config.agents access from the plugin API, which is not reliably available in all install configurations. A follow-up PR should address Layer 3 once the correct API integration path is confirmed. No schema changes, no CI manifest changes, no serialCooldownMs, no excludeAgents — focused scope on the CortexReach#686 guard only.

jlin53882 and others added 7 commits April 22, 2026 13:32

fix: wildcard prefix match and agentId undefined guard (adversarial r…

4bd3ae8

…eview fixes)

fix: wire parsePluginConfig for serialCooldownMs and excludeAgents (F…

42641bd

…2/F5 review fix)

fix: add agentid-validation to manifest baseline (packaging-and-workf…

69fb9f8

…low CI)

fix: add isInvalidAgentIdFormat guard to runMemoryReflection (Issue C…

a1b3bd4

…ortexReach#686)

This was referenced Apr 22, 2026

runMemoryReflection missing isInvalidAgentIdFormat guard (inconsistent with other hook sites) #686

Open

fix: complete Issue #492 protection -- per-agent exclusion + internal session guards #516

Merged

chatgpt-codex-connector Bot reviewed Apr 22, 2026

View reviewed changes

jlin53882 force-pushed the fix/issue-686-missing-guard branch from 883f38a to 1fc99bb Compare April 28, 2026 06:40

rwmjhb requested changes Apr 29, 2026

View reviewed changes

jlin53882 mentioned this pull request Apr 29, 2026

fix: add isInvalidAgentIdFormat guard to runMemoryReflection (Issue #686) #722

Open

jlin53882 closed this Apr 29, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add isInvalidAgentIdFormat guard to runMemoryReflection (Issue #686)#687

fix: add isInvalidAgentIdFormat guard to runMemoryReflection (Issue #686)#687
jlin53882 wants to merge 9 commits intoCortexReach:masterfrom
jlin53882:fix/issue-686-missing-guard

jlin53882 commented Apr 22, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 22, 2026

Uh oh!

jlin53882 commented Apr 24, 2026

Uh oh!

jlin53882 commented Apr 28, 2026

Uh oh!

rwmjhb left a comment

Uh oh!

jlin53882 commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jlin53882 commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Closing in favor of PR #722

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

jlin53882 commented Apr 24, 2026

CI 失敗分析（3 個 Job 均無關聯 PR #687）

失敗摘要

關鍵觀察

建議

Uh oh!

jlin53882 commented Apr 28, 2026

P2 Fix: Allow zero serial cooldown to disable reflection throttle

Uh oh!

rwmjhb left a comment

Choose a reason for hiding this comment

Uh oh!

jlin53882 commented Apr 29, 2026

Closing in favor of PR #722

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jlin53882 commented Apr 22, 2026 •

edited

Loading