Plugins/Hooks: avoid duplicate before_agent_start executions

This commit is contained in:
Vignesh Natarajan
2026-02-21 22:31:51 -08:00
parent 96c985400d
commit 542fc169d2
7 changed files with 174 additions and 41 deletions

View File

@@ -30,6 +30,7 @@ Docs: https://docs.openclaw.ai
- TUI/Status: request immediate renders after setting `sending`/`waiting` activity states so in-flight runs always show visible progress indicators instead of appearing idle until completion. (#21549) Thanks @13Guinness.
- Agents/Fallbacks: treat JSON payloads with `type: "api_error"` + `"Internal server error"` as transient failover errors so Anthropic 500-style failures trigger model fallback. (#23193) Thanks @jarvis-lane.
- Agents/Diagnostics: include resolved lifecycle error text in `embedded run agent end` warnings so UI/TUI “Connection error” runs expose actionable provider failure reasons in gateway logs. (#23054) Thanks @Raize.
- Plugins/Hooks: run legacy `before_agent_start` once per agent turn and reuse that result across model-resolve and prompt-build compatibility paths, preventing duplicate hook side effects (for example duplicate external API calls). (#23289) Thanks @ksato8710.
- Gateway/Pairing: treat operator.admin pairing tokens as satisfying operator.write requests so legacy devices stop looping through scope-upgrade prompts introduced in 2026.2.19. (#23125, #23006) Thanks @vignesh07.
- Gateway/Pairing: treat `operator.admin` as satisfying other `operator.*` scope checks during device-auth verification so local CLI/TUI sessions stop entering pairing-required loops for pairing/approval-scoped commands. (#22062, #22193, #21191) Thanks @Botaccess, @jhartshorn, and @ctbritt.
- Gateway/Pairing: preserve existing approved token scopes when processing repair pairings that omit `scopes`, preventing empty-scope token regressions on reconnecting clients. (#21906) Thanks @paki81.

View File

@@ -1,5 +1,16 @@
import { vi } from "vitest";
export const mockedGlobalHookRunner = {
hasHooks: vi.fn(() => false),
runBeforeAgentStart: vi.fn(async () => undefined),
runBeforePromptBuild: vi.fn(async () => undefined),
runBeforeModelResolve: vi.fn(async () => undefined),
};
vi.mock("../../plugins/hook-runner-global.js", () => ({
getGlobalHookRunner: vi.fn(() => mockedGlobalHookRunner),
}));
vi.mock("../auth-profiles.js", () => ({
isProfileInCooldown: vi.fn(() => false),
markAuthProfileFailure: vi.fn(async () => {}),

View File

@@ -4,6 +4,7 @@ import { pickFallbackThinkingLevel } from "../pi-embedded-helpers.js";
import { compactEmbeddedPiSessionDirect } from "./compact.js";
import { runEmbeddedPiAgent } from "./run.js";
import { makeAttemptResult, mockOverflowRetrySuccess } from "./run.overflow-compaction.fixture.js";
import { mockedGlobalHookRunner } from "./run.overflow-compaction.mocks.shared.js";
import { runEmbeddedAttempt } from "./run/attempt.js";
import type { EmbeddedRunAttemptResult } from "./run/types.js";
import {
@@ -22,6 +23,36 @@ const mockedPickFallbackThinkingLevel = vi.mocked(pickFallbackThinkingLevel);
describe("runEmbeddedPiAgent overflow compaction trigger routing", () => {
beforeEach(() => {
vi.clearAllMocks();
mockedGlobalHookRunner.hasHooks.mockImplementation(() => false);
});
it("passes precomputed legacy before_agent_start result into the attempt", async () => {
const legacyResult = {
modelOverride: "legacy-model",
prependContext: "legacy context",
};
mockedGlobalHookRunner.hasHooks.mockImplementation(
(hookName) => hookName === "before_agent_start",
);
mockedGlobalHookRunner.runBeforeAgentStart.mockResolvedValueOnce(legacyResult);
mockedRunEmbeddedAttempt.mockResolvedValueOnce(makeAttemptResult({ promptError: null }));
await runEmbeddedPiAgent({
sessionId: "test-session",
sessionKey: "test-key",
sessionFile: "/tmp/session.json",
workspaceDir: "/tmp/workspace",
prompt: "hello",
timeoutMs: 30000,
runId: "run-legacy-pass-through",
});
expect(mockedGlobalHookRunner.runBeforeAgentStart).toHaveBeenCalledTimes(1);
expect(mockedRunEmbeddedAttempt).toHaveBeenCalledWith(
expect.objectContaining({
legacyBeforeAgentStartResult: legacyResult,
}),
);
});
it("passes trigger=overflow when retrying compaction after context overflow", async () => {

View File

@@ -1,6 +1,7 @@
import fs from "node:fs/promises";
import type { ThinkLevel } from "../../auto-reply/thinking.js";
import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js";
import type { PluginHookBeforeAgentStartResult } from "../../plugins/types.js";
import { enqueueCommandInLane } from "../../process/command-queue.js";
import { isMarkdownCapableMessageChannel } from "../../utils/message-channel.js";
import { resolveOpenClawAgentDir } from "../agent-paths.js";
@@ -236,6 +237,7 @@ export async function runEmbeddedPiAgent(
// Legacy compatibility: before_agent_start is also checked for override
// fields if present. New hook takes precedence when both are set.
let modelResolveOverride: { providerOverride?: string; modelOverride?: string } | undefined;
let legacyBeforeAgentStartResult: PluginHookBeforeAgentStartResult | undefined;
const hookRunner = getGlobalHookRunner();
const hookCtx = {
agentId: workspaceResolution.agentId,
@@ -256,14 +258,16 @@ export async function runEmbeddedPiAgent(
}
if (hookRunner?.hasHooks("before_agent_start")) {
try {
const legacyResult = await hookRunner.runBeforeAgentStart(
legacyBeforeAgentStartResult = await hookRunner.runBeforeAgentStart(
{ prompt: params.prompt },
hookCtx,
);
modelResolveOverride = {
providerOverride:
modelResolveOverride?.providerOverride ?? legacyResult?.providerOverride,
modelOverride: modelResolveOverride?.modelOverride ?? legacyResult?.modelOverride,
modelResolveOverride?.providerOverride ??
legacyBeforeAgentStartResult?.providerOverride,
modelOverride:
modelResolveOverride?.modelOverride ?? legacyBeforeAgentStartResult?.modelOverride,
};
} catch (hookErr) {
log.warn(
@@ -564,6 +568,7 @@ export async function runEmbeddedPiAgent(
authStorage,
modelRegistry,
agentId: workspaceResolution.agentId,
legacyBeforeAgentStartResult,
thinkLevel,
verboseLevel: params.verboseLevel,
reasoningLevel: params.reasoningLevel,

View File

@@ -1,7 +1,7 @@
import type { AgentMessage } from "@mariozechner/pi-agent-core";
import type { ImageContent } from "@mariozechner/pi-ai";
import { describe, expect, it } from "vitest";
import { injectHistoryImagesIntoMessages } from "./attempt.js";
import { describe, expect, it, vi } from "vitest";
import { injectHistoryImagesIntoMessages, resolvePromptBuildHookResult } from "./attempt.js";
describe("injectHistoryImagesIntoMessages", () => {
const image: ImageContent = { type: "image", data: "abc", mimeType: "image/png" };
@@ -58,3 +58,51 @@ describe("injectHistoryImagesIntoMessages", () => {
expect(firstAssistant?.content).toBe("noop");
});
});
describe("resolvePromptBuildHookResult", () => {
it("reuses precomputed legacy before_agent_start result without invoking hook again", async () => {
const hookRunner = {
hasHooks: vi.fn(
(hookName: "before_prompt_build" | "before_agent_start") =>
hookName === "before_agent_start",
),
runBeforePromptBuild: vi.fn(async () => undefined),
runBeforeAgentStart: vi.fn(async () => ({ prependContext: "from-hook" })),
};
const result = await resolvePromptBuildHookResult({
prompt: "hello",
messages: [],
hookCtx: {},
hookRunner,
legacyBeforeAgentStartResult: { prependContext: "from-cache", systemPrompt: "legacy-system" },
});
expect(hookRunner.runBeforeAgentStart).not.toHaveBeenCalled();
expect(result).toEqual({
prependContext: "from-cache",
systemPrompt: "legacy-system",
});
});
it("calls legacy hook when precomputed result is absent", async () => {
const hookRunner = {
hasHooks: vi.fn(
(hookName: "before_prompt_build" | "before_agent_start") =>
hookName === "before_agent_start",
),
runBeforePromptBuild: vi.fn(async () => undefined),
runBeforeAgentStart: vi.fn(async () => ({ prependContext: "from-hook" })),
};
const messages = [{ role: "user", content: "ctx" }];
const result = await resolvePromptBuildHookResult({
prompt: "hello",
messages,
hookCtx: {},
hookRunner,
});
expect(hookRunner.runBeforeAgentStart).toHaveBeenCalledTimes(1);
expect(hookRunner.runBeforeAgentStart).toHaveBeenCalledWith({ prompt: "hello", messages }, {});
expect(result.prependContext).toBe("from-hook");
});
});

View File

@@ -14,6 +14,11 @@ import { resolveChannelCapabilities } from "../../../config/channel-capabilities
import { getMachineDisplayName } from "../../../infra/machine-name.js";
import { MAX_IMAGE_BYTES } from "../../../media/constants.js";
import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js";
import type {
PluginHookAgentContext,
PluginHookBeforeAgentStartResult,
PluginHookBeforePromptBuildResult,
} from "../../../plugins/types.js";
import {
isCronSessionKey,
isSubagentSessionKey,
@@ -111,6 +116,18 @@ import {
import { detectAndLoadPromptImages } from "./images.js";
import type { EmbeddedRunAttemptParams, EmbeddedRunAttemptResult } from "./types.js";
type PromptBuildHookRunner = {
hasHooks: (hookName: "before_prompt_build" | "before_agent_start") => boolean;
runBeforePromptBuild: (
event: { prompt: string; messages: unknown[] },
ctx: PluginHookAgentContext,
) => Promise<PluginHookBeforePromptBuildResult | undefined>;
runBeforeAgentStart: (
event: { prompt: string; messages: unknown[] },
ctx: PluginHookAgentContext,
) => Promise<PluginHookBeforeAgentStartResult | undefined>;
};
export function injectHistoryImagesIntoMessages(
messages: AgentMessage[],
historyImagesByIndex: Map<number, ImageContent[]>,
@@ -159,6 +176,53 @@ export function injectHistoryImagesIntoMessages(
return didMutate;
}
export async function resolvePromptBuildHookResult(params: {
prompt: string;
messages: unknown[];
hookCtx: PluginHookAgentContext;
hookRunner?: PromptBuildHookRunner | null;
legacyBeforeAgentStartResult?: PluginHookBeforeAgentStartResult;
}): Promise<PluginHookBeforePromptBuildResult> {
const promptBuildResult = params.hookRunner?.hasHooks("before_prompt_build")
? await params.hookRunner
.runBeforePromptBuild(
{
prompt: params.prompt,
messages: params.messages,
},
params.hookCtx,
)
.catch((hookErr: unknown) => {
log.warn(`before_prompt_build hook failed: ${String(hookErr)}`);
return undefined;
})
: undefined;
const legacyResult =
params.legacyBeforeAgentStartResult ??
(params.hookRunner?.hasHooks("before_agent_start")
? await params.hookRunner
.runBeforeAgentStart(
{
prompt: params.prompt,
messages: params.messages,
},
params.hookCtx,
)
.catch((hookErr: unknown) => {
log.warn(
`before_agent_start hook (legacy prompt build path) failed: ${String(hookErr)}`,
);
return undefined;
})
: undefined);
return {
systemPrompt: promptBuildResult?.systemPrompt ?? legacyResult?.systemPrompt,
prependContext: [promptBuildResult?.prependContext, legacyResult?.prependContext]
.filter((value): value is string => Boolean(value))
.join("\n\n"),
};
}
function summarizeMessagePayload(msg: AgentMessage): { textChars: number; imageBlocks: number } {
const content = (msg as { content?: unknown }).content;
if (typeof content === "string") {
@@ -934,42 +998,13 @@ export async function runEmbeddedAttempt(
workspaceDir: params.workspaceDir,
messageProvider: params.messageProvider ?? undefined,
};
const promptBuildResult = hookRunner?.hasHooks("before_prompt_build")
? await hookRunner
.runBeforePromptBuild(
{
prompt: params.prompt,
messages: activeSession.messages,
},
hookCtx,
)
.catch((hookErr: unknown) => {
log.warn(`before_prompt_build hook failed: ${String(hookErr)}`);
return undefined;
})
: undefined;
const legacyResult = hookRunner?.hasHooks("before_agent_start")
? await hookRunner
.runBeforeAgentStart(
{
prompt: params.prompt,
messages: activeSession.messages,
},
hookCtx,
)
.catch((hookErr: unknown) => {
log.warn(
`before_agent_start hook (legacy prompt build path) failed: ${String(hookErr)}`,
);
return undefined;
})
: undefined;
const hookResult = {
systemPrompt: promptBuildResult?.systemPrompt ?? legacyResult?.systemPrompt,
prependContext: [promptBuildResult?.prependContext, legacyResult?.prependContext]
.filter((value): value is string => Boolean(value))
.join("\n\n"),
};
const hookResult = await resolvePromptBuildHookResult({
prompt: params.prompt,
messages: activeSession.messages,
hookCtx,
hookRunner,
legacyBeforeAgentStartResult: params.legacyBeforeAgentStartResult,
});
{
if (hookResult?.prependContext) {
effectivePrompt = `${hookResult.prependContext}\n\n${params.prompt}`;

View File

@@ -2,6 +2,7 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core";
import type { Api, AssistantMessage, Model } from "@mariozechner/pi-ai";
import type { ThinkLevel } from "../../../auto-reply/thinking.js";
import type { SessionSystemPromptReport } from "../../../config/sessions/types.js";
import type { PluginHookBeforeAgentStartResult } from "../../../plugins/types.js";
import type { MessagingToolSend } from "../../pi-embedded-messaging.js";
import type { AuthStorage, ModelRegistry } from "../../pi-model-discovery.js";
import type { NormalizedUsage } from "../../usage.js";
@@ -19,6 +20,7 @@ export type EmbeddedRunAttemptParams = EmbeddedRunAttemptBase & {
authStorage: AuthStorage;
modelRegistry: ModelRegistry;
thinkLevel: ThinkLevel;
legacyBeforeAgentStartResult?: PluginHookBeforeAgentStartResult;
};
export type EmbeddedRunAttemptResult = {