Skip to content

perf: skip runtime-deps manifest scans when materialized#75325

Merged
clawsweeper[bot] merged 4 commits intomainfrom
codex/runtime-deps-fast-path
May 1, 2026
Merged

perf: skip runtime-deps manifest scans when materialized#75325
clawsweeper[bot] merged 4 commits intomainfrom
codex/runtime-deps-fast-path

Conversation

@steipete
Copy link
Copy Markdown
Contributor

@steipete steipete commented May 1, 2026

Summary

  • Problem: packaged plugin lazy loading can still enter the package-level runtime-deps planning path even when the requested plugin deps are already materialized.
  • Why it matters: that path scans bundled plugin manifests and contributes avoidable filesystem work in the gateway CPU/event-loop starvation cluster.
  • What changed: add a fast path that proves the requested plugin deps plus mirrored root deps are already materialized before scanning all bundled plugin manifests.
  • What did NOT change (scope boundary): no install-root selection changes, no package-manager behavior changes, no Telegram/channel behavior changes.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: the lazy plugin runtime-deps path only checked the full package-level plan after collecting all bundled plugin deps, so an already-materialized generated manifest still paid the all-plugin manifest scan cost.
  • Missing detection / guardrail: no regression test asserted that a materialized generated package-level manifest avoids reading unrelated plugin manifests.
  • Contributing context (if known): packaged installs with runtime-deps mirrors and channel/provider plugin loading make this path visible during gateway startup and control-plane operations.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/plugins/bundled-runtime-deps.test.ts
  • Scenario the test should lock in: generated package-level deps are already materialized, the requested plugin loads, and unrelated plugin manifests are not read.
  • Why this is the smallest reliable guardrail: it exercises the exact materialization helper path with filesystem fixtures and a read spy.
  • Existing test that already covers this (if any): adjacent runtime-deps materialization tests covered reinstall avoidance, but not scan avoidance.
  • If no new test is added, why not: N/A.

User-visible / Behavior Changes

None directly. This is a performance/responsiveness optimization for packaged plugin runtime-deps loading.

Diagram (if applicable)

Before:
load plugin -> scan all bundled plugin manifests -> discover generated manifest is enough -> skip install

After:
load plugin -> prove requested deps already materialized -> skip all-plugin scan and install

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: macOS local plus Blacksmith Linux Testbox
  • Runtime/container: Node/pnpm via repo wrappers
  • Model/provider: N/A
  • Integration/channel (if any): bundled plugin runtime-deps
  • Relevant config (redacted): OPENCLAW_PLUGIN_STAGE_DIR fixture

Steps

  1. Create a packaged OpenClaw fixture with two bundled plugins and an external runtime-deps install root.
  2. Materialize a generated package-level manifest containing both plugin deps.
  3. Load one plugin and assert the other plugin manifest is not read.

Expected

  • The lazy runtime-deps path returns without reinstalling and without scanning unrelated plugin manifests.

Actual

  • Before this patch, the path had to collect the package-level plan first. After this patch, the fast path returns before the all-plugin scan.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: focused runtime-deps test passed locally; combined changed gate passed on Blacksmith Testbox before PR split.
  • Edge cases checked: stale generated manifest and stale installed package paths remain covered by existing adjacent tests.
  • What you did not verify: live Linux npm-global Telegram repro/profile.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: an incomplete package-level generated manifest could hide missing deps.
    • Mitigation: the fast path checks only the requested plugin deps plus mirrored root deps with isRuntimeDepsPlanMaterialized; stale/incomplete manifests fall through to the existing full package-level plan.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 1, 2026
@steipete
Copy link
Copy Markdown
Contributor Author

steipete commented May 1, 2026

@clawsweeper automerge

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026

Codex review: passed for ClawSweeper automerge.

What this changes:

The PR adds a bundled plugin runtime-deps fast path that returns before scanning all bundled plugin manifests when the requested package-level deps plus mirrored root deps are already materialized, with regression tests and an unreleased changelog update.

Automerge follow-up:

No repair lane is needed: this automerge-opted PR has no review findings, the changelog blocker is addressed on the latest head, and the remaining action is exact-head CI/automerge gating under the protected maintainer-label workflow.

Security review:

Security review cleared: Security review cleared: the diff changes runtime-deps planning, tests, and changelog text only, with no new dependency source, workflow permission, lockfile change, lifecycle script, secret handling, or command-execution surface.

Review details

Best possible solution:

Let the exact current PR head finish CI and proceed through the existing ClawSweeper automerge path if required checks stay green, while keeping the broader gateway event-loop starvation issues open for follow-up work outside this narrow runtime-deps materialization fast path.

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

Yes. The high-confidence reproduction is the packaged two-plugin fixture described in the PR: current main builds the package-level plan before the materialization check, while the PR's read-spy test verifies the unrelated bundled manifest is not read after the fast path.

Is this the best way to solve the issue?

