From 749cb8048f4aabbdfb8eb37b8af2cba0b78ad3b0 Mon Sep 17 00:00:00 2001 From: Eva Date: Fri, 1 May 2026 22:03:06 +0700 Subject: [PATCH] fix: guard stale run context APIs --- .../run-context-lifecycle.contract.test.ts | 47 +++++++++++++++++++ src/plugins/registry-lifecycle.ts | 13 +++++ src/plugins/registry.ts | 47 +++++++++++++++++-- src/plugins/runtime.ts | 4 ++ 4 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 src/plugins/registry-lifecycle.ts diff --git a/src/plugins/contracts/run-context-lifecycle.contract.test.ts b/src/plugins/contracts/run-context-lifecycle.contract.test.ts index 9ce098550cc..0cacb520963 100644 --- a/src/plugins/contracts/run-context-lifecycle.contract.test.ts +++ b/src/plugins/contracts/run-context-lifecycle.contract.test.ts @@ -23,6 +23,7 @@ import { import { createEmptyPluginRegistry } from "../registry-empty.js"; import { setActivePluginRegistry } from "../runtime.js"; import { createPluginRecord } from "../status.test-helpers.js"; +import type { OpenClawPluginApi } from "../types.js"; async function waitForPluginEventHandlers(): Promise { await new Promise((resolve) => { @@ -38,6 +39,52 @@ describe("plugin run context lifecycle", () => { resetAgentEventsForTest(); }); + it("blocks stale plugin API run-context mutations after registry replacement", () => { + const { config, registry } = createPluginRegistryFixture(); + let capturedApi: OpenClawPluginApi | undefined; + registerTestPlugin({ + registry, + config, + record: createPluginRecord({ + id: "stale-run-context-plugin", + name: "Stale Run Context Plugin", + }), + register(api) { + capturedApi = api; + }, + }); + setActivePluginRegistry(registry.registry); + setActivePluginRegistry(createEmptyPluginRegistry()); + + expect( + capturedApi?.setRunContext({ + runId: "stale-run", + namespace: "state", + value: { stale: true }, + }), + ).toBe(false); + expect( + getPluginRunContext({ + pluginId: "stale-run-context-plugin", + get: { runId: "stale-run", namespace: "state" }, + }), + ).toBeUndefined(); + + expect( + setPluginRunContext({ + pluginId: "stale-run-context-plugin", + patch: { runId: "stale-run", namespace: "state", value: { live: true } }, + }), + ).toBe(true); + capturedApi?.clearRunContext({ runId: "stale-run", namespace: "state" }); + expect( + getPluginRunContext({ + pluginId: "stale-run-context-plugin", + get: { runId: "stale-run", namespace: "state" }, + }), + ).toEqual({ live: true }); + }); + it("does not let delayed non-terminal subscriptions resurrect closed run context", async () => { let releaseToolHandler: (() => void) | undefined; let delayedToolHandlerSawContext: unknown; diff --git a/src/plugins/registry-lifecycle.ts b/src/plugins/registry-lifecycle.ts new file mode 100644 index 00000000000..d5b9fef5a3e --- /dev/null +++ b/src/plugins/registry-lifecycle.ts @@ -0,0 +1,13 @@ +import type { PluginRegistry } from "./registry-types.js"; + +const retiredRegistries = new WeakSet(); + +export function markPluginRegistryRetired(registry: PluginRegistry | null | undefined): void { + if (registry) { + retiredRegistries.add(registry); + } +} + +export function isPluginRegistryRetired(registry: PluginRegistry): boolean { + return retiredRegistries.has(registry); +} diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index 839e71501ad..940b9a1eb69 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -99,6 +99,7 @@ import { } from "./memory-state.js"; import { normalizeRegisteredProvider } from "./provider-validation.js"; import { createEmptyPluginRegistry } from "./registry-empty.js"; +import { isPluginRegistryRetired } from "./registry-lifecycle.js"; import type { PluginCliBackendRegistration, PluginCliRegistration, @@ -303,6 +304,9 @@ const activePluginHookRegistrations = resolveGlobalSingleton< type HookRegistration = { event: string; handler: Parameters[1] }; type HookRollbackEntry = { name: string; previousRegistrations: HookRegistration[] }; +type PluginSideEffectGuard = { + active: boolean; +}; type PluginRegistrationCapabilities = { /** Broad registry writes that discovery and live activation both need. */ @@ -338,6 +342,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { const coreGatewayMethods = new Set(coreGatewayMethodNames); const pluginHookRollback = new Map(); const pluginsWithChannelRegistrationConflict = new Set(); + const pluginSideEffectGuards = new Map>(); const pushDiagnostic = (diag: PluginDiagnostic) => { registry.diagnostics.push(diag); @@ -354,6 +359,25 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { return value; }; + const createPluginSideEffectGuard = (pluginId: string): PluginSideEffectGuard => { + const guard = { active: true }; + const guards = pluginSideEffectGuards.get(pluginId) ?? new Set(); + guards.add(guard); + pluginSideEffectGuards.set(pluginId, guards); + return guard; + }; + + const deactivatePluginSideEffectGuards = (pluginId: string): void => { + const guards = pluginSideEffectGuards.get(pluginId); + if (!guards) { + return; + } + for (const guard of guards) { + guard.active = false; + } + pluginSideEffectGuards.delete(pluginId); + }; + const registerCodexAppServerExtensionFactory = ( record: PluginRecord, factory: Parameters[0], @@ -2169,6 +2193,11 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { const registrationMode = params.registrationMode ?? "full"; const registrationCapabilities = resolvePluginRegistrationCapabilities(registrationMode); pluginRuntimeRecordById.set(record.id, record); + const sideEffectGuard = createPluginSideEffectGuard(record.id); + const shouldCommitWorkflowSideEffect = () => + sideEffectGuard.active && + !isPluginRegistryRetired(registry) && + registry.plugins.some((plugin) => plugin.id === record.id && plugin.status === "loaded"); return buildPluginApi({ id: record.id, name: record.name, @@ -2378,14 +2407,25 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { registerRuntimeLifecycle: (lifecycle) => registerRuntimeLifecycle(record, lifecycle), registerAgentEventSubscription: (subscription) => registerAgentEventSubscription(record, subscription), - setRunContext: (patch) => setPluginRunContext({ pluginId: record.id, patch }), + setRunContext: (patch) => + registryParams.activateGlobalSideEffects !== false && + shouldCommitWorkflowSideEffect() + ? setPluginRunContext({ pluginId: record.id, patch }) + : false, getRunContext: (get) => getPluginRunContext({ pluginId: record.id, get }), - clearRunContext: (params) => + clearRunContext: (params) => { + if ( + registryParams.activateGlobalSideEffects === false || + !shouldCommitWorkflowSideEffect() + ) { + return; + } clearPluginRunContext({ pluginId: record.id, runId: params.runId, namespace: params.namespace, - }), + }); + }, registerSessionSchedulerJob: (job) => registerSessionSchedulerJob(record, job), registerMemoryCapability: (capability) => { if (!hasKind(record.kind, "memory")) { @@ -2548,6 +2588,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { }; const rollbackPluginGlobalSideEffects = (pluginId: string) => { + deactivatePluginSideEffectGuards(pluginId); if (registryParams.activateGlobalSideEffects === false) { return; } diff --git a/src/plugins/runtime.ts b/src/plugins/runtime.ts index d831a21eaed..2d86af508b1 100644 --- a/src/plugins/runtime.ts +++ b/src/plugins/runtime.ts @@ -5,6 +5,7 @@ import { dispatchPluginAgentEventSubscriptions, } from "./host-hook-runtime.js"; import { createEmptyPluginRegistry } from "./registry-empty.js"; +import { markPluginRegistryRetired } from "./registry-lifecycle.js"; import type { PluginRegistry } from "./registry-types.js"; import { PLUGIN_REGISTRY_STATE, @@ -129,6 +130,9 @@ export function setActivePluginRegistry( workspaceDir?: string, ) { const previousRegistry = asPluginRegistry(state.activeRegistry); + if (previousRegistry && previousRegistry !== registry) { + markPluginRegistryRetired(previousRegistry); + } state.activeRegistry = registry; state.activeVersion += 1; syncTrackedSurface(state.httpRoute, registry, true);