Skip to content

[codex] Reuse scoped runtime plugin registries#96231

Closed
omarshahine wants to merge 1 commit into
mainfrom
omarshahine/runtime-reuse-small
Closed

[codex] Reuse scoped runtime plugin registries#96231
omarshahine wants to merge 1 commit into
mainfrom
omarshahine/runtime-reuse-small

Conversation

@omarshahine

@omarshahine omarshahine commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

The iMessage/source-reply latency investigation showed repeated runtime plugin registry work on hot paths after gateway startup. The broad investigation branch proved the direction, but it bundled tracing, iMessage monitor changes, Codex/source-reply fixes, provider-hook reuse, and tool lookup reuse.

This PR keeps only the smallest safe runtime-registry reuse piece: startup-scoped runtime plugin loads can reuse the active gateway startup registry when the existing cache-key and plugin-scope compatibility checks prove it is the same effective registry.

Fixes part of #96148.

Why This Change Was Made

The gateway startup registry already records which plugin ids were loaded for startup-scoped runtime work. Agent turns can request that same startup-scoped set from a turn workspace. Previously, those calls could miss the active gateway registry and rebuild scoped runtime plugin state.

This change tracks the active registry plugin-id scope and lets compatible subset loads reuse the active startup registry. The cross-workspace path is reuse-only: it first checks for a compatible loaded registry and falls back to loading from the caller's original workspace on a miss.

Provider-hook and plugin-tool cross-workspace reuse were intentionally left out of this PR after review because they need stronger compatibility contracts.

User Impact

For users with many configured plugins, agent turns should avoid some redundant startup-scoped runtime plugin reloads after gateway startup. This is a narrow performance improvement, not the full broad-branch optimization.

The live investigation evidence is on #96148 and the superseded draft #96199. The clean broad-branch matched test showed 20.797s -> 17.529s, but this PR claims only the smallest safe subset from that work.

Focused live proof on this PR shows the remaining iMessage latency is mostly outside this small change. With cron disabled on commit a397a64, startup prewarm loaded 149 runtime plugins in 3980.8ms (4022ms gateway phase) and provider auth prewarm took 2679ms. A live Omar-to-Lobster iMessage turn then took 20504ms end-to-end: 20391ms dispatch, 13583ms model/main lane. The same turn still loaded provider/runtime plugin sets on the hot path: 1 plugin in 7.0ms, 9 plugins in 1268.6ms, 51 plugins in 2281.7ms, and agent tools 22 plugins in 120.4ms.

That run also hit incomplete turn detected, but this branch does not include the Codex message-tool/source-reply terminal fix from #95942. Treat the incomplete-turn evidence here only as proof that this perf branch was missing that correctness fix, not as evidence against #95942.

Normal Lobster steady-state follow-up also showed the separate provider catalog-hook cache idea was not a real iMessage hot-path win: model catalog work converged after startup and augmentModelCatalogWithProviderPlugins() did not repeat per message. That branch has been shelved separately and is not part of this PR.

Evidence

  • node scripts/run-vitest.mjs src/agents/runtime-plugins.test.ts src/agents/runtime-plugins.registry-reuse.test.ts src/plugins/loader.runtime-registry.test.ts
  • pnpm tsgo:prod
  • git diff --check origin/main...HEAD
  • .agents/skills/autoreview/scripts/autoreview --mode local --prompt "Review this minimal OpenClaw performance patch after the reuse-only fix. Scope: reuse active gateway startup registries only on a compatible cache hit for startup-scoped runtime plugin loads; fallback loads must use the caller workspace. Provider-hook and plugin-tools cross-workspace reuse are intentionally absent. Focus on cross-workspace safety, cache key correctness, and test coverage for cache miss fallback."
  • Focused live iMessage proof, June 24, 2026, OPENCLAW_SKIP_CRON=1 OPENCLAW_LOG_LEVEL=debug OPENCLAW_VERBOSE=1 pnpm openclaw gateway, commit a397a64: 20504ms total message processing, 20391ms dispatch, 13583ms model/main lane; remaining hot-path plugin loads were 7.0ms + 1268.6ms + 2281.7ms + 120.4ms.
  • Caveat: the focused live run did not include Fix Codex message-tool-only source reply completion #95942, so its incomplete turn detected log is not a verdict on the Codex source-reply fix.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Jun 24, 2026
This was referenced Jun 24, 2026
Closed
Closed
@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 9:28 PM ET / 01:28 UTC.

Summary
The PR tracks active runtime registry plugin-id scope and reuses a compatible gateway startup runtime plugin registry for startup-scoped agent runtime plugin loads across workspace boundaries.

PR surface: Source +80, Tests +192. Total +272 across 8 files.

Reproducibility: yes. with caveat: the linked tracker and PR body contain live timing/log evidence, and the source path from ensureRuntimePluginsLoaded into active registry compatibility checks is clear. I did not run a fresh live iMessage or gateway performance reproduction in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Cross-workspace reuse path: 1 reuse path added. The PR lets runtime plugin loading return an active gateway registry for a different turn workspace, so cache-key and scope compatibility are the core merge question.
  • Process-local registry metadata: 1 state field added. The new active pluginIdScope metadata becomes part of later registry compatibility decisions.

Stored data model
Persistent data-model change detected: persistent cache schema: src/agents/runtime-plugins.registry-reuse.test.ts, persistent cache schema: src/agents/runtime-plugins.test.ts, persistent cache schema: src/plugins/loader.runtime-registry.test.ts, persistent cache schema: src/plugins/runtime/standalone-runtime-registry-loader.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96148
Summary: The open latency tracker is canonical; this PR is a narrow candidate fix for the startup-scoped runtime registry reuse slice, while the closed broad draft overlaps but is no longer a viable landing target.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Add or cite pnpm build proof for the plugin runtime loading change before marking the draft ready.

