Skip to content

fix(openclaw-plugin): default recall to assemble and bound afterTurn#1424

Open
0xble wants to merge 12 commits intovolcengine:mainfrom
0xble:fix/openclaw-assemble-recall
Open

fix(openclaw-plugin): default recall to assemble and bound afterTurn#1424
0xble wants to merge 12 commits intovolcengine:mainfrom
0xble:fix/openclaw-assemble-recall

Conversation

@0xble
Copy link
Copy Markdown
Contributor

@0xble 0xble commented Apr 14, 2026

Description

Default the OpenClaw plugin to an assemble-first recall path when OpenViking
owns the contextEngine slot, and fail open if the plugin's hot-path capture
work gets slow.

This avoids running hook-based recall and context-engine recall on the same
reply path, updates the setup helper to resolve the active OpenClaw config
file so existing installs upgrade cleanly, and bounds afterTurn auto-capture
so slow OV writes do not keep an OpenClaw run open after the model already
replied. It also keeps default plugin logs content-safe: session summaries,
message previews, and captured turn text are only emitted when debug routing
logs are enabled.

Related Issue

Related to #1283

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Moved recall assembly into assemble() by default and added a recallPath
    config so before_prompt_build only runs in explicit compatibility mode.
  • Updated the default OpenClaw plugin install path to set
    recallPath=assemble, disable prompt injection in context-engine mode, and
    resolve the active OpenClaw config file instead of assuming
    openclaw.json.
  • Bounded afterTurn auto-capture with a fail-open timeout so slow OpenViking
    session writes cannot block OpenClaw run completion after the assistant has
    already produced a reply.
  • Gated content-bearing diagnostic logs behind logFindRequests and kept
    default info logs to non-content metadata such as counts, status, task, and
    trace IDs.
  • Kept ingest-reply assist available when hook-mode recall client
    initialization fails, so a recall outage does not suppress the non-client
    prompt assist path.
  • Added regression coverage for the assemble-first flow, compatibility mode,
    setup-helper config handling, content-safe default logging, recall init
    fallback, and afterTurn timeout fallback behavior.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Commands run:

  • npm exec --package typescript -- tsc --noEmit
  • npm test -- tests/ut/local-startup-failure.test.ts tests/ut/plugin-bypass-session-patterns.test.ts tests/ut/context-engine-assemble.test.ts
  • npm test
  • git diff --check
  • cr review --type committed --base upstream/main --agent

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A

Additional Notes

  • I reproduced the original failure locally on OpenClaw 2026.4.12 with the
    OpenViking plugin at v0.3.5.
  • This PR stays plugin-scoped. It does not change OpenClaw's separate
    gateway-timeout fallback behavior.
  • Merged current upstream main into the PR branch to resolve the reported
    conflict.

Move the OpenClaw plugin to an assemble-first recall path when the context-engine slot is active, keep before_prompt_build as compatibility-only behavior, and teach the setup helper to resolve the active OpenClaw config file so json5-based installs upgrade cleanly.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 14, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1283 - Partially compliant

Compliant requirements:

  • Auto-recall logic moved to assemble() via recallPath="assemble" default
  • before_prompt_build hook skipped when recallPath !== "hook"
  • Dual conflict avoided by separating recall paths

Non-compliant requirements:

(empty)

Requires further human verification:

(empty)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 88
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Improve OpenClaw config path detection

Relevant files:

  • examples/openclaw-plugin/setup-helper/install.js

Sub-PR theme: Move recall to assemble() for OpenClaw 4.5 compatibility

Relevant files:

  • examples/openclaw-plugin/config.ts
  • examples/openclaw-plugin/context-engine.ts
  • examples/openclaw-plugin/index.ts
  • examples/openclaw-plugin/recall-context.ts
  • examples/openclaw-plugin/openclaw.plugin.json
  • examples/openclaw-plugin/tests/ut/config.test.ts
  • examples/openclaw-plugin/tests/ut/context-engine-assemble.test.ts
  • examples/openclaw-plugin/tests/ut/local-startup-bad-config-real.test.ts
  • examples/openclaw-plugin/tests/ut/local-startup-failure.test.ts
  • examples/openclaw-plugin/tests/ut/plugin-bypass-session-patterns.test.ts
  • examples/openclaw-plugin/tests/ut/plugin-normal-flow-real-server.test.ts

Sub-PR theme: Update plugin documentation for new recall path

Relevant files:

  • examples/openclaw-plugin/INSTALL.md
  • examples/openclaw-plugin/README.md

⚡ Recommended focus areas for review

Bare Error Swallowing

The resolveMemoryContent function uses a bare catch block that swallows exceptions without logging. This could hide real errors when reading memory content, making debugging harder.

try {
  const fullContent = await readFn(item.uri);
  content =
    fullContent && typeof fullContent === "string" && fullContent.trim()
      ? fullContent.trim()
      : (item.abstract?.trim() || item.uri);
} catch {
  content = item.abstract?.trim() || item.uri;
}

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@0xble 0xble changed the title fix(openclaw-plugin): default recall to assemble fix(openclaw-plugin): default recall to assemble and bound afterTurn Apr 14, 2026
@Mijamind719 Mijamind719 self-assigned this Apr 14, 2026
@Mijamind719
Copy link
Copy Markdown
Collaborator

“这些 logger.info 现在会在非 debug 模式下输出 session context / summary / message preview,默认日志会带上用户内容。建议收敛到 logFindRequests/debug 开关后,或只保留计数与 trace_id 等非内容信息。”

代码有confilict

@0xble
Copy link
Copy Markdown
Contributor Author

0xble commented Apr 15, 2026

Kept the ingest-reply-assist fallback even when recall init fails, pushed the conflict resolution, and tightened the noisy session-content logs so full summaries/message previews stay behind debug logging.

@0xble
Copy link
Copy Markdown
Contributor Author

0xble commented Apr 15, 2026

Did one simplify pass on the current branch head. This only removes a duplicated roughEstimate(messages) call in the assemble path and reuses the same token estimate for passthrough decisions; behavior stays the same.

@Mijamind719
Copy link
Copy Markdown
Collaborator

I found one functional regression in the new afterTurn / assemble flow.

  1. Tool call history no longer round-trips through OpenViking session storage.
    • extractNewTurnMessages() now keeps only text blocks for normal user / assistant messages, so assistant-side toolUse blocks are dropped during capture (text-utils.ts, around extractPartText() and extractNewTurnMessages()).
    • toolResult messages are then persisted as role="user" tool parts (text-utils.ts, context-engine.ts).
    • On the read path, convertToAgentMessages() only reconstructs tool parts when msg.role === "assistant", so those stored tool results are ignored and come back as an empty user message (context-engine.ts).

I reproduced this locally with a focused afterTurn -> assemble round-trip using:

  • one assistant message containing text + a toolUse
  • one matching toolResult

The stored OV session contained:

  • an assistant message with only the text part
  • a user message with the tool part

Then assemble() returned:

  • the assistant text
  • an empty user message

instead of reconstructing the original tool-use / tool-result pair.

Impact: once a session is restored from OV, agents can lose the actual tool invocation/result history they may need for continuity, debugging, or follow-up decisions.

I’d recommend either:

  • preserving tool-use / tool-result under a shape that can round-trip cleanly through convertToAgentMessages(), or
  • teaching convertToAgentMessages() to reconstruct tool parts stored on non-assistant roles,

and adding a dedicated round-trip test that exercises afterTurn() followed by assemble().

For context, I also ran the current plugin test suite locally (vitest run), and it passed; this regression seems to be a missing round-trip coverage gap rather than an already-failing path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants