Skip to content

feat(memory-reflection)+feat(self-improvement): focus self-improvement flow and support dedicated reflection agent#43

Merged
furedericca-lab merged 17 commits intowin4r:mainfrom
furedericca-lab:feat/self-improvement-reflection-agent
Mar 6, 2026
Merged

feat(memory-reflection)+feat(self-improvement): focus self-improvement flow and support dedicated reflection agent#43
furedericca-lab merged 17 commits intowin4r:mainfrom
furedericca-lab:feat/self-improvement-reflection-agent

Conversation

@furedericca-lab
Copy link
Collaborator

@furedericca-lab furedericca-lab commented Mar 4, 2026

Background

In long-running agent setups, users need more than one-off replies. They need a durable behavior system that can:

  • turn completed sessions into reusable reflection artifacts,
  • automatically inherit stable rules in the next run,
  • surface derived execution deltas,
  • continuously govern corrections, failures, and capability gaps into reusable skills.

This PR aligns two feature tracks to support that system:

  • Self-Improvement
  • memoryReflection

Why This Feature Is Needed

Self-Improvement handles governance and accumulation (log, review, skill extraction).

memoryReflection handles session reflection memoryization and exports inheritance/derived deltas for subsequent runs.

Together, they upgrade the agent from single-turn behavior into a cumulative, reusable, self-correcting long-term behavior system.

What This PR Changes

1. Self-Improvement governance workflow

  • Adds and aligns governance tools:
    • self_improvement_log
    • self_improvement_review
    • self_improvement_extract_skill
  • Introduces shared learning-file bootstrap module:
    • src/self-improvement-files.ts
  • Unifies initialization path for:
    • .learnings/LEARNINGS.md
    • .learnings/ERRORS.md
    • .learnings/FEATURE_REQUESTS.md

2. memoryReflection closed-loop workflow

  • On /new and /reset, generates structured reflection from recent session JSONL.
  • Persists reflection markdown artifacts.
  • Optionally stores reflection slices in LanceDB for retrieval-backed reuse.
  • Injects on subsequent runs:
    • <inherited-rules> (stable constraints)
    • <derived-focus> / <error-detected> (execution deltas and error reminders)

3. Dedicated reflection runner agent

  • Adds memoryReflection.agentId.
  • Allows reflection generation on a dedicated agent profile (for example memory-distiller).

4. Runtime hardening and schema/docs updates

  • Replaces fixed embedded-runner path assumptions with resilient runtime fallback:
    • embedded runner -> openclaw agent --local --json -> minimal fallback text
  • Hardens workspace resolution:
    • prefer runtime workspaceDir
    • fallback to ~/.openclaw/workspace (no /root hardcode)
  • Validates memoryReflection.agentId against configured agents:
    • warn clearly when missing
    • fallback to runtime agent id
  • Improves reflection artifact robustness:
    • collision-safe high-resolution filenames
    • safer session source fallback selection via mtime
  • Updates openclaw.plugin.json, README.md, and README_CN.md to match current behavior.
  • Includes small cleanup in scripts/jsonl_distill.py (stale comment removal).

5. Post-review compatibility and reflection correctness fixes

  • Switches default sessionStrategy to systemSessionMemory (compatibility-first behavior for existing users).
  • Keeps legacy sessionMemory.enabled mapping semantics unchanged (true -> systemSessionMemory, false -> none).
  • Fixes reflection prompt clipping to keep recent context (conversation.slice(-maxInputChars)) instead of oldest-first clipping.

Trigger Conditions

Reflection generation trigger

  • sessionStrategy must be memoryReflection (explicitly configured; not default).
  • Trigger event is command:new / command:reset.
  • Reflection generation is skipped when session context is incomplete (for example missing cfg, missing session file, or no readable conversation content).

Reflection runner chain

  • First try embedded runner (runEmbeddedPiAgent).
  • If embedded path fails, automatically fallback to openclaw agent --local --json.
  • Only if both fail, generate minimal fallback reflection text.

LanceDB write trigger

  • Controlled by memoryReflection.storeToLanceDB (effective only when sessionStrategy=memoryReflection).
  • Only non-fallback reflections are eligible for LanceDB persistence.
  • Additional similarity dedupe runs before write (> 0.97 hit skips storing).

Compatibility Notes

  • memoryReflection.agentId behavior is additive and optional.
  • Strategy key remains sessionStrategy.
  • Existing self-improvement workflow remains compatible, now with unified learning-file initialization.
  • Existing reflection workflow remains backward compatible, with safer degradation behavior when embedded runtime layout changes.

Findings (By Severity)

Medium (Addressed in this PR updates)

  • Default strategy compatibility(Resolved): changed default sessionStrategy from memoryReflection to systemSessionMemory to avoid implicit behavior shift for existing users.
  • Reflection correctness(Resolved): changed prompt clipping from slice(0, maxInputChars) to slice(-maxInputChars) so recent decisions are preserved.

Low (Addressed)

  • Workspace fallback path hardcoded as /root/.openclaw/workspace in multiple places (Resolved): now prefers runtime context.workspaceDir / toolCtx.workspaceDir; fallback is homedir() + "/.openclaw/workspace" (no /root hardcode).
  • memoryReflection.agentId validation and fallback (Resolved): when configured id is absent in cfg.agents.list, plugin logs logger.warn and forces fallback to current runtime agentId (no silent degrade).
  • embedded-runner import-path sensitivity (Resolved)

Residual Risk

  • openclaw agent --local --json output prelude may vary across CLI versions; parser tolerates leading non-JSON lines, but still depends on recoverable JSON payloads.
  • memoryReflection remains an opt-in path, but when enabled it adds pre-reset work (/new and /reset) and can increase latency under CLI fallback/timeout scenarios.
  • Reflection core is still concentrated in index.ts with limited focused unit tests for key helpers (for example secret redaction and reflection-slice extraction).

Test Checklist

  • npm test (CLI smoke) passed.
  • Plugin runtime command path verified:
    • openclaw --help
    • openclaw agent --help
  • openclaw agent --local --json execution verified and JSON payload shape inspected in this environment.
  • Reflection CLI-fallback JSON parsing path validated against real command output containing non-JSON prelude logs.
  • Workspace hardcode scan verified no remaining /root/.openclaw/workspace literals in plugin source scope.

Included Commits

  • 9cab679 feat: integrate memory-reflection/self-improvement and harden session safety
  • e3bd720 feat: focused self-improvement and memory-reflection pipeline
  • 1206e14 fix(memory-reflection): prevent stale derived-focus replay on fallback
  • 94389e9 feat(memory-reflection): harden runner fallback and reflection flow
  • 7fdb06e Merge remote-tracking branch 'upstream/main'
  • 3f6769e fix(session-strategy): keep default compatible and trim reflection input tail
  • 6a6fb51 refactor(types): narrow reflection runner boundary from any to unknown
  • 5c2ee3c feat(reflection): unify reflection category and display tag format
  • 828f7f3 docs: document mdMirror dual-write behavior and reflection tag format
  • 10bad30 Merge remote-tracking branch 'upstream/main'

@furedericca-lab furedericca-lab marked this pull request as draft March 4, 2026 08:40
@win4r
Copy link
Owner

win4r commented Mar 4, 2026

Local + CI verification looks good on my side (cli-smoke / gateway load / export+import dry-run / delete-bulk dry-run / migrate check / reembed dry-run).

Before moving this PR from Draft -> Ready, I suggest addressing two items (also mentioned in the PR description):

  1. Workspace fallback path is hardcoded as /root/.openclaw/workspace in multiple places. In non-default workspace setups this may write .learnings / memory/reflections artifacts to the wrong directory.

    • Suggestion: always prefer context.workspaceDir / toolCtx.workspaceDir from hook/tool ctx; if missing, fallback to homedir() + '/.openclaw/workspace' (or warn + skip writing) to avoid writing under /root.
  2. memoryReflection.agentId should be validated. If the agent id is not found in cfg.agents.list, please logger.warn clearly and force fallback to the current runtime agentId (avoid silent degrade).

(Optional) The embedded runner import path may be sensitive to OpenClaw install layout changes; a clearer warn + fallback strategy would help if layout changes in the future.

Once these are addressed and the PR is marked Ready, I can give LGTM and proceed with merge + release.

- squash prior commits 51b6bb6 + 2293550 with current changes

- add embedded -> openclaw agent --local --json -> fallback chain

- validate memoryReflection.agentId with warn + runtime fallback

- remove /root workspace hardcode; prefer runtime workspace and home fallback

- harden reflection file naming/write collision handling and session file mtime fallback

- update README/README_CN behavior docs for trigger/store conditions
@furedericca-lab furedericca-lab force-pushed the feat/self-improvement-reflection-agent branch from 2293550 to 94389e9 Compare March 4, 2026 16:26
@furedericca-lab furedericca-lab marked this pull request as ready for review March 4, 2026 16:27
@furedericca-lab furedericca-lab marked this pull request as draft March 4, 2026 16:31
@furedericca-lab furedericca-lab marked this pull request as ready for review March 4, 2026 16:46
@furedericca-lab
Copy link
Collaborator Author

furedericca-lab commented Mar 4, 2026

  • index.ts

    • Preserves and merges the provider/model pass-through behavior from 51b6bb6
    • Updates reflection execution flow to: embedded -> openclaw agent --local --json -> fallback
    • Adds memoryReflection.agentId config validation with warning + runtime fallback
    • Removes hardcoded /root workspace fallback
    • Adds collision-safe reflection filenames and mtime-based session fallback source selection
  • src/tools.ts

    • Changes workspace fallback to homedir()/.openclaw/workspace
  • README.md / README_CN.md

    • Syncs reflection trigger conditions, LanceDB persistence conditions, and fallback behavior

The "Invariants & Reflections" naming adjustment is included in the final result (retained as net effect).

@rwmjhb
Copy link
Collaborator

rwmjhb commented Mar 5, 2026

Code Review — Cross-Model Analysis

PR scope: +2,118 / −197 lines across 7 files


Recommendation: Split Into Two PRs

Self-Improvement and Memory Reflection share no state, data flow, or config namespace. Recommend splitting into:

PR Scope Verdict
#43a — Self-Improvement Tools (self_improvement_log/review/extract_skill) + bootstrap/reset hooks + .learnings/ management ✅ Can merge as-is
#43b — Memory Reflection Reflection generation + inheritance/derived injection + error signal closed-loop 🔶 Issues below to discuss first

#43a — Self-Improvement: Looks Good

The tool implementations are clean, sanitizeFileToken whitelist is solid, file write queue serialization works, and the bootstrap injection is well-guarded. No blocking issues found.


#43b — Memory Reflection: Issues to Discuss

1. Default Behavior Regression (Breaking Change)

Upgrading silently switches all existing users from no session hooks to sessionStrategy: "memoryReflection", which spawns subprocesses, writes to LanceDB, writes reflection files to disk. If the environment lacks the openclaw CLI or write permissions, every /new and /reset errors out.

Suggestion: Default to "none", require explicit opt-in. Emit a deprecation warning when legacy sessionMemory.enabled is detected.

2. conversation.slice(0, maxInputChars) — Wrong Truncation Direction

buildReflectionPrompt keeps the oldest conversation and discards the newest. Reflections end up summarizing early greetings instead of the actual conclusions and decisions.

Fix: slice(-maxInputChars) to keep the tail.

3. index.ts God File (~2,600 lines) — Untestable Core Logic

PR expands index.ts to ~2,600 lines with 6 concerns, 25+ functions, 8 type definitions. Critical pure functions (redactSecrets with 15 regexes, extractReflectionSlices, buildReflectionPrompt, normalizeErrorSignature) are embedded in the register() closure and cannot be imported or unit tested.

+2,118 lines with 0 test files. Any regex gap in redactSecrets could leak API keys into reflection output with no test coverage to catch it.

Suggestion: Extract to src/reflection/ modules. At minimum, unit test redactSecrets and extractReflectionSlices. Can be a follow-up PR.

4. Multiple any Types in Reflection Core

type EmbeddedPiRunner = (params: Record<string, unknown>) => Promise<any>;
const result: any = await runEmbeddedPiAgent({ ... });
const appendSelfImprovementNote = async (event: any) => { ... };
const runMemoryReflection = async (event: any) => { ... };

The reflection pipeline's core result type is any — TypeScript can't catch API changes. Prefer unknown + type narrowing.


Code Highlights

Both models recognized solid engineering in the reflection design:

  • Reflection degradation chain (embedded → CLI → fallback) — robust fault tolerance
  • Error signal closed-loop (capture → dedupe → inject) — complete learning pipeline
  • redactSecrets() at JSONL parse time — API keys redacted before reaching reflection LLM
  • isInternalReflectionSessionKey — prevents recursive reflection loops
  • shouldSkipReflectionMessage — filters <relevant-memories> circular pollution
  • sanitizeFileToken — whitelist [a-z0-9_-] + 32-char truncation
  • spawn() over exec() + flag: "wx" exclusive writes — good practices throughout

Summary

The feature design quality is high — the degradation chain, error closed-loop, and guard mechanisms are all well thought out. Splitting into two PRs would let Self-Improvement land immediately while Memory Reflection gets the default-behavior and truncation fixes it needs.

@furedericca-lab
Copy link
Collaborator Author

@rwmjhb Thanks for the cross-model review. I re-checked all discussed points and applied the requested fixes in this PR branch.

Addressed in this PR

  1. Default strategy compatibility (Resolved)
  • Changed default sessionStrategy from memoryReflection to systemSessionMemory.
  • Kept legacy mapping behavior:
    • sessionMemory.enabled=true -> systemSessionMemory
    • sessionMemory.enabled=false -> none
  1. Reflection prompt truncation correctness (Resolved)
  • Changed from slice(0, maxInputChars) to slice(-maxInputChars) so recent decisions are preserved.
  1. Workspace fallback hardcode (Resolved)
  • Removed /root/.openclaw/workspace hardcoded fallback.
  • Now always prefers runtime context.workspaceDir / toolCtx.workspaceDir.
  • Fallback is homedir() + "/.openclaw/workspace".
  1. memoryReflection.agentId validation (Resolved)
  • If configured memoryReflection.agentId is not found in cfg.agents.list,
    plugin now logs a clear logger.warn and forces fallback to current runtime agentId.
  1. Embedded runner path robustness (Resolved)
  • Improved embedded runner import-path probing and kept resilient chain:
    embedded -> openclaw agent --local --json -> fallback.
  1. Reflection boundary typing hardening (Resolved in latest update)
  • Tightened reflection runner boundary types:
    • EmbeddedPiRunner: Promise<any> -> Promise<unknown>
    • const result: any -> unknown with local narrowing before payload access
  • Kept event: any for hook boundary for now (dynamic runtime event shape).

Clarifications on review suggestions

On splitting into two PRs: I prefer to keep this as one PR at this point.
The self-improvement and reflection paths share the same runtime integration surface (hook lifecycle, workspace resolution, session strategy/schema/docs sync). Splitting now would introduce duplicated migration work and a higher rebase/conflict risk, and can create temporary inconsistent states if only one side lands first.

On the large index.ts concern: agreed this is a valid structural concern.
For this PR, I prioritized behavioral correctness and compatibility fixes first (default strategy, prompt clipping direction, workspace fallback, agentId validation/fallback), and kept blast radius minimal. I plan to do a follow-up refactor PR to extract reflection helpers/modules and add focused unit tests.

On any usage in reflection boundaries: current any was mainly at external runtime/SDK boundaries with runtime guards.
I applied a small non-behavioral tightening in this PR (unknown + narrowing) without expanding functional scope.

PR management updates

  • Merged latest upstream/main into this branch to avoid merge conflicts.
  • Updated PR description (Findings (By Severity)) with explicit (Resolved) markers and current residual risks.

Included recent commits

  • 7fdb06e merge upstream/main sync
  • 3f6769e default strategy + truncation fix
  • 6a6fb51 reflection boundary typing hardening (any -> unknown)

@AliceLJY
Copy link
Collaborator

AliceLJY commented Mar 6, 2026

PR43 Comment Draft

Thanks for the detailed follow-up and for addressing the items from @rwmjhb's review. I re-tested specifically against the updated memoryReflection half, and the fixes you listed here do look aligned with the code on this branch.

What still seems blocked on my side is not the revised hook registration itself, but the runtime behavior of Gateway 2026.3.2.

In my QA environment:

  • static code inspection suggests Gateway 2026.3.2 should have a command:new / command:reset hook dispatch path
  • your plugin-side command:new / command:reset registration looks correct after this revision
  • but at runtime, both Discord /new and a direct sessions.reset(reason:new) call only perform session reset / transcript archive, without any observable command:new hook side effects

What I expected to see, but did not see:

  • bundled session-memory writing workspace/memory/*.md
  • .learnings/*
  • memory/reflections/*

So at this point I’m leaning toward this being an upstream Gateway 2026.3.2 runtime issue / regression, rather than a problem in this updated PR branch itself.

Could you help confirm:

  1. whether you can reproduce the same behavior on your side,
  2. whether you have already validated this on a newer Gateway build,
  3. and if this is confirmed upstream, whether we should treat it as a separate Gateway follow-up rather than blocking this PR itself.

From my side, the remaining concern is mainly this upstream runtime blocker around the memoryReflection trigger path.

@furedericca-lab
Copy link
Collaborator Author

furedericca-lab commented Mar 6, 2026

Follow-up update for this PR:

  • Merged the fork child PR furedericca-lab/memory-lancedb-pro#1 into fork main
  • Fast-forwarded this PR branch (feat/self-improvement-reflection-agent) to match the reviewed fork mainline
  • This brings in the transient reflection retry fix from commit aa11067 (fix: retry reflection transient upstream failures once)

@furedericca-lab
Copy link
Collaborator Author

@AliceLJY Thanks — I re-checked this specifically against my local OpenClaw 2026.3.2 runtime, and I don’t think the current evidence is enough to conclude that command:new / command:reset is generally not being dispatched on 2026.3.2.

On my side, with 2026.3.2:

  • the plugin registers the expected hooks at startup
  • pre-reset self-improvement note injection is observable
  • reflection artifacts were actually generated under memory/reflections/*
  • and the daily log also recorded the corresponding reflection events

So at least in my environment, the reset/new hook path is not universally non-functional.

I think the remaining concern needs a narrower config/runtime split before we classify it as a Gateway blocker:

  • memory/reflections/* is only expected when sessionStrategy=memoryReflection
  • the plugin default is now systemSessionMemory, not memoryReflection
  • .learnings/* is not a direct proof that command:new/reset fired
  • bundled session-memory output is also not the same validation target as the plugin memoryReflection path

So the remaining possibilities seem narrower:

  1. the QA environment was not explicitly running with sessionStrategy=memoryReflection
  2. chat /new and direct sessions.reset(reason:new) are not following the same runtime path on that surface
  3. or there is a more specific runtime-path discrepancy that still needs isolated repro

My suggestion would be to confirm the exact QA config first (especially sessionStrategy), then test chat /new and direct sessions.reset(reason:new) separately under explicit memoryReflection mode.

furedericca-lab and others added 5 commits March 6, 2026 22:41
…slices (#2)

* refactor(reflection): split invariants vs derived and tighten slice extraction

* fix(memory-reflection): recover sessions from agent dirs

---------

Co-authored-by: furedericca <263020793+furedericca-lab@users.noreply.github.com>
Co-authored-by: furedericca <263020793+furedericca-lab@users.noreply.github.com>
@furedericca-lab
Copy link
Collaborator Author

Final update for this PR:

  • merged the latest upstream/main into this branch again (current head now includes upstream commit 5c89b3d / PR fix: improve embedding provider failure hints #67)
  • merged the reviewed fork child PR furedericca-lab/memory-lancedb-pro#3 into fork main
  • synced that reviewed result back into this PR branch

Current branch status after the final sync/review:

  • merge state remains clean
  • reflection /new live validation already passed in my runtime:
    • markdown reflection artifact written
    • daily log append observed
    • LanceDB reflection entry stored
    • invariants[] / derived[] slices confirmed clean
  • added focused regression coverage for:
    • legacy sessionMemory.enabled -> sessionStrategy mapping
    • slash-command filtering in jsonl_distill.py
    • self-improvement file-write / extract-skill path sanitization flow
    • reset-snapshot fallback helper for reflection session recovery
    • existing retry/session-recovery tests still pass
  • release hygiene was also updated:
    • version bumped to 1.1.0
    • changelog updated
    • README / README_CN now document the current reflection structure, inherit/derive behavior, upgrade semantics, and stored LanceDB metadata fields

Verification run used for the added focused coverage:

  • node --test test/config-session-strategy-migration.test.mjs test/jsonl-distill-slash-filter.test.mjs test/self-improvement-tools-file-write.test.mjs test/reflection-command-reset-fallback.test.mjs test/reflection-retry.test.mjs test/session-recovery-paths.test.mjs
  • npm test

At this point I do not see a remaining known blocker on this branch from my side.

@furedericca-lab furedericca-lab merged commit 95c1c1d into win4r:main Mar 6, 2026
1 check passed
@AliceLJY
Copy link
Collaborator

AliceLJY commented Mar 6, 2026

Follow-up after a more complete runtime check on Gateway 2026.3.2.

I think the main blocker here is now narrowed down to a plugin-side memory-reflection path-resolution bug, not a Gateway dispatch issue.

What I verified:

  • Hook registration is fine: startup logs show both self-improvement and memory-reflection integrated hooks registered for command:new / command:reset.
  • Internal hook dispatch is fine: OpenClaw uses a globalThis.__openclaw_internal_hook_handlers__ singleton, so this does not look like a bundler-isolated Map problem.
  • Direct internal reset is also fine: I successfully called Gateway WS sessions.reset({ key, reason: "new" }) and it returned ok: true.

I tested 3 paths:

  • Discord /new
  • ACP --reset-session
  • Gateway WS sessions.reset(reason:new)

In all cases, hook dispatch is reachable, but memory-reflection still produces no reflection artifact / daily log / write log.

The likely root cause is in runMemoryReflection() around index.ts:1984-1999:

  • sessionEntry.sessionFile is often undefined
  • fallback recovery only searches dirname(currentSessionFile) and workspaceDir/sessions
  • but on Gateway 2026.3.2 the real session files are under ~/.openclaw/agents/<agentId>/sessions/*.jsonl
  • if sessionFile is missing and workspaceDir/sessions does not exist, currentSessionFile stays undefined and the hook returns silently

That matches what I observed in runtime:

  • real files existed under ~/.openclaw/agents/main/sessions/
  • no workspace/sessions/ directory existed
  • session store entries had sessionId but not a usable sessionFile
  • result: session reset/archive succeeds, but reflection never gets input

One more important distinction:

  • self-improvement does not depend on sessionFile, so this looks specifically like a memory-reflection bug
  • so I would not currently blame Gateway for the reflection failure

Suggested fix:

  • extend the fallback search order to include the agent sessions directory, e.g.
    join(homedir(), ".openclaw", "agents", sourceAgentId, "sessions")
  • keep workspaceDir/sessions as a secondary fallback if you still want compatibility with other layouts
  • before the final early return, add a warning log that includes sessionKey, sessionId, and searched directories, otherwise this failure mode stays silent

Something like:

const sourceAgentId = parseAgentIdFromSessionKey(sessionKey) || "main";
searchDirs.add(join(homedir(), ".openclaw", "agents", sourceAgentId, "sessions"));
searchDirs.add(join(workspaceDir, "sessions"));

and if unresolved:

api.logger.warn(
  `memory-reflection: could not resolve session file for ${sessionKey} (${currentSessionId}); searched: ${Array.from(searchDirs).join(", ")}`
);

At this point my current conclusion is:

  • not just Discord /new
  • not just ACP reset surface
  • not just direct Gateway reset surface
  • the common failure is that memory-reflection cannot recover the source session JSONL path under the current Gateway session layout

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.

4 participants