Risk before merge

  • [P1] Cross-workspace startup-registry reuse remains compatibility-sensitive because a wrong cache-key, plugin-scope, or workspace guard can reuse plugin runtime state for the wrong turn workspace.
  • [P1] The PR evidence still does not list pnpm build, and the scoped agent/plugin policies call for build proof when plugin runtime loading or lazy-loading behavior changes.

Maintainer options:

  1. Confirm compatibility and build proof (recommended)
    Keep this PR as the narrow path, but require maintainer review plus build proof for the cross-workspace cache-key and plugin-scope guards before merge.
  2. Accept the scoped runtime-cache risk
    Maintainers may decide the targeted tests and live logs are enough for this limited startup-scoped reuse path despite the missing build proof.
  3. Pause behind the latency tracker
    If the permanent registry-reuse contract is still unsettled, leave the draft parked under the open latency tracker instead of merging this cache behavior now.

Next step before merge

  • [P2] The protected maintainer label, draft state, and compatibility-sensitive registry reuse make this a human review item rather than an automated repair or close candidate.

Security
Cleared: The diff changes runtime registry reuse and tests only; it does not add dependencies, workflows, lockfiles, secret handling, package metadata, or downloaded code.

Review details

Best possible solution:

Land this narrow startup-scoped registry reuse only after maintainer review confirms the compatibility contract and build proof is added or intentionally waived; keep provider-hook and plugin-tool cross-workspace reuse on separate, better-proven work.

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

Yes, with caveat: the linked tracker and PR body contain live timing/log evidence, and the source path from ensureRuntimePluginsLoaded into active registry compatibility checks is clear. I did not run a fresh live iMessage or gateway performance reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, as a narrow merge shape: the PR only reuses an already-loaded compatible startup registry and preserves caller-workspace fallback on miss. The safer alternative to the broad draft is exactly this split, with maintainer compatibility review and build proof before merge.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against a21144d8a606.

Label changes

Label justifications:

  • P2: The PR targets a real runtime-plugin latency problem with bounded blast radius, but it is not an emergency outage or security issue.
  • merge-risk: 🚨 compatibility: The diff changes runtime plugin registry reuse across workspace boundaries, which can affect existing plugin config, installed plugin roots, and upgrade behavior if the guard is wrong.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The PR body and linked tracker provide after-change live gateway/iMessage diagnostic logs showing startup/prewarm timing and remaining hot-path plugin loads for this commit.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and linked tracker provide after-change live gateway/iMessage diagnostic logs showing startup/prewarm timing and remaining hot-path plugin loads for this commit.
Evidence reviewed

PR surface:

Source +80, Tests +192. Total +272 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 5 99 19 +80
Tests 3 192 0 +192
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 291 19 +272

What I checked:

  • Repository policy read: Root policy and the scoped agents/plugins policies were read fully; they make plugin runtime cache/reuse and lazy-loading changes compatibility-sensitive and call for build proof when plugin runtime loading changes. (AGENTS.md:11, a21144d8a606)
  • PR head runtime entry point: At PR head, ensureRuntimePluginsLoaded probes the active gateway workspace only when startup plugin IDs exist, then falls back to the original caller workspace load options on a miss. (src/agents/runtime-plugins.ts:68, a397a64e570a)
  • PR head compatibility guard: The loader records active pluginIdScope and only treats a requested subset as compatible when the active scoped cache key and plugin-id scope contain the requested scope. (src/plugins/loader.ts:1227, a397a64e570a)
  • PR head state tracking: setActivePluginRegistry stores a copied plugin-id scope and exposes it through getActivePluginRegistryPluginIdScope for compatibility checks. (src/plugins/runtime.ts:199, a397a64e570a)
  • Current main behavior: Current main's standalone runtime plugin load always passes the caller workspace into ensureStandaloneRuntimePluginRegistryLoaded; it does not yet have the new cross-workspace startup-registry hit path. (src/agents/runtime-plugins.ts:55, a21144d8a606)
  • Current main compatibility callee: Current main's loaded-registry path delegates active-surface compatibility to resolveCompatibleRuntimePluginRegistry and otherwise rejects workspace mismatches. (src/plugins/active-runtime-registry.ts:95, a21144d8a606)

Likely related people:

  • Shakker: Current-main blame and file history attribute the active runtime plugin and loader files to commit 00b6f49. (role: current source introducer and recent area contributor; confidence: high; commits: 00b6f49b24b2; files: src/agents/runtime-plugins.ts, src/plugins/active-runtime-registry.ts, src/plugins/loader.ts)
  • Vincent Koc: Recent history includes explicit plugin scope handling and runtime registry reuse for provider snapshots in the plugin loader. (role: recent adjacent plugin-loader contributor; confidence: medium; commits: 6a189eec0b1c, 883df8c6a85e; files: src/plugins/loader.ts)
  • Peter Steinberger: Recent history shows repeated plugin loader/runtime and gateway-bindable compatibility work adjacent to this cache/reuse path. (role: recent adjacent runtime/plugin contributor; confidence: medium; commits: fa67ab2358df, 725cbcc36246, 510fca655afd; files: src/plugins/loader.ts, src/plugins/active-runtime-registry.ts, src/agents/runtime-plugins.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 24, 2026
@omarshahine

omarshahine commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@/var/folders/ks/54l0_v5j441gbf_8nbz9kgb80000gn/T/tmp.gp8Pu8Syl7

@clawsweeper clawsweeper Bot mentioned this pull request Jun 27, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@omarshahine