Skip to content

fix(status): surface should-run plugin drift#97878

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
masatohoshino:fix/plugin-should-run-drift
Jun 29, 2026
Merged

fix(status): surface should-run plugin drift#97878
vincentkoc merged 1 commit into
openclaw:mainfrom
masatohoshino:fix/plugin-should-run-drift

Conversation

@masatohoshino

@masatohoshino masatohoshino commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

/status plugins (detailed) distinguishes runtime-Loaded: plugins from a neutral Installed (not active): inventory line (added in #97479), but it never compares against the gateway's desired startup set. So a plugin the gateway's eager startup plan requires — yet that is absent from the live runtime-loaded set and did not surface an error — is silently bucketed into the neutral "Installed (not active)" line. A genuine startup drift (config changed since the gateway started, or a planned plugin that did not come up) is invisible today.

Why This Change Was Made

This builds on #97479's runtimeLoadedPluginIds provenance. With the observed set already available, the missing half is the desired set: the gateway's eager should-run plugin plan. Comparing the two turns an invisible partial-startup failure into a visible, observer-only diagnostic.

Computing that plan correctly requires the same config assembly the gateway performs at boot (auto-enable the operator activation source, then merge it into the runtime config). Rather than re-implement that inline in the status path — which is drift-prone — this extracts the existing boot assembly (server-startup-plugins.ts, non-minimal branch) into a small shared pure helper, resolveGatewayStartupPluginActivationConfig, so the status diagnostic and gateway boot use the same code and cannot diverge. The gateway change is a mechanical, behavior-preserving call-site swap; server-plugin-bootstrap.ts (the plugin-loading assembly) is intentionally left untouched.

User Impact

Detailed /status plugins gains one new line, and only when there is real drift:

Configured to run but not loaded: N (id, ...)
  • Observer-only. No repair / retry / reload / config / doctor / channel / provider behavior changes; gateway startup behavior is unchanged (the helper extraction is mechanical and test-covered).
  • Surfaces plugins expected by the gateway startup plan but missing from runtimeLoadedPluginIds.
  • Installed-but-not-planned plugins stay neutral — the existing "Installed (not active)" inventory is unchanged; only genuine should-run drift is elevated, and a drift id never appears in both lines.
  • Excludes deferred plugins (configuredDeferredChannelPluginIds): they finish loading only after the gateway starts listening, so they are never false-positive drift.
  • Excludes plugins already explained by an error/disabled record.
  • Compact /status plugin line is unchanged — no new error chip; drift appears only in the detailed view.
  • Uses the shared startup activation config helper, so the should-run set matches the real gateway boot plan (auto-enable + operator activation source + runtime/defaulted merge).
  • Plan resolution failure (or absent config) degrades to previous behavior — the should-run set is resolved lazily inside a try/catch; back-compat: snapshots without it render exactly as before.

Evidence

Reviewed at HEAD 74ad546d24b2c6354da818182874aef0fb2926d9.

Real behavior proof (production path, no mocks)

Ran the real /status plugins handler buildStatusPluginsReply against PR HEAD in an isolated OPENCLAW_STATE_DIR (timestamp 2026-06-29T17:10:13Z) — real collectInstalledPluginHealthSnapshot (real buildPluginSnapshotReport installed scan + real should-run plan via the shared resolveGatewayStartupPluginActivationConfig helper → real applyPluginAutoEnable + real loadGatewayStartupPluginPlan) → real formatDetailedPluginHealth. The live active registry was set empty via the real setActivePluginRegistry API (the planned-but-not-loaded condition); no records are fabricated and nothing is mocked.

Config {commands:{plugins:true}, plugins:{entries:{diffs:{enabled:true}}}}; real should-run plan = [acpx, browser, canvas, device-pair, diffs, file-transfer, memory-core, phone-control, talk-voice]; runtimeLoadedPluginIds = []. Observed /status plugins (detailed) output:

🔌 Plugins: OK
Loaded: 0
Disabled: 59
Installed (not active): 67 (alibaba, amazon-bedrock, amazon-bedrock-mantle, anthropic, +63 more)
Configured to run but not loaded: 9 (acpx, browser, canvas, device-pair, diffs, file-transfer, memory-core, phone-control, +1 more)
Full inventory: /plugins list

This shows, from the real handler: (1) the 9 planned-but-not-loaded plugins surfaced as Configured to run but not loaded; (2) the 67 installed-but-not-planned plugins staying neutral under Installed (not active) (not drift); (3) the drift ids absent from the neutral line (no double-listing); (4) the compact line unchanged (🔌 Plugins: OK).

Targeted tests / checks

  • Status plugin health tests (src/status/status-plugin-health{,.runtime,.installed}.test.ts) — 32/32.
  • Gateway startup tests (src/gateway/server-startup-plugins.test.ts) — 50/50 (boot behavior preserved after the helper extraction).
  • New helper tests (src/gateway/plugin-activation-runtime-config.test.ts) — passed.
  • tsgo -p tsconfig.core.json + test/tsconfig/tsconfig.core.test.json — clean.
  • pnpm build — passed, no [INEFFECTIVE_DYNAMIC_IMPORT]. pnpm check:import-cycles0 cycles. oxfmt --check — clean.
  • Independent codex / GPT-5.5 review — clean ("patch is correct"; confirms behavior-preserving gateway refactor + shared assembly reuse).

Known limitations

  • A full live gateway (real plugin-module loading + chat transport + authorized sender over a running gateway) was not used; the proof drives the real handler/plan/formatter directly with an empty live registry as the closest deterministic production-path proof.
  • The drift comparison is recomputed from current config vs the live runtime-loaded set, so it reflects "current config's plan vs what is loaded now" (the intended semantics).

Non-Scope

  • No repair / retry / reload / auto-enable of plugins.
  • No gateway startup behavior change (the helper extraction is a behavior-preserving call-site swap; server-plugin-bootstrap.ts plugin-loading assembly untouched).
  • No config / doctor / channel / provider behavior change; no compact-line change; no new config or env surface.
  • No CHANGELOG.md edit (per contributor guidance — maintainers add changelog entries on landing).

Risk checklist

  • Observer-only; no repair/auto-mutation. Plan resolution failure (or absent config) degrades to prior behavior (try/catch, no drift line).
  • Back-compat: snapshots without shouldRunPluginIds/runtimeLoadedPluginIds render exactly as before.
  • Compact /status plugin line unchanged (no new error chip).
  • Gateway behavior preserved — proven by server-startup-plugins.test.ts 50/50 + the helper extraction being a byte-equivalent call-site swap.
  • No secret/PII surfaced — renders plugin ids only (same class fix(status): distinguish runtime-loaded plugins from installed inventory #97479 already renders).
  • Lazy imports verified effective (pnpm build shows no [INEFFECTIVE_DYNAMIC_IMPORT]); no new import cycles.
  • No CHANGELOG; no #97320.

Builds on #97479. Touches src/status/status-plugin-health{,.runtime}.ts and src/gateway/plugin-activation-runtime-config.ts + server-startup-plugins.ts (plus colocated tests).

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 1:45 PM ET / 17:45 UTC.

Summary
The PR adds a detailed /status plugins should-run-but-not-loaded line, extracts the gateway startup activation config assembly into a shared helper, and adds focused gateway/status tests.

PR surface: Source +107, Tests +302. Total +409 across 7 files.

Reproducibility: yes. at source level: current main carries runtime-loaded provenance but detailed plugin status never compares it with the gateway should-run startup set. The PR body also includes after-fix real handler output showing the new drift line.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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:

  • none.

Next step before merge

  • No automated repair is needed; the PR is ready for normal maintainer review and exact-head merge gating.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes TypeScript status/gateway diagnostics and tests without dependencies, workflows, lockfiles, downloads, secrets, or write paths.

Review details

Best possible solution:

Land the observer-only diagnostic after maintainer review, keeping the shared startup-plan assembly as the single source for the should-run comparison and leaving startup/repair behavior unchanged.

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

Yes, at source level: current main carries runtime-loaded provenance but detailed plugin status never compares it with the gateway should-run startup set. The PR body also includes after-fix real handler output showing the new drift line.

Is this the best way to solve the issue?

Yes, this is the best narrow fix shape I found: reuse the gateway boot activation-config assembly to compute the desired eager plan, then render drift only in detailed status when runtime provenance is present. That avoids a second startup-plan implementation and preserves compact/status fallback behavior.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded status/gateway diagnostic fix for plugin startup drift with limited blast radius and focused coverage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live/terminal output from the real /status plugins handler path showing the configured-but-not-loaded line and unchanged compact status behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live/terminal output from the real /status plugins handler path showing the configured-but-not-loaded line and unchanged compact status behavior.
Evidence reviewed

PR surface:

Source +107, Tests +302. Total +409 across 7 files.

View PR surface stats
Area Files Added Removed Net
Source 4 119 12 +107
Tests 3 302 0 +302
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 7 421 12 +409

What I checked:

  • Repository policy applied: The root AGENTS.md and scoped gateway AGENTS.md were read fully and applied; the review used the gateway/status hot-path, compatibility, proof, and best-fix guidance. (AGENTS.md:1, 5f60724b6d7b)
  • Gateway scoped policy applied: The scoped gateway guide warns against broad plugin runtime materialization in gateway startup/tests; this PR preserves lazy/static startup-plan boundaries and uses the existing plan resolver rather than loading plugin runtimes in status formatting. (src/gateway/AGENTS.md:1, 5f60724b6d7b)
  • Current main lacks requested comparison: Current main has runtimeLoadedPluginIds and Installed (not active), but no shouldRunPluginIds field or configured-to-run-but-not-loaded line, so the central diagnostic is not already implemented. (src/status/status-plugin-health.ts:246, 5f60724b6d7b)
  • Current installed snapshot stops before startup plan: Current main merges installed and runtime plugin health and returns the snapshot without resolving the gateway eager startup plan. (src/status/status-plugin-health.runtime.ts:183, 5f60724b6d7b)
  • PR renders detailed drift only: The PR adds shouldRunPluginIds, compares it to runtimeLoadedPluginIds, excludes disabled/error records, removes drift ids from neutral inventory, and leaves compact status unchanged. (src/status/status-plugin-health.ts:252, 74ad546d24b2)
  • PR resolves should-run through gateway plan: collectInstalledPluginHealthSnapshot now lazily resolves eager should-run ids via the gateway startup plan, maps runtime config back to the activation source, and removes deferred channel plugin ids before exposing drift data. (src/status/status-plugin-health.runtime.ts:225, 74ad546d24b2)

Likely related people:

  • masatohoshino: Authored the merged predecessor that introduced runtimeLoadedPluginIds and Installed (not active), which this PR directly extends, and authored this follow-up branch. (role: recent feature contributor; confidence: high; commits: 949b1af433c6, 74ad546d24b2; files: src/status/status-plugin-health.ts, src/status/status-plugin-health.runtime.ts, src/plugins/active-runtime-registry.ts)
  • vincentkoc: GitHub metadata lists this person as mergedBy for the prerequisite status PR that created the runtime-loaded provenance this branch builds on. (role: merger of related work; confidence: medium; commits: 949b1af433c6; files: src/status/status-plugin-health.ts, src/status/status-plugin-health.runtime.ts)
  • Alix-007: Local blame on current main points to this author for the existing status formatter, installed snapshot collector, and gateway activation assembly lines sampled for this review. (role: current-main line provenance; confidence: low; commits: f0e2f7b4f5ae; files: src/status/status-plugin-health.ts, src/status/status-plugin-health.runtime.ts, src/gateway/server-startup-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 rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 29, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 29, 2026
@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. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 29, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 29, 2026
@vincentkoc
vincentkoc merged commit 888f399 into openclaw:main Jun 29, 2026
177 of 194 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 30, 2026
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime 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

Development

Successfully merging this pull request may close these issues.

2 participants