Yes. The patch reuses the existing isRuntimeDepsPlanMaterialized contract, limits the early return to the requested plugin plan plus mirrored root deps, includes config-dependent manifest runtime deps when needed, and falls back to the existing full package-level plan when the proof is incomplete.

What I checked:

  • Current main scans before materialization proof: On current main, ensureBundledPluginRuntimeDeps enters collectBundledPluginRuntimeDeps for the package-level plan before checking isRuntimeDepsPlanMaterialized, so a materialized generated manifest can still pay the all-plugin manifest scan cost first. (src/plugins/bundled-runtime-deps.ts:344, b277ae3f4c40)
  • Materialization contract is strong enough for a fast path: isRuntimeDepsPlanMaterialized accepts generated manifest supersets only when all requested specs are included and each requested package is satisfied on disk. (src/plugins/bundled-runtime-deps-materialization.ts:131, b277ae3f4c40)
  • PR adds requested-plugin materialization check before full scan: The PR patch adds arePackageLevelRuntimeDepsAlreadyMaterialized and collectPackageLevelRuntimeDepsForPlugin, then calls them before the existing full package-level collectBundledPluginRuntimeDeps fallback. (src/plugins/bundled-runtime-deps.ts:208, 3a03170b52e3)
  • Regression tests cover scan avoidance and manifest deps: The PR adds one test asserting beta's manifest is not read when alpha's requested package-level deps are materialized, and a second test proving config-dependent manifest runtime deps such as local memory embeddings are not skipped. (src/plugins/bundled-runtime-deps.test.ts:2305, 3a03170b52e3)
  • Changelog blocker addressed on latest head: The latest patch updates the active unreleased Plugins/runtime-deps entry with the event-loop pressure context and contributor credits. (CHANGELOG.md:15, 3a03170b52e3)
  • Exact-head PR state and checks: The GitHub API reports the PR open, non-draft, mergeable, and on head 3a03170; check-runs showed no failures, with 73 success, 1 neutral, 22 skipped, and 2 still in progress at review time. (3a03170b52e3)

Likely related people:

  • steipete: Current-main blame for ensureBundledPluginRuntimeDeps points to Peter Steinberger-authored runtime-deps work, and recent adjacent commits touched bundled runtime mirror and selection behavior in the same performance cluster. (role: introduced behavior and recent runtime-deps maintainer; confidence: high; commits: 165d62b15ff2, b743506549d6; files: src/plugins/bundled-runtime-deps.ts, src/plugins/bundled-runtime-deps-selection.ts, src/plugins/bundled-runtime-root.ts)
  • Vincent Koc: The generated-manifest superset behavior used by this fast path was introduced in the recent runtime-deps reinstall-loop fix. (role: recent adjacent materialization maintainer; confidence: medium; commits: e311ffdcb94e; files: src/plugins/bundled-runtime-deps-materialization.ts, src/plugins/bundled-runtime-deps.test.ts)

Remaining risk / open question:

  • Two exact-head check runs were still in progress at review time; merge should remain gated on their completion.
  • This PR only removes one manifest-scan path in the broader gateway event-loop starvation cluster, so the related open reports still need their remaining mirror, async, and registry-cache work tracked separately.

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

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

clawsweeper Bot commented May 1, 2026

🦞🦞
ClawSweeper automerge is enabled for this PR.

I added clawsweeper:automerge and asked ClawSweeper to review this head. If ClawSweeper emits a repair marker or requests changes, I will repair/rebase the branch and ask for another review, up to the configured round limit.

Draft PRs stay fix-only until GitHub marks them ready for review. A maintainer can pause this with /clawsweeper stop.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026

🦞🦞
Thanks, ClawSweeper. ClawSweeper saw the passing review, but did not merge yet.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=eccd38da0b5b5fb4f48412351c781d0cdbb8aecb)
Merge status: CHANGELOG.md entry is required for user-facing ClawSweeper automerge changes

I left the PR open for the remaining gate instead of bypassing it.

@steipete steipete force-pushed the codex/runtime-deps-fast-path branch from 613eaa7 to 3a03170 Compare May 1, 2026 01:17
@clawsweeper clawsweeper Bot merged commit 3c48510 into main May 1, 2026
99 checks passed
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026

🦞🦞
Thanks, ClawSweeper. ClawSweeper merged this PR after the passing review.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=3a03170b52e395b9071ce09f57a6cc4f45468868)
Merge status: merged by ClawSweeper automerge
Merged at: 2026-05-01T01:25:46Z

The automerge loop is complete.

@clawsweeper clawsweeper Bot deleted the codex/runtime-deps-fast-path branch May 1, 2026 01:25
lxe pushed a commit to lxe/openclaw that referenced this pull request May 6, 2026
)

* perf: skip runtime-deps manifest scans when materialized

* fix: include manifest deps in runtime fast path

* fix: type runtime deps normalizer helper

* docs: credit runtime deps event-loop fix
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
)

* perf: skip runtime-deps manifest scans when materialized

* fix: include manifest deps in runtime fast path

* fix: type runtime deps normalizer helper

* docs: credit runtime deps event-loop fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant