Skip to content

ISI-515: Fix hook registration timing — register hooks in register() not start()#6

Merged
henrikrexed merged 2 commits intomainfrom
fix/hook-registration-timing
Apr 16, 2026
Merged

ISI-515: Fix hook registration timing — register hooks in register() not start()#6
henrikrexed merged 2 commits intomainfrom
fix/hook-registration-timing

Conversation

@henrikrexed
Copy link
Copy Markdown
Owner

Summary

All 15 plugin hooks were registered but never fired. Root cause: registerHooks() was called from inside registerService.start() (async, runs ~30s after the gateway is ready), while OpenClaw snapshots typed hooks at plugin registration time.

Fix

  • Move registerHooks(api, getTelemetry, config) into the synchronous register() phase in index.ts so the gateway sees the listeners before it boots.
  • Change registerHooks in src/hooks.ts to take a lazy getTelemetry: () => TelemetryRuntime | null instead of the runtime directly, since telemetry is still initialised in start().
  • Each hook handler reads the live runtime when it fires and is a no-op until telemetry is ready (handles the brief window between register() and start()).

Files

  • index.ts — register hooks in register(), drop the call from start()
  • src/hooks.ts — accept getTelemetry lazy getter, look up tracer/counters/histograms per call, build securityCounters per call

Verification

  • tsc --noEmit passes locally.

Acceptance (per ISI-515)

  • Hooks fire during message processing — look for [otel] Root span started for session=..., [otel] Agent turn span started, [otel] Trace completed for session=... in logs.
  • Connected spans visible in Dynatrace: openclaw.requestopenclaw.agent.turntool.*.

Paperclip ticket: ISI-515

PerformanceChef and others added 2 commits April 16, 2026 14:24
OpenClaw snapshots typed hooks at plugin registration time. The plugin
was calling registerHooks() inside the registerService start() callback,
which runs ~30s after the gateway is ready, so all 15 hooks (typed via
api.on plus event-stream via api.registerHook) were attached too late
and never fired.

Move the registerHooks() call into register() so the listeners are in
place before the gateway starts processing events. Telemetry init still
happens in start(), so registerHooks() now takes a lazy
getTelemetry: () => TelemetryRuntime | null. Each hook reads the live
runtime when it fires and no-ops if telemetry is not yet initialised
(handles the brief window between register() and start()).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
registerHooks now returns a disposer that clears the 60s setInterval
stale-session sweeper. index.ts captures it and invokes it from
service.stop() so the timer is released on plugin shutdown/reload
instead of leaking across lifecycles.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@henrikrexed henrikrexed merged commit d4b7554 into main Apr 16, 2026
1 check failed
henrikrexed pushed a commit that referenced this pull request Apr 16, 2026
Phase 1 doc refresh following PR #6 (ISI-515) and commit b668a4f (ISI-522).

- README: add Plugin Lifecycle section (register/start/stop phases, lazy
  telemetry getter, setInterval cleanup on stop). Add "Validate your first
  trace" walk-through of the [otel] log markers in registration order.
  Add Troubleshooting section covering the "hooks register but never fire"
  symptom with confirmation steps.
- docs/architecture.md: new Plugin Lifecycle section with phase table and
  lazy-getter code sample, explaining why typed hooks must register in
  register() and how hooks resolve telemetry after start().
- docs/getting-started.md: expand "Verify Connected Traces" with the six
  registration log lines and the per-message debug markers. Add a dedicated
  "Hooks register but never fire" troubleshooting entry linking to the
  architecture lifecycle section.

Phase 2 (per-span/attribute/metric semconv reference table) tracked on
ISI-520 and will ship in a follow-up PR. mkdocs build --strict passes.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants