fix(gateway): clear stale credential warnings after channel plugin removal (#127503)

* fix(gateway): retire removed channel credential owners

Retire stale account diagnostics only after successful channel plugin removal. Preserve independently owned credential diagnostics across snapshot replacement and rollback.

* test(gateway): cover plugin-disable owner pruning
This commit is contained in:
Peter Steinberger
2026-08-21 15:58:48 -07:00
committed by GitHub
parent a7a68cfb47
commit be0bcb395d
7 changed files with 88 additions and 6 deletions
@@ -340,6 +340,7 @@ describe("mcp connection resolver helpers", () => {
};
const reloadLog = { info: vi.fn(), warn: vi.fn(), error: vi.fn() };
const requestRecoveryRestart = vi.fn(() => ({ status: "failed" as const }));
const pruneInactiveChannelAccountState = vi.fn();
const nextConfig: OpenClawConfig = {
plugins: {
entries: {
@@ -365,6 +366,7 @@ describe("mcp connection resolver helpers", () => {
},
async startChannel() {},
async stopChannel() {},
pruneInactiveChannelAccountState,
async reloadPlugins({ beforeReplace, commitRuntime }) {
await beforeReplace(new Set());
await commitRuntime();
@@ -392,6 +394,7 @@ describe("mcp connection resolver helpers", () => {
});
expect(refreshContextWindowCache).toHaveBeenCalledWith(nextConfig);
expect(requestRecoveryRestart).not.toHaveBeenCalled();
expect(pruneInactiveChannelAccountState).toHaveBeenCalledExactlyOnceWith(new Set());
expect(isPluginRegistryRetired(previous.registry)).toBe(true);
expect(peekSessionMcpRuntime({ sessionId })).toBeUndefined();
+38
View File
@@ -3087,6 +3087,44 @@ describe("server-channels auto restart", () => {
]);
});
it("prunes only credential owners and account state for inactive channel plugins", async () => {
installTestRegistry(
...(["discord", "slack"] as const).map((channelId) =>
createTestPlugin({
id: channelId,
listAccountIds: () => ["Ops Team"],
startAccount: async () => {},
resolveAccount: (_cfg, accountId) => ({
enabled: true,
configured: true,
credentialDiagnostics: [
{
code: "CREDENTIAL_FILE_UNAVAILABLE" as const,
path: `channels.${channelId}.accounts.${accountId}.tokenFile`,
reason: "not-found",
},
],
}),
}),
),
);
const manager = createManager({ channelIds: ["discord", "slack"] });
await manager.startChannels();
expect(listActiveDegradedSecretOwners().map((owner) => owner.ownerId)).toEqual([
"discord:ops-team",
"slack:ops-team",
]);
manager.pruneInactiveChannelAccountState(new Set(["slack"]));
expect(listActiveDegradedSecretOwners().map((owner) => owner.ownerId)).toEqual([
"slack:ops-team",
]);
expect(manager.resolveRuntimeAccountId("discord", "ops-team")).toBeUndefined();
expect(manager.resolveRuntimeAccountId("slack", "ops-team")).toBe("Ops Team");
});
it("resolves only an unambiguous authoritative runtime account for a normalized owner", async () => {
let accountIds = ["Ops Team"];
installTestRegistry(
+10
View File
@@ -284,6 +284,7 @@ export type ChannelManager = {
// Channel docking: lifecycle hooks (`plugin.gateway`) flow through this manager.
export function createChannelManager(opts: ChannelManagerOptions): ChannelManager & {
pruneInactiveChannelAccountState: (activeChannelIds: ReadonlySet<ChannelId>) => void;
resolveRuntimeAccountId: (channelId: ChannelId, accountId: string) => string | undefined;
} {
const {
@@ -500,6 +501,14 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
}
};
const pruneInactiveChannelAccountState = (activeChannelIds: ReadonlySet<ChannelId>): void => {
for (const [channelId, store] of channelStores) {
if (!activeChannelIds.has(channelId)) {
evictStaleChannelAccountState(channelId, store, []);
}
}
};
const startChannelProcessOwned = async (
channelId: ChannelId,
accountId?: string,
@@ -1420,6 +1429,7 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
startChannels,
startChannel,
stopChannel,
pruneInactiveChannelAccountState,
setAutostartSuppression: (suppression) => {
autostartSuppression = suppression;
},
+5 -2
View File
@@ -151,6 +151,7 @@ export type GatewayReloadHandlerParams = {
getPluginMetadataSnapshot?: () => PluginMetadataSnapshot | undefined;
startChannel: GatewayChannelManager["startChannel"];
stopChannel: GatewayChannelManager["stopChannel"];
pruneInactiveChannelAccountState: (activeChannelIds: ReadonlySet<ChannelKind>) => void;
getChannelAutostartSuppression?: GatewayChannelManager["getAutostartSuppression"];
stopPostReadySidecars?: () => Promise<void> | void;
reloadPlugins: (params: {
@@ -187,7 +188,7 @@ export type GatewayReloadHandlerParams = {
export type ManagedGatewayConfigReloaderParams = Omit<
GatewayReloadHandlerParams,
"assertRestartReady" | "createHealthMonitor" | "logReload"
"assertRestartReady" | "createHealthMonitor" | "logReload" | "pruneInactiveChannelAccountState"
> & {
configRevisionProjector: import("./config-revision-token.js").GatewayConfigRevisionProjector;
minimalTestGateway: boolean;
@@ -204,7 +205,9 @@ export type ManagedGatewayConfigReloaderParams = Omit<
promoteSnapshot: typeof import("../config/config.js").promoteConfigSnapshotToLastKnownGood;
subscribeToWrites: typeof import("../config/config.js").registerConfigWriteListener;
logReload: GatewayReloadLog & { error: (msg: string) => void };
channelManager: GatewayChannelManager;
channelManager: GatewayChannelManager & {
pruneInactiveChannelAccountState: GatewayReloadHandlerParams["pruneInactiveChannelAccountState"];
};
activateRuntimeSecrets: ActivateRuntimeSecrets;
/** Applies one immutable effective config/compare snapshot before reload planning. */
prepareConfigCandidate?: (params: {
+26 -3
View File
@@ -133,6 +133,7 @@ function createGatewayReloadHandlers(
setState: vi.fn(),
startChannel: vi.fn(async () => {}),
stopChannel: vi.fn(async () => {}),
pruneInactiveChannelAccountState: vi.fn(),
stopPostReadySidecars: vi.fn(),
reloadPlugins: vi.fn(async () => makePluginReloadResult()),
logHooks: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
@@ -196,7 +197,7 @@ function startManagedGatewayConfigReloader(params: ManagedReloaderTestParams) {
logChannels: { info: vi.fn(), error: vi.fn() },
logCron: { error: vi.fn() },
logReload: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
channelManager: {} as never,
channelManager: { pruneInactiveChannelAccountState: vi.fn() } as never,
activateRuntimeSecrets: vi.fn(async (config: OpenClawConfig) =>
makePreparedSecretsSnapshot(config),
) as never,
@@ -611,6 +612,7 @@ function createReloadHandlersForTest(
channels?: {
start: ReloadHandlerParams["startChannel"];
stop: ReloadHandlerParams["stopChannel"];
pruneInactiveChannelAccountState?: ReloadHandlerParams["pruneInactiveChannelAccountState"];
},
reloadPlugins?: Parameters<typeof createGatewayReloadHandlers>[0]["reloadPlugins"],
stopPostReadySidecars = vi.fn(),
@@ -648,6 +650,9 @@ function createReloadHandlersForTest(
setState,
startChannel: channels?.start ?? vi.fn(async () => {}),
stopChannel: channels?.stop ?? vi.fn(async () => {}),
...(channels?.pruneInactiveChannelAccountState
? { pruneInactiveChannelAccountState: channels.pruneInactiveChannelAccountState }
: {}),
...(reloadPlugins ? { reloadPlugins } : {}),
getChannelAutostartSuppression: options?.getChannelAutostartSuppression,
stopPostReadySidecars,
@@ -5583,6 +5588,7 @@ describe("gateway plugin hot reload handlers", () => {
it("restarts pre-stopped channel targets when runtime publication fails", async () => {
const events: string[] = [];
const pruneInactiveChannelAccountState = vi.fn();
const publish = vi.fn(async () => {
throw new Error("publication failed");
});
@@ -5611,6 +5617,7 @@ describe("gateway plugin hot reload handlers", () => {
start: vi.fn(async (channel, accountId) => {
events.push(`start:${channel}:${accountId ?? "all"}`);
}),
pruneInactiveChannelAccountState,
},
reloadPlugins,
);
@@ -5629,11 +5636,13 @@ describe("gateway plugin hot reload handlers", () => {
"start:slack:catalog-account",
"start:discord:all",
]);
expect(pruneInactiveChannelAccountState).not.toHaveBeenCalled();
expect(handlers.setState).not.toHaveBeenCalled();
});
it("restarts pre-stopped account targets when plugin replacement is cancelled", async () => {
const events: string[] = [];
const pruneInactiveChannelAccountState = vi.fn();
const reloadPlugins = vi.fn(
async (params: {
beforeReplace: (
@@ -5656,6 +5665,7 @@ describe("gateway plugin hot reload handlers", () => {
start: vi.fn(async (channel, accountId) => {
events.push(`start:${channel}:${accountId ?? "all"}`);
}),
pruneInactiveChannelAccountState,
},
reloadPlugins,
);
@@ -5665,6 +5675,7 @@ describe("gateway plugin hot reload handlers", () => {
).rejects.toThrow("config hot reload cancelled by config supersession or in-process restart");
expect(events).toEqual(["stop:discord:catalog-account", "start:discord:catalog-account"]);
expect(pruneInactiveChannelAccountState).not.toHaveBeenCalled();
expect(handlers.setState).not.toHaveBeenCalled();
});
@@ -5675,6 +5686,7 @@ describe("gateway plugin hot reload handlers", () => {
const logChannels = { info: vi.fn(), error: vi.fn() };
const events: string[] = [];
const startRootCounts: number[] = [];
const pruneInactiveChannelAccountState = vi.fn();
const startChannel = vi.fn(async (channel: ChannelKind) => {
events.push(`start:${channel}`);
startRootCounts.push(getActiveGatewayRootWorkCount({ excludeCurrent: true }));
@@ -5700,6 +5712,7 @@ describe("gateway plugin hot reload handlers", () => {
setState,
startChannel,
stopChannel,
pruneInactiveChannelAccountState,
reloadPlugins,
logChannels,
});
@@ -5734,6 +5747,7 @@ describe("gateway plugin hot reload handlers", () => {
expect(startChannel).toHaveBeenCalledWith("telegram");
expect(startChannel).toHaveBeenCalledWith("discord");
expect(startRootCounts).toEqual([1, 1]);
expect(pruneInactiveChannelAccountState).not.toHaveBeenCalled();
expect(setState).not.toHaveBeenCalled();
});
@@ -5743,6 +5757,10 @@ describe("gateway plugin hot reload handlers", () => {
const setState = vi.fn();
const startChannel = vi.fn(async () => {});
const events: string[] = [];
const activeChannels = new Set<ChannelKind>(["slack"]);
const pruneInactiveChannelAccountState = vi.fn(() => {
events.push("prune");
});
const stopChannel = vi.fn(async () => {
events.push("stop");
});
@@ -5753,7 +5771,7 @@ describe("gateway plugin hot reload handlers", () => {
events.push("reload:start");
await params.beforeReplace(new Set(["discord"]));
events.push("registry:replace");
return makePluginReloadResult();
return makePluginReloadResult({ activeChannels });
},
);
const { applyHotReload } = createGatewayReloadHandlers({
@@ -5761,6 +5779,7 @@ describe("gateway plugin hot reload handlers", () => {
setState,
startChannel,
stopChannel,
pruneInactiveChannelAccountState,
reloadPlugins,
});
const sourceConfig: OpenClawConfig = {
@@ -5798,7 +5817,8 @@ describe("gateway plugin hot reload handlers", () => {
expect(reloadParamsRecord?.changedPaths).toEqual(["plugins.enabled"]);
expect(stopChannel).toHaveBeenCalledWith("discord", undefined, { manual: false });
expect(startChannel).not.toHaveBeenCalled();
expect(events).toEqual(["reload:start", "stop", "registry:replace"]);
expect(pruneInactiveChannelAccountState).toHaveBeenCalledExactlyOnceWith(activeChannels);
expect(events).toEqual(["reload:start", "stop", "registry:replace", "prune"]);
expect(setState).toHaveBeenCalledTimes(1);
});
@@ -6163,6 +6183,7 @@ describe("deferred channel reload abort generation", () => {
start: vi.fn(async () => {}),
stop: vi.fn(async () => {}),
};
const pruneInactiveChannelAccountState = vi.fn();
let receivedIsAborted = false;
let reloadWasCancelled = false;
const reloadPlugins = vi.fn(
@@ -6185,6 +6206,7 @@ describe("deferred channel reload abort generation", () => {
const { applyHotReload } = createGatewayReloadHandlers({
startChannel: channels.start,
stopChannel: channels.stop,
pruneInactiveChannelAccountState,
reloadPlugins,
logChannels,
});
@@ -6217,6 +6239,7 @@ describe("deferred channel reload abort generation", () => {
// No channel should be started — cancelledByRestart = pluginReloadAborted = true
expect(channels.start).not.toHaveBeenCalled();
expect(channels.stop).not.toHaveBeenCalled();
expect(pruneInactiveChannelAccountState).not.toHaveBeenCalled();
} finally {
vi.useRealTimers();
hoisted.activeTaskBlockers.length = 0;
+5 -1
View File
@@ -550,12 +550,16 @@ export function createGatewayReloadHandlers(params: GatewayReloadHandlerParams)
}
// beforeReplace may have set pluginReloadAborted inside reloadPlugins;
// skip metadata/runtime updates when the reload was cancelled mid-flight.
if (!pluginReloadAborted) {
if (!pluginReloadAborted && !isLifecycleReloadAborted()) {
for (const channel of pluginReloadResult.restartChannels) {
channelsToRestart.add(channel);
}
activePluginChannelsAfterReload = pluginReloadResult.activeChannels;
// Only a successfully published replacement can authoritatively retire channel owners.
params.pruneInactiveChannelAccountState(activePluginChannelsAfterReload);
resetPreparedModelRuntimeStateForHotReload();
} else {
pluginReloadAborted = true;
}
}
}
+1
View File
@@ -142,6 +142,7 @@ export function startManagedGatewayConfigReloader(
getPluginMetadataSnapshot: params.getPluginMetadataSnapshot,
startChannel: params.startChannel,
stopChannel: params.stopChannel,
pruneInactiveChannelAccountState: params.channelManager.pruneInactiveChannelAccountState,
getChannelAutostartSuppression: params.getChannelAutostartSuppression,
stopPostReadySidecars: params.stopPostReadySidecars,
reloadPlugins: params.reloadPlugins,