fix: collapse duplicate registration log and dedupe auto-recall hook

[gorkem2020]

What this fixes

register() is re-invoked by the host once per scope init, including on a scope cache-miss mid-session, sometimes several times within a single turn. Two side effects of that were not accounted for:

1. Log spam. register() logged "plugin registered" unconditionally on every call, while _initPluginState() (guarded to run exactly once) logged a second, near-identical "plugin registered [singleton init]" line. A single turn with a few re-registrations produced three or four overlapping copies of what is really one event.
2. Handler-accumulation risk. The existing _dedupHookEvent() guard, which collapses duplicate hook invocations keyed on (handlerName, sessionKey, timestamp), already covers the agent:bootstrap, self-improvement, and reflection hooks. The auto-recall before_prompt_build handler, the one that actually does the (expensive) embedding + retrieval + rerank pipeline, was the only before_prompt_build site not covered. If a re-registration ever re-attaches handlers without the host clearing the previous ones, the recall pipeline would silently run multiple times per turn instead of once, with no guard to catch it.

How

• Track whether a given register() call is the one that actually creates the singleton (isFirstRegistration = !_singletonState, captured before the existing if (!_singletonState) { _singletonState = _initPluginState(api); } check) and gate the "plugin registered" log on that flag. Removed the now-redundant duplicate log that lived inside _initPluginState(), so there is exactly one log line, once per process, regardless of how many times register() re-runs.
• Added if (_dedupHookEvent("autoRecall", event)) return; to the auto-recall before_prompt_build handler, placed after the existing cheap validation/skip checks (subagent session, invalid agent id, include/exclude lists, short-message gating) so that legitimately skipped events do not pollute the shared dedup set, matching the placement convention already used by the other three guarded handlers.

What this intentionally does not change: hook attachment itself (api.on(...), api.registerHook(...)) still runs unconditionally on every register() call. A comment already in this code documents that the host clears internal hooks between register() calls, so skipping attachment for a "duplicate" scope risks leaving a live api instance with no handlers at all, silently breaking that scope. There is also no scope identifier available on the plugin api object to key a safe registration-level dedup on. Given that, deduping registration itself was judged unsafe without deeper knowledge of host internals, so this change only hardens the two concrete, verifiable side effects above.

Tests

New test/register-scope-dedup.test.mjs (3 tests):

• a second register() call with the same api instance stays a no-op (existing WeakSet guard) and does not re-log,
• a fresh api instance, simulating a scope cache-miss, still gets its own working before_prompt_build hook, and the "plugin registered" log stays collapsed to one across both registrations,
• the auto-recall handler runs the recall pipeline exactly once for one logical prompt-build event, even when two separately registered handler instances both receive it.

Verified (b) and (c) fail against the pre-fix code (3 and 2 calls respectively, where 1 was expected), confirming they exercise the actual bug rather than passing vacuously.

Wired into package.json's test script and scripts/ci-test-manifest.mjs (core-regression group).

Full local run: npx tsc -p tsconfig.json builds clean. All 47 commands in npm test pass individually, including the new test file. The one pre-existing failure in this environment, test/cjk-recursion-regression.test.mjs failing with EADDRINUSE: address already in use 127.0.0.1:11434, is unrelated to this change: it reproduces identically when run standalone against an unmodified checkout, because something else on the host already holds that port.

dist/ is committed in this repo, so a final commit rebuilds it via npx tsc -p tsconfig.json.

[rwmjhb]

Approved on head 90e3f29. Orchestrator verdict: approve. Note: adversarial R4 returned invalid JSON twice, so orchestrator confidence was reduced; I completed independent verification on the same head before approving.

Independent verification run:

• npm ci --silent
• npm run build --if-present
• git diff --exit-code -- dist src index.ts package.json package-lock.json openclaw.plugin.json README.md scripts test
• node --test test/register-scope-dedup.test.mjs
• node test/plugin-manifest-regression.mjs
• node test/cli-smoke.mjs
• npm test
• git diff --exit-code -- dist src index.ts package.json package-lock.json openclaw.plugin.json README.md scripts test

All passed; build left committed dist/source artifacts clean.

Non-blocking follow-up: consider deriving the auto-recall dedup key from the same normalized session identity the hook uses (ctx.sessionKey / ctx.sessionId fallback included), and add coverage for before_prompt_build events where session identity is supplied only through ctx.

[rwmjhb]

This PR now has merge conflicts after the latest changes on master (looks like after #914 landed). Please rebase or merge master and resolve the conflicts, then push an updated head so I can re-check it.

[gorkem2020]

Merged master into this branch to pick up #914 (noise-bank gating and reasoning-field JSON recovery in the smart extractor and LLM client). The only real conflict was package.json's test script, resolved as a union so both #914's new noise-gating test entry and this branch's register-scope-dedup test entry are present; scripts/ci-test-manifest.mjs auto-merged cleanly with both entries intact. Typecheck and all touched tests (register-scope-dedup, llm-api-key-client, smart-extractor-noise-gating) pass, dist was rebuilt from the merged source and matches exactly, so this is ready for re-check.

[app3apps]

I completed the review on head 75a9f91. One blocking issue remains: the auto-recall dedup guard still builds its key from event.sessionKey and event.timestamp, but the real before_prompt_build hook carries run/session identity in ctx. The regression test fabricates those event fields, so accumulated handlers can still execute embedding, retrieval, and reranking more than once.

Since #920 merged, GitHub now reports this branch as mergeable=CONFLICTING / merge_state_status=DIRTY.

Please rebase onto or merge the latest master, resolve the conflicts, update the dedup logic to use stable identity from ctx (with an appropriate fallback), and make the regression test use the real event shape. Push the updated head for re-review.

[gorkem2020]

Fixed: the dedup guard now keys off ctx.sessionKey/ctx.sessionId, matching how before_prompt_build actually carries identity, and falls back to the prompt text instead of Date.now() when no timestamp exists (Date.now() made every key unique, so dedup was a no-op in production, and it also risked collapsing two distinct prompts landing in the same millisecond). The regression test now uses the real event shape and covers the two-distinct-prompts case. Rebased onto latest master.

[app3apps]

Reviewed head d13c6f8. The previous blocking issue is fixed: auto-recall dedup now uses the hook context's session identity, and the updated regression test uses the real event shape.

The orchestrator verdict is approve, but confidence is limited to 0.50 because R2a did not produce a valid artifact. Two non-blocking but material risks remain:

• When no timestamp exists, the raw prompt becomes part of a process-wide key retained until the dedup set is pruned. A legitimate later turn with the same prompt in the same session can therefore skip memory recall.
• If no session identity is available, identical prompts can collide across logical sessions.

Please consider an invocation-specific identity, event-object identity, or a short-lived dedup window instead of persistent raw-prompt keying. I am recording this as a review comment rather than adding a new formal approval because the primary code-review round was incomplete.

[app3apps]

Re-reviewed the rebased head 157c3c3. The targeted registration/dedup tests and full suite pass, and the duplicate-handler case is correctly collapsed to one auto-recall execution.

Non-blocking follow-up: when no event timestamp exists, the fallback key uses a process-wide, long-lived session + full prompt value and ignores ctx.runId. Two legitimate turns or retries with the same prompt can therefore skip the later recall; missing session identity can also cause cross-session collisions. Prefer event/run identity (or a short-lived/WeakSet fallback) and add coverage for identical prompts on distinct run IDs. A fixed-size digest would also avoid retaining complete prompts in the Set.