fix(openclaw): align plugin docs with actual runtime output - #662
Merged
Conversation
Collaborator
Style Guide AuditAudited 3 file(s) against the Sui Documentation Style Guide. 4 violation(s) found. All must be fixed before merge.
|
Collaborator
Style Guide AuditAudited 3 file(s) against the Sui Documentation Style Guide. 10 violation(s) found. All must be fixed before merge.
|
ducnmm
requested changes
Aug 18, 2026
ducnmm
left a comment
ducnmm
left a comment
Collaborator
There was a problem hiding this comment.
Need a real 0.0.6 release before this can ship to users.
@mysten-incubation/oc-memwal@0.0.5 is already on npm. This PR (and #668) change runtime behavior — drop required on the manifest so plugins install can write an entry, add contracts.tools, add requestTimeoutMs / withTimeout, plus the docs/config-key fixes. Merging to dev only publishes 0.0.5-dev.N. Merging to main will skip publish as already-released, and users stay on the broken 0.0.5.
Please bump:
packages/openclaw-memory-memwal/package.json→ 0.0.6- plugin / marketplace manifests if they pin the same version
packages/openclaw-memory-memwal/CHANGELOG.md— new## 0.0.6(install deadlock,contracts.tools, hung-relayer timeout, config key / log prefix /allowConversationAccess)docs/openclaw/changelog.mdx— same section + frontmatteranswer:
Can be this PR or a follow-up stacked on it, but it has to land before main.
Nits, not blocking the bump:
- Rebase onto
dev. This branch is ~52 commits behind and overlaps #668 onconfig.ts,recall.ts, andpackage.json. #668 adds dual-namespace recall; a naive merge of this file will drop that. - CLI
stats/searchandclient.health()still have no timeout. Hooks are the turn-blocker; CLI can hang separately.
harrymove-ctrl
force-pushed
the
fix/openclaw-docs-drift
branch
from
August 18, 2026 04:50
64be258 to
bd937cb
Compare
ducnmm
approved these changes
Aug 18, 2026
ducnmm
left a comment
ducnmm
left a comment
Collaborator
There was a problem hiding this comment.
The 0.0.6 bump and changelogs look right. package.json is 0.0.6, and both packages/openclaw-memory-memwal/CHANGELOG.md and docs/openclaw/changelog.mdx have a ## 0.0.6 covering install, contracts.tools, timeout, config key, and allowConversationAccess.
The extra test commit (degrade by behaviour, not log wording) is the right way to stay compatible with #668's dual-namespace recall.
Approve. Merge this to dev first; main is what actually publishes 0.0.6 to npm latest.
The quick-start told testers to confirm a working install by looking for `oc-memwal:` log lines. The plugin logs `memory-memwal:` — its plugin id, not its npm package name — so all three "Verify" steps failed even for a correctly working install. Aligned the four quoted log lines with src/index.ts, hooks/recall.ts and hooks/capture.ts. The published changelog stopped at 0.0.2 while the package shipped 0.0.5, and its frontmatter `answer:` (the text AI search cites) stated the plugin had only two releases. Backfilled 0.0.3, 0.0.4 and 0.0.5 from the package CHANGELOG and corrected the summary. The plugin config UI still offered `relayer.dev.memwal.ai` as the serverUrl placeholder, missed by the WALM-84 migration to memory.walrus.xyz. It now points at the staging relayer the quick-start recommends.
Review of the previous commit found the docs advertise a third lifecycle hook, `before_reset`, described as saving a session summary. It does not exist: the only `api.on(...)` registrations are `before_prompt_build` (hooks/recall.ts) and `agent_end` (hooks/capture.ts), and `git log -S` shows the string only ever appeared in documentation, never in source. SKILL.md is the worst placement — it lists the hook under "Lifecycle hooks run automatically", so an agent reading the integration guide will plan around a hook that never fires. Removed there, in the changelog 0.0.1 entry, in the package CHANGELOG, and in the `answer:` frontmatter that AI search cites. Also corrected the 0.0.4 entry: it claimed the SDK dependency was pinned to 0.0.7, but `npm view @mysten-incubation/oc-memwal@0.0.4 dependencies` shows the published artifact carried `workspace:*` literally. That is precisely the install failure 0.0.5 fixed, so the two entries contradicted each other.
Running an actual gateway against the staging relayer showed the plugin loads and reports `connected`, while two of its three surfaces are dead. Tools never registered. The gateway logs `plugin must declare contracts.tools before registering agent tools`, because the manifest had no `contracts` key at all. memory_search and memory_store therefore never existed, which also makes the documented `tools.allow` step unreachable — there is nothing to allow. Declared both, matching the shape stock plugins use (memory-core ships `contracts.tools: ["memory_get", "memory_search"]`). Auto-capture never ran. The gateway logs `typed hook "agent_end" blocked because non-bundled plugins must set hooks.allowConversationAccess=true`. That setting appeared nowhere in the plugin or the docs, so a config built exactly as the quick-start describes loses fact capture entirely, with no error surfaced to the user. Documented it in the config block with a warning explaining the failure mode. The config key was also wrong. A binding config uses `memory-memwal`, the manifest id, for both `plugins.slots.memory` and the `plugins.entries` key; the quick-start documented the npm name `oc-memwal` for both, which leaves the plugin unbound. The installer states this itself: "using manifest id as the config key". Both fixes verified by experiment: with them applied the blocked-hook and must-declare warnings disappear and the gateway still reaches `ready` and `connected (status: ok, version: 0.1.0)`. Finally, the CLI reference claimed `openclaw memwal` needs a running gateway. It does not — the commands talk to the relayer directly and were confirmed working with the gateway stopped.
… turns
Two defects found by installing the plugin into a real OpenClaw gateway and
pointing it at a mock relayer that misbehaves.
Install was a deadlock. `openclaw plugins install` writes its config entry
before the user supplies credentials, but the manifest marked privateKey,
accountId and serverUrl as required, so OpenClaw's own validation rejected
that write and the install aborted. Pre-creating the config failed the other
way — `plugins.slots.memory: plugin not found` — and nothing extracted. The
remedy the CLI suggests, `doctor --fix`, silently deleted the entry and
reassigned the memory slot to `memory-core`, so a user following the error
message lost the plugin entirely.
Dropped `required` from the manifest schema. Validation already lives in
parseConfig, which reports better errors at a better time: install now
succeeds and writes `{enabled: true}`, and a gateway started without
credentials fails loudly at register with per-field messages while staying
up. That is the install-then-configure order the docs describe.
Second, there was no client-side timeout. Against a relayer that accepts the
socket and never replies, the recall hook stayed pending indefinitely and
blocked the agent turn. An earlier resilience pass missed this because it
used an unreachable hostname, which fails fast at DNS; a slow or overloaded
relayer holding the socket open is the realistic case. Added `withTimeout`,
applied to every relayer call in the hooks and both tools, configurable via
`requestTimeoutMs` (default 10s, 1s–60s). In capture it sits inside the
retry so each attempt gets its own deadline. A hung call now settles in 10s
and degrades like any other failure.
Added `test/plugin.test.mjs` — 21 node:test cases pinning both fixes as
regressions, plus config validation, key masking, timeout and retry
behaviour, hook degradation, and the escaping and tag-stripping paths.
Publishing is gated on the version: 0.0.5 is already on npm, so the main publish step reports "already published, skipping" and none of the install, tools, timeout or injection fixes reach users. Bumped to 0.0.6 with matching sections in the package CHANGELOG and the published changelog, following the file set used for the MCP 0.0.8 release. Also cleared the Sui style-guide audit on the touched docs: replaced em dashes with commas or sentence breaks, swapped italics for backticks on the installer message, moved a future-tense clause to present tense, and gave every changelog version an introductory sentence so no heading stacks directly on another.
Merged this branch with #668 locally to check the overlap Henry flagged. The merge conflicts in exactly one place, hooks/recall.ts, and the resolution is small: #668 turns the single recall into a dual read across the canonical and legacy namespaces, so each read needs its own deadline rather than one around the pair. Promise.allSettled waits for every promise, so a single namespace holding its socket open would otherwise still stall the turn. On the merged tree the behaviour is right (no throw, namespace instruction preserved, timeout fires) but this test failed on the log string: allSettled absorbs the rejection and logs "canonical recall failed" instead of reaching the outer catch. Assert the timeout fired and accept either message, so the test holds before and after #668 lands.
Re-reviewing this branch, the comments overstated the gap. `recall()` does have a client-side abort at 15s. What has none is `analyze()`, which goes through `signedRequest` without a signal, and the compatibility preflight (`GET /version`, falling back to `/health`) that runs ahead of every protected request. That preflight is where the mock relayer actually hung, before `recall()`'s own abort could apply. The fix is unchanged and still correct, since it bounds the whole call whichever leg stalls. Only the reasoning was wrong, and it would have shipped an inaccurate claim about the SDK.
Now that #668 has landed, I measured its filter against a corpus of realistic developer statements: 7 of 12 were flagged as injection and silently dropped, against 4 of 12 before that PR. Nothing is surfaced to the user, so a memory simply never appears. Two patterns cause it, and both match ordinary speech as readily as attacks. `run|execute|call|invoke ... tool|command|shell|bash` catches "I need to run the deploy command before standup", and `forget ... everything ... before` catches "I forget everything before my first coffee". The regex alone cannot separate the two readings. What separates them is who is being addressed. Injection has to reach the model, so it either names it ("you", "your") or leads with a bare injection verb. A sentence doing neither is someone describing their own workflow. Those two patterns now live in a contextual set that only counts when the text addresses the model; every explicit-attack pattern still fires unconditionally. False positives fall from 7 of 12 to 1 of 12. The remaining one is a bare "system prompt" mention, which predates #668 and stays flagged. The 15-payload attack set from #639 is unchanged at 4 bypasses, so the relief does not come at the cost of coverage. Both corpora are pinned as tests.
harrymove-ctrl
force-pushed
the
fix/openclaw-docs-drift
branch
from
August 18, 2026 07:03
f9e491b to
5d111ac
Compare
Merged
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
4 participants
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.
Drift fixes found while preparing the E2E test pass for the OpenClaw plugin (Notion task: E2e test openclaw SDK). All three verified against
dev.1. Quick-start verification steps were unfollowable — blocker for the E2E
The quick-start told testers to confirm a working install by looking for these log lines:
The plugin logs
memory-memwal:— its pluginid, not its npm package name. Seesrc/index.ts:43,59,63,68,src/hooks/recall.ts:56,68,src/hooks/capture.ts:37,75,81(11 call sites, allmemory-memwal:).Net effect: all three "Verify" steps in the quick-start fail for a correctly working plugin. Fixed the four quoted lines to match the code.
2. Published changelog was 3 releases stale
docs/openclaw/changelog.mdxended at0.0.2while the package ships0.0.5. Its frontmatteranswer:— the text AI search cites — stated the plugin had releases "from the initial 0.0.1 through 0.0.2".Backfilled
0.0.3(rebrand),0.0.4(temporal anchoring /occurredAt),0.0.5(theworkspace:*publish fix) from the packageCHANGELOG.md, which was already current, and corrected theanswer:summary.3. Stale
memwal.aidomain in the plugin config UIopenclaw.plugin.jsonstill offeredhttps://relayer.dev.memwal.aias theserverUrlplaceholder — missed by the WALM-84 migration tomemory.walrus.xyz. This string is user-visible in the OpenClaw plugin config UI. Now points at the staging relayer the quick-start recommends.Deliberately not in this PR
/openclaw/changelog404s on the live site — but so do the other threechangelog.mdxpages (sdk,mcp,python-sdk) and two plain.mdpages (sdk/example-map,sdk/research-app-example) that exist onmain. I falsified.mdx-only, deploy-lag, and frontmatter-shape as causes;check-docs-freshness.mjspasses and reports 91 pages but only 44 routes. Root cause looks like it sits in the docs publishing pipeline, which is not in this repo (no docs deploy workflow here, only a Mintlify$schemareference). Needs the docs-platform owner rather than a guessed rename.oc-memwal@0.0.5exact-pins@mysten-incubation/memwal@0.0.7(2026-06-02); current SDK is0.1.2. Not an auth break —x-seal-sessionandx-noncelanded in SDK0.0.2, and0.0.7 >= minSupportedSdk 0.0.4— but the plugin therefore misses the0.1.0hardening that makes recalled memory nonce-delimited untrusted data, plus0.1.2idempotency keys and Zod 4 compat. That is a functional change needing its own test cycle.Heads-up on release
This touches
packages/openclaw-memory-memwal/**, so merging todevtriggersrelease-oc-memwal.ymland publishes a0.0.5-dev.Nprerelease. Expected, flagging so it is not a surprise.Verification
node scripts/check-docs-freshness.mjs→docs freshness OK (91 pages, 44 routes, IDs, URLs, versions, limits)(this is theDocs / FreshnessCI job)openclaw.plugin.jsonre-parsed as valid JSONoc-memwal:log prefix remains anywhere outside the package name itself