fix(status): surface should-run plugin drift#97878
Conversation
Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 1:45 PM ET / 17:45 UTC. Summary 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 follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest 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 changesLabel justifications:
Evidence reviewedPR surface: Source +107, Tests +302. Total +409 across 7 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
What Problem This Solves
/status plugins(detailed) distinguishes runtime-Loaded:plugins from a neutralInstalled (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
runtimeLoadedPluginIdsprovenance. 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 pluginsgains one new line, and only when there is real drift:runtimeLoadedPluginIds.configuredDeferredChannelPluginIds): they finish loading only after the gateway starts listening, so they are never false-positive drift.error/disabledrecord./statusplugin line is unchanged — no new error chip; drift appears only in the detailed view.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 pluginshandlerbuildStatusPluginsReplyagainst PR HEAD in an isolatedOPENCLAW_STATE_DIR(timestamp 2026-06-29T17:10:13Z) — realcollectInstalledPluginHealthSnapshot(realbuildPluginSnapshotReportinstalled scan + real should-run plan via the sharedresolveGatewayStartupPluginActivationConfighelper → realapplyPluginAutoEnable+ realloadGatewayStartupPluginPlan) → realformatDetailedPluginHealth. The live active registry was set empty via the realsetActivePluginRegistryAPI (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: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 underInstalled (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
src/status/status-plugin-health{,.runtime,.installed}.test.ts) — 32/32.src/gateway/server-startup-plugins.test.ts) — 50/50 (boot behavior preserved after the helper extraction).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-cycles— 0 cycles.oxfmt --check— clean.Known limitations
Non-Scope
server-plugin-bootstrap.tsplugin-loading assembly untouched).CHANGELOG.mdedit (per contributor guidance — maintainers add changelog entries on landing).Risk checklist
try/catch, no drift line).shouldRunPluginIds/runtimeLoadedPluginIdsrender exactly as before./statusplugin line unchanged (no new error chip).server-startup-plugins.test.ts50/50 + the helper extraction being a byte-equivalent call-site swap.pnpm buildshows no[INEFFECTIVE_DYNAMIC_IMPORT]); no new import cycles.#97320.Builds on #97479. Touches
src/status/status-plugin-health{,.runtime}.tsandsrc/gateway/plugin-activation-runtime-config.ts+server-startup-plugins.ts(plus colocated tests).