Skip to content

Fix plugin-only tool and registry latency regressions#75922

Merged
obviyus merged 3 commits intomainfrom
test/latency-cluster-regressions
May 2, 2026
Merged

Fix plugin-only tool and registry latency regressions#75922
obviyus merged 3 commits intomainfrom
test/latency-cluster-regressions

Conversation

@obviyus
Copy link
Copy Markdown
Contributor

@obviyus obviyus commented May 2, 2026

Summary

  • Skip core coding tool construction when an explicit allowlist only requests plugin tools.
  • Keep the full workspace plugin registry cache separate from scoped plugin registry loads.
  • Add regressions for both latency paths.

Tests

  • OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test src/agents/pi-embedded-runner/run/attempt.tools-allow-regression.test.ts src/agents/pi-embedded-runner/run/attempt.test.ts src/agents/tool-policy.plugin-only-allowlist.test.ts src/agents/pi-tools.create-openclaw-coding-tools.test.ts src/plugins/plugin-lru-cache.test.ts src/plugins/loader.runtime-registry.test.ts src/plugins/loader.test.ts
  • pnpm exec oxfmt --check --threads=1 src/agents/pi-embedded-runner/run/attempt.ts src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts src/agents/pi-embedded-runner/run/attempt.tools-allow-regression.test.ts src/plugins/loader.ts src/plugins/loader.runtime-registry.test.ts
  • git diff --check origin/main...HEAD

Fixes #75882
Fixes #75907
Fixes #75906
Fixes #75887
Fixes #75851

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels May 2, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 2, 2026

Codex review: needs changes before merge.

Summary
The PR skips core coding-tool construction for plugin-only allowlists, splits full-workspace plugin registry caching from scoped loads, and adds regression tests plus a changelog entry.

Reproducibility: yes. On exact PR head, toolsAllow: ["process"] or toolsAllow: ["heartbeat_respond"] makes includeCoreTools false because those normalized names are missing from the new classifier, and the final allow filter cannot recover tools that were never built.

Next step before merge
Automerge is opted in and the remaining blocker is a narrow repair to restore canonical core-tool classification and plugin/core collision context on the PR branch.

Security
Needs attention: The diff can lose built-in tool identity and core/plugin collision protection on the plugin-only fast path.

Review findings

  • [P2] Derive core allow detection from canonical tools — src/agents/pi-embedded-runner/run/attempt.ts:499-529
Review details

Best possible solution:

Keep the registry-cache split, but derive fast-path core detection and plugin conflict names from the canonical built-in tool inventory before automerge.

Do we have a high-confidence way to reproduce the issue?

Yes. On exact PR head, toolsAllow: ["process"] or toolsAllow: ["heartbeat_respond"] makes includeCoreTools false because those normalized names are missing from the new classifier, and the final allow filter cannot recover tools that were never built.

Is this the best way to solve the issue?

No. The latency optimization direction is sound, but the hardcoded classifier and empty conflict context are less maintainable than reusing a canonical core-tool inventory or shared helper.

Full review comments:

  • [P2] Derive core allow detection from canonical tools — src/agents/pi-embedded-runner/run/attempt.ts:499-529
    The new classifier omits current built-ins: process is created by createOpenClawCodingTools, and heartbeat's real name is heartbeat_respond, not heartbeat_response. With either name in toolsAllow, this sets includeCoreTools: false, so the requested built-in is absent and plugin resolution runs without the expected core-name conflict context.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.92

Security concerns:

  • [medium] Preserve core tool identity on plugin-only path — src/agents/pi-tools.ts:594
    Because the new branch decides whether to build core tools from an incomplete hardcoded set and resolves plugin tools without full core names, an allowlist for an omitted built-in such as process can lose the built-in tool or be satisfied by plugin resolution instead.
    Confidence: 0.82

Acceptance criteria:

  • OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test src/agents/pi-embedded-runner/run/attempt.tools-allow-regression.test.ts src/agents/pi-embedded-runner/run/attempt.test.ts src/agents/tool-policy.plugin-only-allowlist.test.ts src/agents/pi-tools.create-openclaw-coding-tools.test.ts src/plugins/plugin-lru-cache.test.ts src/plugins/loader.runtime-registry.test.ts src/plugins/loader.test.ts
  • pnpm exec oxfmt --check --threads=1 src/agents/pi-embedded-runner/run/attempt.ts src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts src/agents/pi-embedded-runner/run/attempt.tools-allow-regression.test.ts src/agents/pi-tools.ts src/plugins/loader.ts src/plugins/loader.runtime-registry.test.ts CHANGELOG.md
  • git diff --check origin/main...HEAD
  • pnpm check:changed in Testbox before handoff

What I checked:

  • Exact-head classifier omits current core names: The PR adds a hardcoded core allowlist containing heartbeat_response but not the current process or heartbeat_respond names, then uses it to decide includeCoreTools. (src/agents/pi-embedded-runner/run/attempt.ts:499, a2f321ef5584)
  • Current built-in process tool exists: Current main constructs a built-in tool named process, so toolsAllow: ["process"] still needs core tools built before the final allow filter runs. (src/agents/pi-tools.ts:128, 4d801fadab61)
  • Current heartbeat tool name differs from the PR classifier: The canonical heartbeat response tool constant is heartbeat_respond, not heartbeat_response. (src/auto-reply/heartbeat-tool-response.ts:4, 4d801fadab61)
  • Current plugin resolution preserves core conflict context: Current main passes the built core OpenClaw tool names into plugin tool resolution through existingToolNames. (src/agents/openclaw-tools.ts:349, 4d801fadab61)
  • Plugin resolver enforces existing-name conflicts: Plugin tool resolution normalizes supplied existing tool names and rejects plugin ids or tools that conflict with them; the PR's plugin-only branch does not supply the full core-name set. (src/plugins/tools.ts:246, 4d801fadab61)
  • Registry-cache change direction is covered: The PR adds a separate full-workspace plugin registry cache and a regression asserting scoped cron loads do not evict the full registry entry. (src/plugins/loader.ts:249, a2f321ef5584)

Likely related people:

  • steipete: Recent history shows Peter Steinberger maintaining the plugin cache and tool-routing surfaces, including the plugin cache primitive refactor and plugin tool delivery-context refactor. (role: recent maintainer and adjacent owner; confidence: high; commits: f43a184103b6, 6ba0c434ba1a; files: src/plugins/loader.ts, src/agents/openclaw-tools.ts, src/agents/pi-tools.ts)
  • gumadeiras: Gustavo Madeira Santana added the runtime registry compatibility helper and reviewed adjacent tool-loading workspace reuse work, both central to the cache and plugin-tool paths touched here. (role: plugin registry/runtime owner; confidence: medium; commits: fd0aac297c96, b98a6c223dc2; files: src/plugins/loader.ts, src/agents/openclaw-tools.ts)
  • neeravmakwana: Neerav Makwana authored the gateway session-workspace reuse change in openclaw-tools, an adjacent path for plugin tool loading and workspace-scoped runtime resolution. (role: adjacent tool-loading contributor; confidence: medium; commits: b98a6c223dc2; files: src/agents/openclaw-tools.ts)
  • Tak Hoffman: Tak Hoffman authored a recent plugin registry reuse regression fix that scoped cache reuse by gateway methods, adjacent to the registry-cache split in this PR. (role: plugin registry cache contributor; confidence: medium; commits: c0d4c07b888f; files: src/plugins/loader.ts)

Remaining risk / open question:

  • The exact PR head can omit requested built-ins such as process and heartbeat_respond because core tools are never constructed before the final allow filter.
  • The plugin-only branch resolves plugin tools without the full set of core tool names, so plugin/core name collision protection is weaker on the optimized path.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4d801fadab61.

@obviyus obviyus force-pushed the test/latency-cluster-regressions branch 3 times, most recently from 55fcd34 to 8fecaf0 Compare May 2, 2026 03:25
@obviyus obviyus force-pushed the test/latency-cluster-regressions branch from 8fecaf0 to 769b818 Compare May 2, 2026 03:36
@obviyus
Copy link
Copy Markdown
Contributor Author

obviyus commented May 2, 2026

@clawsweeper automerge

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 2, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 2, 2026

ClawSweeper 🐠 automerge status

ClawSweeper took another look; no safe branch change was available on this pass.

Executor outcome: Codex /review did not pass after 2 attempt(s): Not merge-ready. Security-sensitive issues appear absent, changelog credit follows repo policy, and pnpm check:changed plus diff checks passed. Merge is blocked by a current origin/main conflict, and the embe....
Worker summary: this PR is already merged and closed, so the adopted-PR branch repair loop cannot continue on that PR. Latest main still has the narrow ClawSweeper finding: the plugin-only fast-path classifier uses a hardcoded core-tool set that omits process and misspells heartbeat_respond as heartbeat_response, so core allowlists can incorrectly skip core tool construction. Plan a credited follow-up fix PR; do not close or merge anything from this job.

Worker actions:

  • keep_closed on this PR: skipped - Already merged/closed; retain as the canonical source PR and credit source for the follow-up fix.
  • keep_closed on #75851: skipped - Historical linked issue only; no mutation is valid for an already-closed issue.
  • keep_closed on #75882: skipped - Historical linked issue only; no mutation is valid for an already-closed issue.
  • keep_closed on #75887: skipped - Historical linked issue only; no mutation is valid for an already-closed issue.
  • keep_closed on #75906: skipped - Historical linked issue only; no mutation is valid for an already-closed issue.
  • keep_closed on #75907: skipped - Historical linked issue only; no mutation is valid for an already-closed issue.

ClawSweeper left the PR as-is: no push, no rebase, no replacement PR, no merge, and no fresh ClawSweeper pass.

fish notes: model gpt-5.5, reasoning high.

Automerge progress:

  • 2026-05-02 04:26:34 UTC review requested repair [`185bba08fd03`](https://github.com/openclaw/openclaw/commit/185bba08fd03eec6f03e604121513df05c607bf8) (structured ClawSweeper marker: fix-required (finding=security-review sha=185bba...)
  • 2026-05-02 04:26:41 UTC repair queued [`185bba08fd03`](https://github.com/openclaw/openclaw/commit/185bba08fd03eec6f03e604121513df05c607bf8) (autonomous) Run: https://github.com/openclaw/clawsweeper/actions/runs/25243720448
  • 2026-05-02 04:28:47 UTC repair completed [`0e0b2979f9a2`](https://github.com/openclaw/openclaw/commit/0e0b2979f9a2616d528be5e86fa4aca7349c34d8) (deterministic rebase) in 1m 28s Run: https://github.com/openclaw/clawsweeper/actions/runs/25243625051 clean deterministic rebase
  • 2026-05-02 04:28:46 UTC review queued [`0e0b2979f9a2`](https://github.com/openclaw/openclaw/commit/0e0b2979f9a2616d528be5e86fa4aca7349c34d8) (after repair)
  • 2026-05-02 04:32:25 UTC review requested repair [`0e0b2979f9a2`](https://github.com/openclaw/openclaw/commit/0e0b2979f9a2616d528be5e86fa4aca7349c34d8) (structured ClawSweeper marker: fix-required (finding=security-review sha=0e0b29...)
  • 2026-05-02 04:32:35 UTC repair queued [`0e0b2979f9a2`](https://github.com/openclaw/openclaw/commit/0e0b2979f9a2616d528be5e86fa4aca7349c34d8) (autonomous) Run: https://github.com/openclaw/clawsweeper/actions/runs/25243822290
  • 2026-05-02 04:34:44 UTC repair completed [`4b94d4dfa7b1`](https://github.com/openclaw/openclaw/commit/4b94d4dfa7b111cec016ea1255301d5be205a210) (deterministic rebase) in 1m 30s Run: https://github.com/openclaw/clawsweeper/actions/runs/25243720448 clean deterministic rebase
  • 2026-05-02 04:34:43 UTC review queued [`4b94d4dfa7b1`](https://github.com/openclaw/openclaw/commit/4b94d4dfa7b111cec016ea1255301d5be205a210) (after repair)
  • 2026-05-02 04:38:53 UTC review requested repair [`4b94d4dfa7b1`](https://github.com/openclaw/openclaw/commit/4b94d4dfa7b111cec016ea1255301d5be205a210) (structured ClawSweeper marker: fix-required (finding=security-review sha=4b94d4...)
  • 2026-05-02 04:39:03 UTC repair queued [`4b94d4dfa7b1`](https://github.com/openclaw/openclaw/commit/4b94d4dfa7b111cec016ea1255301d5be205a210) (autonomous) Run: https://github.com/openclaw/clawsweeper/actions/runs/25243927199
  • 2026-05-02 04:40:45 UTC repair completed [`a2f321ef5584`](https://github.com/openclaw/openclaw/commit/a2f321ef5584855ceabaa422ea034ee4b0b597b5) (deterministic rebase) in 1m 30s Run: https://github.com/openclaw/clawsweeper/actions/runs/25243822290 clean deterministic rebase
  • 2026-05-02 04:40:44 UTC review queued [`a2f321ef5584`](https://github.com/openclaw/openclaw/commit/a2f321ef5584855ceabaa422ea034ee4b0b597b5) (after repair)
  • 2026-05-02 04:44:19 UTC review requested repair [`a2f321ef5584`](https://github.com/openclaw/openclaw/commit/a2f321ef5584855ceabaa422ea034ee4b0b597b5) (structured ClawSweeper marker: fix-required (finding=security-review sha=a2f321...)
  • 2026-05-02 04:44:29 UTC repair queued [`a2f321ef5584`](https://github.com/openclaw/openclaw/commit/a2f321ef5584855ceabaa422ea034ee4b0b597b5) (autonomous) Run: https://github.com/openclaw/clawsweeper/actions/runs/25244017445
  • 2026-05-02 05:15:17 UTC repair completed (no branch change) in 21m 57s Run: https://github.com/openclaw/clawsweeper/actions/runs/25243927199 Codex /review did not pass after 2 attempt(s): Blocked. `pnpm check:changed` and `git diff --check origin/main...HEAD` passed, and the explicit `process` / `he...
  • 2026-05-02 05:32:34 UTC repair completed (no branch change) in 13m 55s Run: https://github.com/openclaw/clawsweeper/actions/runs/25244017445 Codex /review did not pass after 2 attempt(s): Not merge-ready. Security-sensitive issues appear absent, changelog credit follows repo policy, and `pnpm check:...

@clawsweeper clawsweeper Bot force-pushed the test/latency-cluster-regressions branch 5 times, most recently from 185bba0 to 0e0b297 Compare May 2, 2026 04:28
@obviyus obviyus self-assigned this May 2, 2026
@clawsweeper clawsweeper Bot force-pushed the test/latency-cluster-regressions branch from 837abc7 to 4b94d4d Compare May 2, 2026 04:34
@clawsweeper clawsweeper Bot force-pushed the test/latency-cluster-regressions branch from 4b94d4d to a2f321e Compare May 2, 2026 04:40
@obviyus
Copy link
Copy Markdown
Contributor Author

obviyus commented May 2, 2026

@clawsweeper stop

@clawsweeper clawsweeper Bot added the clawsweeper:human-review Needs maintainer review before ClawSweeper can continue label May 2, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 2, 2026

🦞🦞
Got it. ClawSweeper will leave this item for human review.

I added clawsweeper:human-review and paused the automation trail until a maintainer asks again.

@obviyus obviyus force-pushed the test/latency-cluster-regressions branch from a2f321e to 409a4e5 Compare May 2, 2026 04:47
@obviyus obviyus merged commit 0d31ab6 into main May 2, 2026
94 checks passed
@obviyus obviyus deleted the test/latency-cluster-regressions branch May 2, 2026 04:48
@obviyus
Copy link
Copy Markdown
Contributor Author

obviyus commented May 2, 2026

Landed on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment