perf: avoid heavy ACP provider checks

This commit is contained in:
Peter Steinberger
2026-04-05 22:01:28 +01:00
parent 58f95b8000
commit 8279375bdf
4 changed files with 143 additions and 26 deletions

View File

@@ -3,6 +3,8 @@ import { setTimeout as sleep } from "node:timers/promises";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import type { AcpSessionRuntimeOptions, SessionAcpMeta } from "../../config/sessions/types.js";
import { resetHeartbeatWakeStateForTests } from "../../infra/heartbeat-wake.js";
import { resetSystemEventsForTest } from "../../infra/system-events.js";
import { withTempDir } from "../../test-helpers/temp-dir.js";
import type { AcpRuntime, AcpRuntimeCapabilities } from "../runtime/types.js";
@@ -42,6 +44,7 @@ let findTaskByRunId: typeof import("../../tasks/task-registry.js").findTaskByRun
let resetTaskRegistryForTests: typeof import("../../tasks/task-registry.js").resetTaskRegistryForTests;
let resetTaskFlowRegistryForTests: typeof import("../../tasks/task-flow-registry.js").resetTaskFlowRegistryForTests;
let installInMemoryTaskRegistryRuntime: typeof import("../../test-utils/task-registry-runtime.js").installInMemoryTaskRegistryRuntime;
let configureTaskFlowRegistryRuntime: typeof import("../../tasks/task-flow-registry.store.js").configureTaskFlowRegistryRuntime;
const baseCfg = {
acp: {
@@ -58,6 +61,17 @@ async function withAcpManagerTaskStateDir(run: (root: string) => Promise<void>):
resetTaskRegistryForTests({ persist: false });
resetTaskFlowRegistryForTests({ persist: false });
installInMemoryTaskRegistryRuntime();
configureTaskFlowRegistryRuntime({
store: {
loadSnapshot: () => ({
flows: new Map(),
}),
saveSnapshot: () => {},
upsertFlow: () => {},
deleteFlow: () => {},
close: () => {},
},
});
try {
await run(root);
} finally {
@@ -186,6 +200,8 @@ describe("AcpSessionManager", () => {
({ AcpRuntimeError } = await import("../runtime/errors.js"));
({ findTaskByRunId, resetTaskRegistryForTests } = await import("../../tasks/task-registry.js"));
({ resetTaskFlowRegistryForTests } = await import("../../tasks/task-flow-registry.js"));
({ configureTaskFlowRegistryRuntime } =
await import("../../tasks/task-flow-registry.store.js"));
({ installInMemoryTaskRegistryRuntime } =
await import("../../test-utils/task-registry-runtime.js"));
});
@@ -205,6 +221,8 @@ describe("AcpSessionManager", () => {
} else {
process.env.OPENCLAW_STATE_DIR = ORIGINAL_STATE_DIR;
}
resetSystemEventsForTest();
resetHeartbeatWakeStateForTests();
resetTaskRegistryForTests({ persist: false });
resetTaskFlowRegistryForTests({ persist: false });
});
@@ -328,7 +346,6 @@ describe("AcpSessionManager", () => {
mode: "prompt",
requestId: "direct-parented-run",
});
await flushMicrotasks();
expect(findTaskByRunId("direct-parented-run")).toMatchObject({

View File

@@ -1,8 +1,7 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { resolveAgentWorkspaceDir } from "../agents/agent-scope.js";
import type { ChannelConfiguredBindingProvider, ChannelPlugin } from "../channels/plugins/types.js";
import type { OpenClawConfig } from "../config/config.js";
import { parseTelegramTopicConversation } from "../plugin-sdk/telegram.js";
import { setActivePluginRegistry } from "../plugins/runtime.js";
import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js";
import { buildConfiguredAcpSessionKey } from "./persistent-bindings.types.js";
@@ -84,9 +83,44 @@ const discordBindings: ChannelConfiguredBindingProvider = {
},
};
function parseTelegramTopicConversationForTest(params: {
conversationId: string;
parentConversationId?: string;
}): {
canonicalConversationId: string;
chatId: string;
topicId?: string;
} | null {
const conversationId = params.conversationId.trim();
const parentConversationId = params.parentConversationId?.trim() || undefined;
if (!conversationId) {
return null;
}
const canonicalTopicMatch = /^(-[^:]+):topic:([^:]+)$/.exec(conversationId);
if (canonicalTopicMatch) {
const [, chatId, topicId] = canonicalTopicMatch;
return {
canonicalConversationId: `${chatId}:topic:${topicId}`,
chatId,
topicId,
};
}
if (parentConversationId) {
return {
canonicalConversationId: `${parentConversationId}:topic:${conversationId}`,
chatId: parentConversationId,
topicId: conversationId,
};
}
return {
canonicalConversationId: conversationId,
chatId: conversationId,
};
}
const telegramBindings: ChannelConfiguredBindingProvider = {
compileConfiguredBinding: ({ conversationId }) => {
const parsed = parseTelegramTopicConversation({ conversationId });
const parsed = parseTelegramTopicConversationForTest({ conversationId });
if (!parsed || !parsed.chatId.startsWith("-")) {
return null;
}
@@ -96,7 +130,7 @@ const telegramBindings: ChannelConfiguredBindingProvider = {
};
},
matchInboundConversation: ({ compiledBinding, conversationId, parentConversationId }) => {
const incoming = parseTelegramTopicConversation({
const incoming = parseTelegramTopicConversationForTest({
conversationId,
parentConversationId,
});
@@ -376,8 +410,20 @@ function mockReadySession(params: {
return sessionKey;
}
beforeEach(async () => {
vi.resetModules();
beforeAll(async () => {
persistentBindingsResolveModule = await import("./persistent-bindings.resolve.js");
lifecycleBindingsModule = await import("./persistent-bindings.lifecycle.js");
persistentBindings = {
resolveConfiguredAcpBindingRecord:
persistentBindingsResolveModule.resolveConfiguredAcpBindingRecord,
resolveConfiguredAcpBindingSpecBySessionKey:
persistentBindingsResolveModule.resolveConfiguredAcpBindingSpecBySessionKey,
ensureConfiguredAcpBindingSession: lifecycleBindingsModule.ensureConfiguredAcpBindingSession,
resetAcpSessionInPlace: lifecycleBindingsModule.resetAcpSessionInPlace,
};
});
beforeEach(() => {
setActivePluginRegistry(
createTestRegistry([
{
@@ -405,16 +451,6 @@ beforeEach(async () => {
managerMocks.initializeSession.mockReset().mockResolvedValue(undefined);
managerMocks.updateSessionRuntimeOptions.mockReset().mockResolvedValue(undefined);
sessionMetaMocks.readAcpSessionEntry.mockReset().mockReturnValue(undefined);
persistentBindingsResolveModule = await import("./persistent-bindings.resolve.js");
lifecycleBindingsModule = await import("./persistent-bindings.lifecycle.js");
persistentBindings = {
resolveConfiguredAcpBindingRecord:
persistentBindingsResolveModule.resolveConfiguredAcpBindingRecord,
resolveConfiguredAcpBindingSpecBySessionKey:
persistentBindingsResolveModule.resolveConfiguredAcpBindingSpecBySessionKey,
ensureConfiguredAcpBindingSession: lifecycleBindingsModule.ensureConfiguredAcpBindingSession,
resetAcpSessionInPlace: lifecycleBindingsModule.resetAcpSessionInPlace,
};
});
describe("resolveConfiguredAcpBindingRecord", () => {

View File

@@ -1,8 +1,13 @@
import { describe, expect, it, vi } from "vitest";
const hoisted = vi.hoisted(() => ({
classifyProviderFailoverReasonWithPlugin: vi.fn(() => null),
matchesProviderContextOverflowWithPlugin: vi.fn(() => false),
}));
vi.mock("../../plugins/provider-runtime.js", () => ({
classifyProviderFailoverReasonWithPlugin: () => null,
matchesProviderContextOverflowWithPlugin: () => false,
classifyProviderFailoverReasonWithPlugin: hoisted.classifyProviderFailoverReasonWithPlugin,
matchesProviderContextOverflowWithPlugin: hoisted.matchesProviderContextOverflowWithPlugin,
}));
import { classifyFailoverReason, isContextOverflowError } from "./errors.js";
@@ -12,6 +17,15 @@ import {
} from "./provider-error-patterns.js";
describe("matchesProviderContextOverflow", () => {
it("skips provider hook dispatch for unrelated errors", () => {
hoisted.matchesProviderContextOverflowWithPlugin.mockClear();
expect(
matchesProviderContextOverflow("Permission denied for /root/oc-acp-write-should-fail.txt."),
).toBe(false);
expect(hoisted.matchesProviderContextOverflowWithPlugin).not.toHaveBeenCalled();
});
it.each([
// AWS Bedrock
"ValidationException: The input is too long for the model",
@@ -37,9 +51,11 @@ describe("matchesProviderContextOverflow", () => {
});
it("does not match unrelated errors", () => {
hoisted.matchesProviderContextOverflowWithPlugin.mockClear();
expect(matchesProviderContextOverflow("rate limit exceeded")).toBe(false);
expect(matchesProviderContextOverflow("invalid api key")).toBe(false);
expect(matchesProviderContextOverflow("internal server error")).toBe(false);
expect(hoisted.matchesProviderContextOverflowWithPlugin).not.toHaveBeenCalled();
});
});

View File

@@ -6,10 +6,7 @@
* yet ship a dedicated provider plugin hook surface.
*/
import {
classifyProviderFailoverReasonWithPlugin,
matchesProviderContextOverflowWithPlugin,
} from "../../plugins/provider-runtime.js";
import { resolveNodeRequireFromMeta } from "../../logging/node-require.js";
import type { FailoverReason } from "./types.js";
type ProviderErrorPattern = {
@@ -71,15 +68,65 @@ export const PROVIDER_SPECIFIC_PATTERNS: readonly ProviderErrorPattern[] = [
},
];
type ProviderRuntimeHooks = {
classifyProviderFailoverReasonWithPlugin: (params: {
context: { errorMessage: string };
}) => FailoverReason | null;
matchesProviderContextOverflowWithPlugin: (params: {
context: { errorMessage: string };
}) => boolean;
};
const requireProviderRuntime = resolveNodeRequireFromMeta(import.meta.url);
let cachedProviderRuntimeHooks: ProviderRuntimeHooks | null | undefined;
const PROVIDER_CONTEXT_OVERFLOW_SIGNAL_RE =
/\b(?:context|window|prompt|token|tokens|input|request|model)\b/i;
const PROVIDER_CONTEXT_OVERFLOW_ACTION_RE =
/\b(?:too\s+(?:large|long|many)|exceed(?:s|ed|ing)?|overflow|limit|maximum|max)\b/i;
function resolveProviderRuntimeHooks(): ProviderRuntimeHooks | null {
if (cachedProviderRuntimeHooks !== undefined) {
return cachedProviderRuntimeHooks;
}
if (!requireProviderRuntime) {
cachedProviderRuntimeHooks = null;
return cachedProviderRuntimeHooks;
}
try {
const loaded = requireProviderRuntime(
"../../plugins/provider-runtime.js",
) as ProviderRuntimeHooks;
cachedProviderRuntimeHooks = {
classifyProviderFailoverReasonWithPlugin: loaded.classifyProviderFailoverReasonWithPlugin,
matchesProviderContextOverflowWithPlugin: loaded.matchesProviderContextOverflowWithPlugin,
};
} catch {
cachedProviderRuntimeHooks = null;
}
return cachedProviderRuntimeHooks;
}
function looksLikeProviderContextOverflowCandidate(errorMessage: string): boolean {
return (
PROVIDER_CONTEXT_OVERFLOW_SIGNAL_RE.test(errorMessage) &&
PROVIDER_CONTEXT_OVERFLOW_ACTION_RE.test(errorMessage)
);
}
/**
* Check if an error message matches any provider-specific context overflow pattern.
* Called from `isContextOverflowError()` to catch provider-specific wording.
*/
export function matchesProviderContextOverflow(errorMessage: string): boolean {
if (!looksLikeProviderContextOverflowCandidate(errorMessage)) {
return false;
}
const runtimeHooks = resolveProviderRuntimeHooks();
return (
matchesProviderContextOverflowWithPlugin({
runtimeHooks?.matchesProviderContextOverflowWithPlugin({
context: { errorMessage },
}) || PROVIDER_CONTEXT_OVERFLOW_PATTERNS.some((pattern) => pattern.test(errorMessage))
}) === true || PROVIDER_CONTEXT_OVERFLOW_PATTERNS.some((pattern) => pattern.test(errorMessage))
);
}
@@ -88,7 +135,8 @@ export function matchesProviderContextOverflow(errorMessage: string): boolean {
* Returns null if no provider-specific pattern matches (fall through to generic classification).
*/
export function classifyProviderSpecificError(errorMessage: string): FailoverReason | null {
const pluginReason = classifyProviderFailoverReasonWithPlugin({
const runtimeHooks = resolveProviderRuntimeHooks();
const pluginReason = runtimeHooks?.classifyProviderFailoverReasonWithPlugin({
context: { errorMessage },
});
if (pluginReason) {