fix(memory-core): suppress expected dreaming cleanup scope warnings by christnorman · Pull Request #68681 · openclaw/openclaw

christnorman · 2026-04-18T20:08:44Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: dreaming narrative cleanup can log missing scope: operator.admin during synthetic plugin-runtime cleanup.
Why it matters: this is expected best-effort cleanup behavior, but it shows up as warning noise and makes dreaming failures look more serious than they are.
What changed: suppress the expected cleanup warning for missing scope: operator.admin and add regression coverage in dreaming-narrative.test.ts.
What did NOT change (scope boundary): this does not change gateway auth, scope minting, or cleanup behavior for other real cleanup failures.:

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #
This PR fixes a bug or regression

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write N/A. If the cause is unclear, write Unknown.

Root cause:dreaming narrative cleanup always attempted deleteSession() and logged any resulting error, even when synthetic plugin runtime scope is intentionally not allowed to perform admin-scoped session deletion.
Missing detection / guardrail:memory-core did not recognize the expected missing scope: operator.admin cleanup path as non-actionable noise.
Contributing context (if known): the dreaming scrubber already runs afterward as the best-effort cleanup fallback, so the warning was noisy rather than useful.O

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file:
Scenario the test should lock in:
Why this is the smallest reliable guardrail:
Existing test that already covers this (if any):
If no new test is added, why not:

User-visible / Behavior Changes

List user-visible changes (including defaults/config).
If none, write None.

Diagram (if applicable)

For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write N/A.

Before:
[user action] -> [old state]

After:
[user action] -> [new state] -> [result]

Security Impact (required)

New permissions/capabilities? (Yes/No)
Secrets/tokens handling changed? (Yes/No)
New/changed network calls? (Yes/No)
Command/tool execution surface changed? (Yes/No)
Data access scope changed? (Yes/No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS:
Runtime/container:
Model/provider:
Integration/channel (if any):
Relevant config (redacted):

Steps

Expected

Actual

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
Edge cases checked:
What you did not verify:

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes/No)
Config/env changes? (Yes/No)
Migration needed? (Yes/No)
If yes, exact upgrade steps:

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk:
- Mitigation:

greptile-apps · 2026-04-18T20:10:49Z

Greptile Summary

Suppresses the expected "missing scope: operator.admin" warning that fires during dreaming narrative cleanup when the synthetic plugin-runtime scope lacks admin permissions. A module-level regex (MISSING_ADMIN_SCOPE_RE) and a small predicate function gate the logger.warn call inside the deleteSession catch block, leaving all other cleanup errors visible. A regression test is added that verifies the warning is suppressed without skipping the deleteSession call or dropping the narrative write.

Confidence Score: 5/5

Safe to merge — the only finding is a minor style concern about string-based error matching.

The change is small and correctly targeted. Existing and new tests cover both the suppressed and non-suppressed paths. No behavior changes outside log noise reduction. The single P2 comment about regex fragility vs. typed error codes is non-blocking.

No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/memory-core/src/dreaming-narrative.ts
Line: 116-118

Comment:
**String-pattern matching for error suppression is fragile**

`isExpectedNarrativeCleanupScopeError` matches against the human-readable error message rather than a typed error code or class. If the upstream session layer ever changes its error message format (e.g. `"scope missing: operator.admin"` or `"insufficient scope: operator.admin"`), the suppression silently stops working and warning noise returns — or the pattern might match unintended errors.

Consider checking `extractErrorCode(err)` for a stable scope-error code first (the same pattern used in `isRequestScopedSubagentRuntimeError`), and falling back to the regex only if no typed code is available. A brief comment noting the fragility and why no code exists would also help future readers.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(memory-core): suppress expected drea..." | Re-trigger Greptile}

prtags · 2026-04-23T11:12:27Z

Related work from PRtags group ultimate-chow-4ei0

Title: Open PR duplicate: [Bug]: memory-core: narrative session cleanup fails with "missing scope: operator.admin"

Number	Title
#68020	fix(memory-core): silence expected operator.admin scope miss in dreaming cleanup
#68087	fix(memory-core): downgrade cleanup warning to debug when missing operator.admin scope
#68130	fix: reduce log level for narrative session cleanup scope failures
#68312	fix(memory-core): downgrade narrative cleanup WARN to debug for missing-scope errors
#68681*	fix(memory-core): suppress expected dreaming cleanup scope warnings

* This PR

clawsweeper · 2026-04-26T05:32:35Z

Closing this as duplicate or superseded after Codex automated review.

#68681 is duplicate/superseded by the open maintainer-owned canonical PR #68020 for the same memory-core dreaming cleanup missing scope: operator.admin warning. Current main does not implement the suppression yet, so the remaining fix should stay consolidated under #68020 rather than this contributor PR.

Best possible solution:

Close #68681 as duplicate/superseded and keep the remaining product fix under maintainer PR #68020. When finishing the canonical fix, carry over the useful parts from #68681: preserve gateway error codes, suppress only the expected missing-admin cleanup warning, keep other cleanup failures visible, and retain the least-privilege synthetic runtime behavior.

What I checked:

Current main still logs every narrative cleanup failure: generateAndAppendDreamNarrative still calls params.subagent.deleteSession({ sessionKey }) in finally and warns for any cleanup error, including the reported missing scope: operator.admin path. (extensions/memory-core/src/dreaming-narrative.ts:923, d54d2d6b9b8a)
Synthetic plugin subagent fallback remains least-privilege: The gateway plugin runtime fallback client defaults to operator.write, while deleteSession dispatches sessions.delete; dispatchGatewayMethod still throws from the gateway error message without the PR's code-preservation change. (src/gateway/server-plugins.ts:219, d54d2d6b9b8a)
sessions.delete remains admin-scoped: The method-scope table lists sessions.delete under operator.admin, and failed scope checks surface as missing scope: <scope>. (src/gateway/method-scopes.ts:159, d54d2d6b9b8a)
Current tests preserve the security invariant: rejects fallback session deletion without minting admin scope asserts fallback deletion rejects with missing scope: operator.admin and that dispatched scopes are exactly operator.write, not operator.admin. (src/gateway/server-plugins.test.ts:822, d54d2d6b9b8a)
Canonical duplicate is maintainer-owned and open: The provided GitHub context shows fix(memory-core): silence expected operator.admin scope miss in dreaming cleanup #68020 is open, authored by a MEMBER, labeled maintainer, and describes the same root cause and intended fix: suppress only the expected memory-core dreaming cleanup missing scope: operator.admin warning while preserving admin-scope enforcement. (d7c14b5c7bb7)
PRtags already grouped this as duplicate work: The PR discussion links fix(memory-core): suppress expected dreaming cleanup scope warnings #68681 into duplicate group ultimate-chow-4ei0 with fix(memory-core): silence expected operator.admin scope miss in dreaming cleanup #68020, fix(memory-core): downgrade cleanup warning to debug when missing operator.admin scope #68087, fix: reduce log level for narrative session cleanup scope failures #68130, and fix(memory-core): downgrade narrative cleanup WARN to debug for missing-scope errors #68312 for the same memory-core cleanup warning.

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against d54d2d6b9b8a.

openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: XS labels Apr 18, 2026

greptile-apps Bot reviewed Apr 18, 2026

View reviewed changes

Comment thread extensions/memory-core/src/dreaming-narrative.ts

fix(memory-core): suppress expected dreaming cleanup scope warnings

173ce22

christnorman force-pushed the codex/memory-dreaming-cleanup branch from 14657ae to 173ce22 Compare April 18, 2026 20:15

openclaw-barnacle Bot added gateway Gateway runtime size: S and removed size: XS labels Apr 18, 2026

prtags Bot mentioned this pull request Apr 23, 2026

fix(memory-core): downgrade cleanup warning to debug when missing operator.admin scope #68087

Closed

3 tasks

clawsweeper Bot closed this Apr 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(memory-core): suppress expected dreaming cleanup scope warnings#68681

fix(memory-core): suppress expected dreaming cleanup scope warnings#68681
christnorman wants to merge 1 commit into
openclaw:mainfrom
christnorman:codex/memory-dreaming-cleanup

christnorman commented Apr 18, 2026

Uh oh!

greptile-apps Bot commented Apr 18, 2026

Uh oh!

Uh oh!

prtags Bot commented Apr 23, 2026

Uh oh!

clawsweeper Bot commented Apr 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

christnorman commented Apr 18, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

prtags Bot commented Apr 23, 2026

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Apr 26, 2026 •

edited

Loading