Skip to content

feat(openclaw): drop redundant before_agent_start + add debugPerfTiming - #1477

Merged
nicoloboschi merged 2 commits into
mainfrom
feat/openclaw-perf-timing-and-cleanup
May 6, 2026
Merged

feat(openclaw): drop redundant before_agent_start + add debugPerfTiming#1477
nicoloboschi merged 2 commits into
mainfrom
feat/openclaw-perf-timing-and-cleanup

Conversation

@nicoloboschi

Copy link
Copy Markdown
Collaborator

Summary

Two small, unrelated openclaw plugin changes bundled in one PR.

#1354 — drop before_agent_start registration

The hook's body did nothing observable: a resolveAndCacheIdentity() call and a debug log. The same identity resolution already happens in:

  • before_dispatch — earlier in the inbound channel path, populates the cache.
  • before_prompt_build — re-resolves before recall, can also infer senderId from the raw prompt when ctx lacks it.
  • agent_end — re-resolves before retain.

Subscribing was duplicate work on the hot path. Removed the registration and the \"agent_start\" member of the logSkipOnce operation union.

Also fixed a stale comment in `stripMemoryTags` doc that referenced `before_agent_start` — the actual injection happens in `before_prompt_build`.

#1406 — opt-in `debugPerfTiming` flag

Adds `debugPerfTiming?: boolean` plugin config (default `false`). When on, the plugin emits one info-level perf line per recall path and per retain path:

```
perf: before_prompt_build hook_total=4200ms recall_main=3800ms source=fresh results=3
perf: agent_end hook_total=1200ms retain=1100ms outcome=ok bank=main messages=4
```

This lets users diagnose latency without patching the compiled dist (which is what the issue reporter resorted to). Specifics:

  • Info-level, not debug-level, despite the flag name. Perf data is small-volume / high-signal — gating it behind `debug: true` would silently produce no output for users who only want timing. Documented.
  • Only emitted on the recall/retain RPC paths, not on the many fast skip-filter early returns. Those are sub-millisecond and not interesting.
  • `source=fresh|reused` reflects in-flight recall dedup at the hash key.
  • `outcome=ok|queued|error` reflects whether retain succeeded inline, was queued for retry, or failed outright (queue-less mode).
  • The pure formatter `formatHookPerf()` is exported and unit-tested for the rendering contract.

Test plan

  • `npm test` — 212 tests pass (5 new for `formatHookPerf` + `debugPerfTiming` parsing).
  • `npm run build` — tsc clean.
  • `./scripts/hooks/lint.sh` — clean.
  • Manually enable `debugPerfTiming: true` in a real openclaw setup, verify perf lines appear with sensible values for both hooks.

Out of scope

Closes #1354
Closes #1406

…fTiming

Two unrelated-but-tiny openclaw improvements:

- #1354: Stop registering `before_agent_start`. Its body only called
  `resolveAndCacheIdentity()` + emitted a debug log. The same identity
  resolution already happens in `before_dispatch` (earlier in the
  inbound path), `before_prompt_build` (re-resolves before recall, can
  infer senderId from prompt content), and `agent_end` (re-resolves
  before retain). Subscribing here was duplicate work on the hot path.

- #1406: Add `debugPerfTiming?: boolean` plugin config flag (default
  false). When enabled, the plugin emits one info-level perf line per
  recall path and per retain path:

    perf: before_prompt_build hook_total=4200ms recall_main=3800ms source=fresh results=3
    perf: agent_end hook_total=1200ms retain=1100ms outcome=ok bank=main messages=4

  Lets users diagnose latency without patching the dist. The
  `source=fresh|reused` field reflects in-flight recall dedup; the
  `outcome=ok|queued|error` field reflects whether retain succeeded
  inline, was queued for retry, or failed outright.

Also fixes a stale comment that referenced before_agent_start where the
actual lifecycle stage is before_prompt_build.
OpenClaw's plugin loader runs configSchema validation with
`additionalProperties: false`, so any PluginConfig field not declared
in openclaw.plugin.json is silently rejected at config-set time. The
manifest had drifted from the type:

- retainMission, observationsMission (added in #1473) — never declared
- debugPerfTiming (added earlier in this PR) — never declared
- retainDocumentScope — pre-existing gap, declared now
- enableKnowledgeTools — was in configSchema but missing from uiHints

All five are now in both configSchema.properties and uiHints. Also
fixed the bankMission description to match the corrected README from
#1353 (only affects /reflect, not retain).

Added a manifest.test.ts parity test that compares the type's keys to
the manifest's declared keys and fails on either side of drift. This
is the same class of bug as #1443 (whitelist drift) — having a test
prevents the next round.
@nicoloboschi
nicoloboschi merged commit d37940a into main May 6, 2026
54 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant