fix(reflection): restore reflection on current hosts (hook transcript, runEmbeddedAgent runner, headless CLI fallback) - #995
Open
gorkem2020 wants to merge 6 commits into
Conversation
…on SQLite hosts OpenClaw hosts with SQLite session storage no longer expose a transcript file to plugins: the session entry carries no sessionFile, the legacy sessions directories are gone, and the command:new / command:reset hook context provides the departing session's recent messages instead (previousSessionMemory, one role-prefixed JSON record per line). The reflection hook kept resolving a session file, logged "missing session file after recovery" and returned, so no reflection ran on those hosts. Reflection now parses previousSessionMemory back into turns and runs the same pipeline on them first; the session-file lookup remains as the legacy fallback for hosts that still write files. Session-dir recovery also reads agents.entries (the object form of the agent config) alongside agents.list, matching the md-mirror path. Regressions: reflection runs from the hook transcript with no session file, the file path still works without a hook transcript, an empty or unavailable hook transcript falls back to the file lookup, and the search dirs enumerate agents.entries ids and workspaces.
…dless CLI exec turn The host renamed api.runtime.agent.runEmbeddedPiAgent to runEmbeddedAgent and removed the alias, so the embedded distiller layer stopped resolving a runner; the extensionAPI.js import layer no longer exists on those hosts; and the CLI layer hardcoded `openclaw agent --local`, which the host now refuses while a gateway owns the state directory. Every reflection fell through to the minimal fallback pointer, and the 400-char diagnostic clip showed only startup banners instead of the refusal. The runner lookup now accepts runEmbeddedAgent first and the legacy name second (SDK layer and extensionAPI.js layer alike); the distiller run is marked sessionPersistence: "detached" so it never lands in the session store; the CLI fallback drives `openclaw agent exec` (prompt over stdin, workspace via --cwd, resolved provider/model via --model) and retries the legacy --local shape only when the host rejects the exec arguments; and stderr diagnostics keep the tail with state-migration banners and ANSI codes removed. Regressions: runner-name preference and legacy acceptance, the detached embedded run through runEmbeddedAgent, the exec and legacy argument shapes, the legacy retry decision, tail-preserving diagnostic clipping, and the exec JSON envelope (payloads then final).
… runner
Current hosts run a plugin session-ownership check on every embedded run
and treat a sessionFile that is not a session key as a foreign transcript
("Plugin session ownership checks require a SQLite transcript marker"), so
the distiller's temporary jsonl path made runEmbeddedAgent refuse the run
and reflection fell through to the CLI layer on every boundary. The
legacy runEmbeddedPiAgent still reads its transcript from that path.
The runner loader now records which export it resolved, and the distiller
passes sessionFile only when the legacy runner is in use; the detached
run on current hosts carries no transcript path at all.
Regressions: the renamed-runner run sees no sessionFile; the legacy runner
still receives a jsonl path.
gorkem2020
marked this pull request as ready for review
September 12, 2026 16:15
… before_reset messages The Gateway command path emits command:new before the departing transcript is captured, so the hook can arrive with neither previousSessionMemory nor a session file. Park such a boundary per session key and finish the same reflection pipeline from the typed before_reset hook, which core fires right after the command hooks on every path with the departing messages. A boundary reflects at most once; orphan or non-boundary before_reset events are ignored.
…d's root-work context Core refuses embedded sub-runs enqueued from a released root-work context, which is where the fire-and-forget before_reset hook runs, so the continuation fell back to the CLI runner on every /new. Capture an async context snapshot at registration and run the continuation in it; the global restart and suspension fences still apply.
rwmjhb
requested changes
Sep 13, 2026
rwmjhb
left a comment
Collaborator
There was a problem hiding this comment.
The core direction is good, and I verified the relevant contracts against current OpenClaw 2026.9.1: before_reset, runEmbeddedAgent, sessionPersistence: "detached", and the proposed agent exec flags all exist. The full suite and npm run build also pass. The fallback still has four correctness/safety blockers:
- The embedded path is explicitly model-only (
disableTools: true,modelRun: true), but the CLI fallback sends the session transcript toopenclaw agent execin the real workspace with no enforced tool restriction. Current OpenClaw defaults that command totools.profile = "coding"andtools.exec.mode = "full", and its CLI exposes no disable-tools option. A user-controlled instruction in the transcript can therefore invoke shell/filesystem tools during an automatic reset. Please use a genuinely tool-free completion path or otherwise enforce a deny-all tool surface and test that enforcement. - Exec mode drops
params.agentId. Currentagent exechas no--agentflag and resolves the ambient/default owner and that owner's auth directory, so a non-default agent can use the wrong OAuth profile/configuration or fail despite having valid credentials. The fallback must preserve the source agent identity. embeddedRunnerExportNameis updated independently of theembeddedPiRunnerPromisecached with??=. Reproducing a legacy-runner resolution followed by a current-runner resolution returns the cached legacy runner twice, while the second call labels it current and omitssessionFile. Cache the runner and its kind atomically.- A recovered but empty/unusable legacy transcript goes directly to the empty guard and never parks for
before_reset. On a migrated host with a stale JSONL artifact, that stale file can therefore hide the valid SQLite-backed messages supplied by the typed hook. Park the boundary when the file yields no usable conversation as well.
Please add integration coverage for a tool-free, source-agent-preserving CLI fallback, plus regressions for runner-kind cache transitions and the empty-file-to-before_reset continuation.
…park empty transcripts for before_reset The CLI fallback ran the departing transcript through an agent turn with the host's default tool surface and no --agent flag; replace it with a plain completion on the plugin's own LLM lane (completeText on every transport), so no tool can be reached and no agent identity has to be resolved. Cache the embedded runner together with its kind so a later host surface cannot relabel it. A recovered transcript file that yields no usable conversation parks the boundary for the typed before_reset messages instead of recording the empty guard.
Contributor
Author
|
Thanks, all four are addressed on the new head (307a21c).
Suite and build are green; the PR body is updated to match. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On OpenClaw hosts with SQLite session storage the reflection hook never produced a reflection. Two independent breaks, both fixed here:
command:new/command:resethook context no longer carries a transcript file path (core removed that surface in its plugin SDK, registry entryremoved-session-transcript-file-api, shipped in release 2026.7.1 via openclaw#98236), the legacyagents/<id>/sessions/*.jsonllayout is gone once an install migrates, and the session-dir recovery only reads the array form of the agent config. The hook loggedmissing session file after recoveryand returned.runEmbeddedPiAgenttorunEmbeddedAgentand the alias removed (core compat registry entryembedded-pi-agent-sdk-aliases, status removed),dist/extensionAPI.jsno longer exists, and the CLI layer hardcodesopenclaw agent --local, which the host refuses while a gateway owns the state directory. Generation fell through to the minimal fallback pointer on every session boundary, and the 400-char stderr clip showed only startup banners, never the refusal.Core hands the departing session's recent messages to the same hook as
previousSessionMemory(onerole: "<json text>"record per line). Reflection now uses that transcript first (session-file lookup stays as the legacy fallback, the same messages-first shape the plugin'sbefore_resethandler already uses) and runs the distiller through the current runner names, with a tool-free completion on the plugin's own LLM lane as the fallback.Changes
index.ts, transcript:conversationFromHookSessionMemoryparsespreviousSessionMemoryback into turns and feeds the existingsummarizeRecentConversationMessagespipeline;runMemoryReflectiontries it before any file resolution and logsusing the hook-provided transcript. The file path, the reset-suffix fallback and the empty-session guard are unchanged for hosts that still write files.src/session-recovery.ts: agent enumeration readsagents.entries(object keyed by agent id) alongsideagents.list(array), for both ids and workspace-derived homes.index.ts, runner:resolveEmbeddedRunnerExportNamepicksrunEmbeddedAgentfirst and still acceptsrunEmbeddedPiAgent, for both the SDK layer and theextensionAPI.jsimport layer. The distiller run is markedsessionPersistence: "detached"so it never lands in the host session store (hosts that predate the field ignore it).index.tsandsrc/llm-client.ts, fallback: when the embedded runner is unavailable the distiller prompts go tocompleteText, a new plain-text completion on the plugin's LLM client (host, api-key and OAuth transports), built for the reflection lane frommemoryReflection.modeland the configuredllm.*credentials. No agent turn, no tool surface, no agent identity to resolve. The CLI runner (openclaw agent exec/--local) and its argument, retry and diagnostic helpers are removed. Model resolution lives inresolveReflectionModelTarget.index.ts, runner cache: the embedded runner and its export name are cached together (loadEmbeddedPiRunnerresolves{ runner, exportName }), so a host surface seen later cannot relabel an already cached runner; the transcript-file decision reads the cached kind.test/reflection-hook-session-memory.test.mjs(reflection from the hook transcript with no session file, the file path without a hook transcript, empty and unavailable hook transcripts falling back to the file lookup, search dirs from both config shapes) andtest/reflection-runner-embedded-agent.test.mjs(runner-name preference and legacy acceptance, the detached embedded run throughrunEmbeddedAgent, the legacy runner's transcript file, the completion fallback receiving the distiller prompts, the static fallback when the completion returns nothing or no client exists, and the runner cache keeping runner and kind together across host surfaces). Both registered in thenpm testchain and the CI manifest (core-regression).Notes
previousSessionMemoryis bounded by the host'shooks.internal.entries["session-memory"].messages(default 15), which is smaller thanmemoryReflection.messageCount; hosts that want the full window can raise that setting. A deeper read through the plugin SDK's transcript helpers is a possible follow-up; the upstream request for companion-friendly transcript access is openclaw#79902.command:resetandcommand:newboth loggedmissing session file after recoverywith a single non-existent candidate dir, while auto-capture on the same session kept working becauseagent_endcarries the messages directly. With the transcript fix alone, generation ended inrunner=fallbackand the written file held only the fallback pointer; reproducing the CLI layer by hand showed the--localrefusal that the clipped diagnostic had hidden.Verification
npm run build,npm test(935 passing after round 2), both new regression files red against the previous code by construction, plus the existingcommand-reflection-guard,issue606_sdk-migration,raw-run-distiller-hooks,reflection-per-agent-laneandwindows-reflection-fallbacksuites.Update: typed
before_resetcontinuation (third commit)On the Gateway command path core emits
command:newbefore it captures the departing transcript, sopreviousSessionMemoryis absent there (and on the immediate web chat/newpath), while/resetreaches the reply path with it present. Core's maintainers pointed at the typedbefore_resethook as the supported integration (openclaw#146358): it fires right after the command hooks on every path and carries the departing messages plus the session identity.command:new/command:resethook that finds neither a hook transcript nor a session file now parks the event (per session key, 60 s TTL) instead of recording the empty guard, and logs that it waits forbefore_reset.before_reset; for a parked session key with reasonneworresetit builds the conversation fromevent.messages(the same summarizer the session-memory feature uses) and finishes the same reflection pipeline with it, bypassing the event dedup and the serial-guard stamp left by the parked hook. Nothing runs when no hook is parked, when the reason is not a session boundary, or when the command hook already carried the transcript, so a boundary reflects at most once.before_resetmessages end in the existing empty/unusable guard.test/reflection-before-reset-transcript.test.mjs(park, then finish frombefore_resetwith exactly one run; an orphanbefore_resetis ignored; no second run when the command hook had the transcript; empty messages record the guard; non-boundary reasons leave the parked hook alone). The hook-session-memory test's missing-file cases now assert the parking log line.AsyncLocalStorage.snapshot()taken at plugin registration. Core refuses embedded sub-runs enqueued from a released root-work context (isGatewaySubordinateWorkAdmissionClosed), and the fire-and-forgetbefore_resethook inherits the released/newroot, so without the snapshot every/newreflection fell back to the completion path. The global restart and suspension fences still apply to the continuation. Covered by a test that fires the hooks inside a foreign async context and asserts the embedded runner is invoked outside it.Round 2 (review of 2026-09-13)
before_resettoo; the continuation ignores the empty guard and records it only when thebefore_resetmessages are empty.test/command-reflection-guard.test.mjsdrives the repeated-empty case throughbefore_resetaccordingly, andtest/reflection-before-reset-transcript.test.mjsadds the stale-file case.test/windows-reflection-fallback.test.mjskeeps its import-specifier cases and loses the two spawn-command cases with the CLI code.