Repository navigation
[OPIK-8663] fix: support OpenClaw 2026.9 - #147
Conversation
The published plugin exports nothing on OpenClaw 2026.9.7: - The root `openclaw/plugin-sdk` barrel is gone, so the plugin failed to load. Runtime imports now use `openclaw/plugin-sdk/core` and `openclaw/plugin-sdk/diagnostic-runtime`. - OpenClaw registers the plugin again for every agent turn and dispatches typed hooks to that registration, while services only start on the gateway's registration, so every hook ran without an Opik client. Hooks now go through a process-wide router to the registration whose service started. - llm_output arrives after agent_end in a later tick, and the trace was already finalized, so LLM output, usage and cost were lost. agent_end now waits for llm_output (or a 1 s grace) when the LLM span is still open, and a new turn in that window finalizes the previous trace first. - subagent_spawning was removed; subagent_spawned creates the span. - loadConfig/writeConfigFile were removed; the CLI uses config.current() and mutateConfigFile, writing back only the Opik plugin entry. Also map OpenClaw's Gemini providers (google, google-vertex) to Opik's canonical names so those spans get a cost, raise the compat floor to 2026.9.7, and run the E2E job on Node 24 with the install consent flags the new host requires. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
| Topic | Details | |||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| Runtime and tooling | Require OpenClaw 2026.9.7 and Node.js 24.16, migrate CLI configuration writes to config.current() and mutateConfigFile, and update README prerequisites, hook documentation, packaging, release, and E2E installation workflows.Modified files (7)
Latest Contributors(2)
| |||||||||
| OpenClaw compatibility | Update plugin registration and tracing to work across OpenClaw 2026.9.7’s module copies and changed hook lifecycle, routing hooks to the active service and waiting for late llm_output events. Capture provider failures, normalize Gemini providers for pricing, and migrate subagent handling to subagent_spawned.Modified files (12)
Latest Contributors(2)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| // Clear deferred finalization state so stale microtasks and timers no-op. | ||
| clearTimeout(active.pendingFinalize); | ||
| active.pendingFinalize = undefined; | ||
| active.agentEnd = undefined; | ||
| active.output = undefined; | ||
|
|
There was a problem hiding this comment.
Shutdown discards completed trace data
When shutdown occurs during the grace window, stop() routes active traces through closeActiveTrace, which clears agentEnd/output and ends the trace without trace.update, so collected completion/output/usage data is omitted before the final flush — should we use finalizeTrace’s metadata/output path, retain a fallback for truly incomplete traces, and add a regression test?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In src/service.ts
around lines 333-338, review the `closeActiveTrace`/`stop()` shutdown logic: stopping
during the agent_end LLM-output grace window clears `agentEnd` and `output` before
`trace.update`, permanently dropping collected metadata and output. Refactor `stop()` to
finalize each eligible active trace through `finalizeTrace` before ending it, while
retaining `closeActiveTrace` as a safe fallback for genuinely incomplete traces, and add
a regression test covering shutdown during the grace window.
There was a problem hiding this comment.
There was a problem hiding this comment.
Fixed in b2e3f90: stop() now finalizes a turn that already reached agent_end, so its output and metadata are written; only turns without agent_end are just closed.
| if (active.llmSpan && agentEndLlmOutputGraceMs > 0) { | ||
| // llm_output hasn't arrived yet; it lands after agent_end on OpenClaw 2026.9+. | ||
| clearTimeout(active.pendingFinalize); | ||
| active.pendingFinalize = setTimeout(finalizeIfCurrent, agentEndLlmOutputGraceMs); | ||
| active.pendingFinalize.unref?.(); |
There was a problem hiding this comment.
Stale sweep drops late LLM output
Arming the grace timer leaves lastActivityAt stale, so stale cleanup can delete the pending trace before the one-second grace ends when agent_end arrives after the timeout boundary, causing the subsequent llm_output to discard its output/usage — should we refresh or protect pending traces until grace finalization, with a boundary regression test?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In `src/service.ts`
around lines 698-702, the `registerHooks` `agent_end` grace-finalization logic leaves
`lastActivityAt` unchanged, so the stale-trace sweep can delete the trace before late
`llm_output` is processed. Refactor this flow to refresh activity or explicitly protect
traces with a pending finalization timer, ensuring stale cleanup cannot remove them
until the grace period completes and late output/usage is retained. Add a regression
test covering an `agent_end` near the stale timeout boundary followed by late
`llm_output`.
There was a problem hiding this comment.
Fixed in b2e3f90: agent_end refreshes lastActivityAt, so a trace waiting for llm_output isn't swept as stale.
There was a problem hiding this comment.
| // A new turn inside the grace window after agent_end belongs to a new trace. | ||
| if (deps.activeTraces.get(sessionKey)?.pendingFinalize) { | ||
| deps.finalizeEndedTurn(sessionKey); | ||
| } | ||
| const existing = deps.activeTraces.get(sessionKey); |
There was a problem hiding this comment.
New turn is closed by prior finalizer
After llm_output clears pendingFinalize and queues its microtask, a same-session llm_input bypasses the guard and reuses the old trace, so the unchanged traceRef check finalizes it and closes/deletes the new turn. Could we track deferred-finalization generation/trace state separately from the timer marker and add a regression test for late llm_output followed immediately by llm_input?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`src/service/hooks/llm.ts` around lines 64-68, update `registerLlmHooks` so deferred
finalization tracks its generation or trace identity independently of `pendingFinalize`;
a same-session `llm_input` arriving after a late `llm_output` must invalidate the queued
callback or force a new trace rather than letting it finalize the new turn. Ensure the
callback verifies the captured generation/trace state before closing or deleting
anything, and add a regression test for late `llm_output` followed immediately by
`llm_input`.
There was a problem hiding this comment.
Fixed in b2e3f90: a late llm_output now finalizes synchronously, and llm_input finalizes any trace whose agent_end already arrived before starting the new turn. Regression test added.
| alias: [ | ||
| { | ||
| find: /^openclaw\/plugin-sdk(\/.*)?$/, | ||
| replacement: fileURLToPath( | ||
| new URL("./.scripts/vitest-openclaw-plugin-sdk.mjs", import.meta.url), | ||
| ), | ||
| }, |
There was a problem hiding this comment.
Tests mask SDK import regressions
The alias maps openclaw/plugin-sdk and every subpath to one local stub, so Vitest accepts unsupported imports that later fail in the published plugin with ERR_PACKAGE_PATH_NOT_EXPORTED — should we limit it to the subpaths the tests need or add a resolution test against the real package exports?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In `vitest.config.mjs`
around lines 7-13, the `resolve.alias` configuration maps every `openclaw/plugin-sdk`
subpath to the same local stub, hiding unsupported production imports that would fail
when published. Narrow the alias to only the exact SDK subpaths required by the tests,
and add or update a resolution test that checks production imports against the installed
OpenClaw package exports so `ERR_PACKAGE_PATH_NOT_EXPORTED` regressions remain
detectable.
There was a problem hiding this comment.
Fair point. The unit tests can't see export maps; the load path is covered by the E2E job, which installs the packed tarball into the latest OpenClaw (it fails with ERR_PACKAGE_PATH_NOT_EXPORTED on the published 0.2.17 and passes here). Leaving the alias as is.
There was a problem hiding this comment.
Thanks for the clarification—I'll remember that the E2E packed-tarball job covers export-map compatibility here once the PR is merged.
| }, | ||
| "peerDependencies": { | ||
| "openclaw": ">=2026.3.2" | ||
| "openclaw": ">=2026.9.7" |
There was a problem hiding this comment.
Compatibility fix reuses published version
Keeping the package at 0.2.17 lets the v0.2.17 tag pass but makes the release workflow treat the npm version as already published, so the new OpenClaw compatibility floor is not delivered — should we bump the package version, since the README reserves 0.2.17 for older hosts?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In `package.json`
around lines 33-33, update the package `version` from `0.2.17` to the next release
version appropriate for this compatibility-floor change. Ensure the new version aligns
with the release workflow and preserves `0.2.17` as the documented version for older
OpenClaw hosts.
There was a problem hiding this comment.
Version bumps go in a separate chore(release) commit in this repo, so the bump comes with the release, not this PR.
There was a problem hiding this comment.
Got it—version bumps are handled in the separate chore(release) commit here. I’ll save this to memory once the PR is merged.
| "pluginApi": ">=2026.9.7", | ||
| "minGatewayVersion": "2026.9.7" |
There was a problem hiding this comment.
Advertised Node floor contradicts host requirement
The new OpenClaw 2026.9.7 metadata still advertises Node >=22.12.0 and Node-22 CI/release gates even though the branch requires Node >=24.16, so the published and validated runtime contracts diverge — should we update the package/lockfile engine and validation/publish workflows together?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In package.json around
lines 77-78, update the package and lockfile engine requirements from Node >=22.12.0 to
the documented Node >=24.16 floor, keeping the OpenClaw compatibility metadata
consistent. Also update all validation and publish/release workflows and their Node
setup or version gates to use and enforce Node 24.16, so the advertised runtime contract
matches what CI and releases actually validate.
There was a problem hiding this comment.
Fixed in b2e3f90: engines and the CI, release and ClawHub publish workflows now use Node 24.16.
Address review: - service stop during the grace window finalizes a turn that already reached agent_end, so its output and metadata are written instead of dropped; - agent_end refreshes lastActivityAt, so stale cleanup can't take a trace that is only waiting for llm_output; - a late llm_output finalizes synchronously, and llm_input finalizes any trace whose agent_end already arrived, so a next turn in the same tick gets its own trace instead of being closed with the previous one; - engines and the CI, release and ClawHub publish workflows move to Node 24.16, matching what OpenClaw 2026.9 requires. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| finalizeEndedTurn: (sessionKey) => { | ||
| const active = activeTraces.get(sessionKey); | ||
| if (active?.pendingFinalize || active?.agentEnd) finalizeTrace(sessionKey); |
There was a problem hiding this comment.
Preserve correlation across trace rollover
The new active.agentEnd branch erases the mapping that llm_input just recorded, so context-poor llm_output and after_tool_call hooks for the replacement turn are dropped or misrouted when another trace is active — should we finalize the ended trace before recording the new correlation, or re-register it afterward?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In src/service.ts
around lines 575-577, update the `finalizeEndedTurn` callback in `registerHooks` so
finalizing an ended trace does not erase the replacement turn’s session correlation.
Finalize the ended trace before recording the new turn’s correlation, or explicitly
re-register the correlation after `finalizeTrace(sessionKey)`, ensuring context-poor
`llm_output` and `after_tool_call` hooks are still routed to the replacement trace.
There was a problem hiding this comment.
Fixed in 18a9024: llm_input now finalizes the ended turn before recording the new turn's correlation. Regression test added.
There was a problem hiding this comment.
Commit 18a9024 addressed this comment by llm_input now finalizes an ended turn before recording the replacement turn’s session correlation. A regression test verifies that context-poor llm_output hooks reach the new trace.
Finalizing the previous trace forgot the session's agentId mapping that llm_input had just recorded, so the new turn's context-poor hooks could not find it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
awkoy
changed the title
On OpenClaw 2026.9 a failed provider call (for example an HTTP 401 from a bad API key) comes back as agent_end success=true with no error; the failure is only in llm_output's lastAssistant (stopReason "error", errorMessage). Such turns showed up in Opik as successful. The LLM span and the trace now get errorInfo with the provider message, and the trace is marked success=false. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| const routed: HookHandler = (...args) => { | ||
| const router = getHookRouter(); | ||
| const ownerHandler = | ||
| router.owner && router.owner !== registrationId | ||
| ? router.handlers.get(hookName) | ||
| : undefined; | ||
| return (ownerHandler ?? handler)(...args); |
There was a problem hiding this comment.
❓ question | Architecture
The router assumes each hook event reaches exactly one copy of the plugin. Every copy forwards to the copy whose service started, and nothing checks for repeats. So if the host ever sends the same event to two copies, the started copy handles it twice. That would happen if the host sends to both the gateway's copy and the per-turn copy, or if copies from earlier turns stay registered. The result would be duplicate LLM/tool spans, or a second trace.
Your two-turn live run suggests this doesn't happen on 2026.9.7. Is "one copy per event" a guarantee from OpenClaw, or just how it behaves today? If it's only today's behaviour, a cheap guard would be a WeakSet per hook name in the router. Skip an event object the started copy has already handled. Or at least document the assumption next to the comment above HOOK_ROUTER_KEY.
🤖 Review posted via /review-github-pr
There was a problem hiding this comment.
Good question. It comes from how the host is built, not from a documented contract. In 2026.9.7 the hook runner merges registries per pluginId + hookName (overlayHookRegistries in hook-runner-global). The per-turn registration's hook replaces the gateway's hook for the same plugin instead of running next to it. A generation registry is used on its own, and retired registries are skipped.
I also checked it live: OpenClaw 2026.9.7, Gemini 2.5 Flash and a stdio MCP server, with a debug log on every routed dispatch. That was 40 dispatches over 5 turns, with built-in read calls, MCP echo-mcp__mcp_echo calls and a config hot reload in between. Every event went through exactly one registration. No event object and no (hook, runId, toolCallId) key repeated. After the reload the old registration got nothing. Opik had one trace per turn and no duplicate spans.
So I didn't add the WeakSet, since it would guard against something the host rules out. In e3f0c4e I documented the assumption and where it comes from above HOOK_ROUTER_KEY. In the same commit, registerHooks now throws if a hook name is registered twice on one service. Routing looks handlers up by name, so a second handler would otherwise quietly replace the first for routed events.
There was a problem hiding this comment.
Commit e3f0c4e addressed this comment by The change documents the one-registration-per-event assumption and explains the OpenClaw behavior supporting it, including the duplicate-span risk if it changes. It does not add a WeakSet deduplication guard, but documentation was explicitly offered as an alternative.
| if (router.owner === registrationId) { | ||
| router.owner = null; | ||
| router.handlers = new Map(); |
There was a problem hiding this comment.
💡 suggestion | Testing
The router.owner === registrationId check covers a config reload where the new service starts before the old one stops. The new tests only cover a single start/stop. Please add a test for start A → start B → stop A, checking that hooks from a third copy still reach B. This check is the only thing that keeps telemetry flowing after that reload, and it would be easy to "simplify" away later.
🤖 Review posted via /review-github-pr
There was a problem hiding this comment.
Added in e3f0c4e. The test does start old → start new → stop old, then checks that a hook from a third registration still creates a trace. Each service is loaded from a fresh module instance (vi.resetModules()), because that's what the host does on reload. I confirmed it live: the module instance changes across a config hot reload. The test fails if the owner check in stop() is removed.
For context, 2026.9.7 stops the old service before starting the new one on hot reload, so this order doesn't happen today. The test is there so the check doesn't get simplified away.
There was a problem hiding this comment.
Commit e3f0c4e addressed this comment by A regression test now covers starting the old service, starting the new service, then stopping the old one. It also registers a third module instance and verifies its hook reaches the new service.
|
👋 Review summary What looks good
Overall Inline comments: 1 question, 1 suggestion. Nothing blocking. 🤖 Review posted via /review-github-pr |
… reloads in tests - Note above HOOK_ROUTER_KEY that the router relies on OpenClaw delivering each hook event to exactly one registration of a plugin, and why that holds on 2026.9.7. - Throw when a hook name is registered twice on one service: routing looks handlers up by name, so the second handler would replace the first for routed events. - Test a reload that starts the new service before stopping the old one, with each service in its own module instance, so the owner check in stop() can't be dropped unnoticed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
JetoPistola
left a comment
JetoPistola
left a comment
There was a problem hiding this comment.
All review feedback is addressed, and CI and E2E pass on e3f0c4e. LGTM.
2 participants
Details
The published plugin (0.2.17) exports nothing on OpenClaw 2026.9.7, the current release. I ran it against a real gateway, reproduced each problem below, fixed it, and checked the fix the same way.
openclaw/plugin-sdkmodule in its July SDK cleanup, so loading fails withERR_PACKAGE_PATH_NOT_EXPORTED. The two runtime imports now come fromopenclaw/plugin-sdk/coreandopenclaw/plugin-sdk/diagnostic-runtime.client=nulland dropped its data. A process-wide router (globalThis[Symbol.for("opik-openclaw.hook-router")]) now sends hooks to the copy whose service started, and lets go onstop().llm_outputnow arrives afteragent_end, in a later tick, after the trace was already closed. If the LLM span is still open atagent_end, the trace now waits forllm_output, or for one second if it never comes. A new turn in the same session during that wait closes the previous trace first.subagent_spawninghook, so the plugin no longer registers it.subagent_spawnedcreates the span and now records the label, requester andthreadRequested.loadConfigandwriteConfigFile.openclaw opik statusandopenclaw opik configureuseconfig.current()andmutateConfigFile, and write back only the Opik plugin entry.agent_endstill sayssuccess: truewith no error. The failure is only inllm_output's last assistant message (stopReason: "error"). The LLM span and the trace now geterrorInfowith the provider's message, and the trace is markedsuccess: false.googleorgoogle-vertex, which Opik's price table doesn't know. They now map togoogle_aiandgoogle_vertexai.compat.pluginApi,minGatewayVersion). Older hosts stay on 0.2.17. The README says so.mainsince 2026-09-28. It also passes--force --accept-capabilitiestoplugins install, which the new host asks for when installing a local tarball.This replaces #146. On 2026.9.7 OpenClaw already passes the full context to bundle-MCP tools, so MCP tool spans show up without that change once the plugin loads and its hooks have a client.
One thing this doesn't change:
llm_outputupdates whichever LLM span is open. If call N'sllm_outputarrived after call N+1'sllm_input, it would land on N+1. On 2026.9.7 both hooks fire once per turn, so I didn't see it happen.Change checklist
Issues
Testing
npm test: 115 passed, 1 skipped (the same skip as onmain).npm run typecheck,npm run buildandnpm run pack:checkare clean.llm_outputstill lands on the span and the trace; the one-second fallback; a new turn during the wait gets its own trace; a provider error reported only inllm_output; the Gemini provider mapping;createConfigDepswrites back only the Opik entry.npm pack, OpenClaw 2026.9.7, Node 24). It passes with this branch and fails with the published 0.2.17, which exports 0 traces.ERR_PACKAGE_PATH_NOT_EXPORTED)google_ai, cost setclaude-haiku-4-5: trace with output, tokens, costread)echo-mcp__mcp_echo)errorInfo(key masked)openclaw opik statusDocumentation
The README prerequisites now say OpenClaw
>=2026.9.7and Node>=24.16, and the hooks table no longer listssubagent_spawning.🤖 Generated with Claude Code