ci(openclaw): run the plugin unit suite per PR and the E2E suite on staging/main - #683
Open
harrymove-ctrl wants to merge 2 commits into
Open
ci(openclaw): run the plugin unit suite per PR and the E2E suite on staging/main#683harrymove-ctrl wants to merge 2 commits into
harrymove-ctrl wants to merge 2 commits into
Conversation
The plugin has two test files and neither ran anywhere. #668 added test/security.test.mjs for the GH #639/#640 injection work and #662 added test/plugin.test.mjs, but `test.yml` never referenced oc-memwal, so both only ran if someone typed the pnpm filter by hand. The regression tests guarding a security fix were not guarding anything. Adds an OpenClaw Plugin / Unit tests job in the same shape as the Noter one: install, build the SDK because the plugin depends on it through the workspace, then run the package's own test script. That script is `tsc && node --test`, so a type error fails the job too, and the 30 cases now hold the line on every defect found by running the plugin against a real gateway: the manifest must not mark credential fields required or `plugins install` deadlocks, the agent tools must stay declared in contracts.tools or neither registers, relayer calls must keep their deadline, and the injection filter must not regress in either direction.
… main The E2E work lived in a gitignored scratch directory, so nobody but me could run it and CI could not run it at all. Moves it into the package as two suites split by what they cost. mock-relayer.test.mjs serves a broken relayer from the test process: rate limiting, 5xx, and a socket held open with no reply. No credentials, no network, no cost. The hang case is the point of it. Before the request deadline existed a quiet relayer left the recall hook pending forever and the turn never completed, and an unreachable hostname does not reproduce that because DNS fails fast. live-relayer.test.mjs runs against a real relayer and skips itself unless credentials are present, so it stays safe locally and in any job without secrets. Writes are separately opt-in behind MEMWAL_E2E_WRITE because Walrus is append-only with no per-blob delete: every write is permanent and costs gas plus storage. They land in a throwaway e2e-<timestamp> namespace so they can never touch `default`. Authorisation is checked with a signed recall rather than /health, which is unauthenticated and answers even for a revoked key. CI gets an OpenClaw Plugin / E2E job restricted to pushes on staging and main, never per PR, and pointed at the staging relayer even on a main merge since there is no reason to leave test data on mainnet. Missing secrets degrade to skips rather than failures, so the job can land before the secrets exist. The unit glob narrows to test/*.test.mjs so the per-PR job does not pick up E2E, with test:e2e covering test/e2e/*.test.mjs. One detail worth recording: the hang case first took 301s despite the deadline firing in 1.5s. withTimeout races the request rather than aborting it, since recall() builds its own AbortController and takes no external signal, so the abandoned fetch held its socket and server.close() waited on it. The mock now force-closes connections. That socket leak is real in production too, and fixing it properly means threading an AbortSignal through the SDK.
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.
The OpenClaw plugin had tests that ran nowhere, and E2E coverage that lived in a gitignored scratch directory.
#668 added
test/security.test.mjsfor the GH #639 / #640 injection work and #662 addedtest/plugin.test.mjs, butoc-memwalappears nowhere intest.yml, so both only ran if someone typed the pnpm filter by hand. The regression tests guarding a security fix were not guarding anything.Two jobs, split by cost
OpenClaw Plugin / Unit tests— every PR. Runstest/*.test.mjs: 30 cases, no credentials, no network.OpenClaw Plugin / E2E— pushes tostagingandmainonly, never per PR. Runstest/e2e/*.test.mjs.The split exists because Walrus storage is append-only with no per-blob delete, so every E2E write is permanent and costs gas plus storage. Running that per PR would be wrong.
The E2E suites
test/e2e/mock-relayer.test.mjsserves a deliberately broken relayer from the test process — rate limiting, 5xx, and a socket held open with no reply. No credentials, no network, no cost. The hang case is the point: before the request deadline existed, a relayer that accepted the connection and went quiet left the recall hook pending forever and the turn never completed. An unreachable hostname does not reproduce that, because DNS fails fast.test/e2e/live-relayer.test.mjsruns against a real relayer and skips itself unless credentials are present, so it is safe locally and in any job without secrets. Writes are separately opt-in behindMEMWAL_E2E_WRITE=1and land in a throwawaye2e-<timestamp>namespace, neverdefault. Authorisation is proven with a signedrecall()rather than/health, which is unauthenticated and answers even for a revoked key.Secrets
The E2E job reads
MEMWAL_E2E_PRIVATE_KEYandMEMWAL_E2E_ACCOUNT_ID. Neither exists yet, and that is fine — without them the live cases skip and the mock cases still run, so this job cannot fail a branch. It targets the staging relayer even on amainmerge, since there is no reason to leave test data on mainnet.What the cases hold the line on
Each maps to a defect found by running the plugin against a real gateway:
required, orplugins installdeadlocks in both directionscontracts.toolsmust stay declared, or neither agent tool registers and the documentedtools.allowstep is impossibleVerification
skippingOne detail worth flagging for review: the hang case initially took 301s despite the deadline firing at 1.5s.
withTimeoutraces the request rather than aborting it, becauserecall()builds its ownAbortControllerand accepts no external signal, so the abandoned fetch kept its socket andserver.close()waited on it. The mock now force-closes connections. That socket leak is real in production too; fixing it properly means threading anAbortSignalthrough the SDK, which is out of scope here.