mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(agents): serve skill instructions whole instead of rejecting read windows (#130286)
* fix(agents): serve skill instructions whole instead of rejecting read windows * test(agents): cover materialized whole skill reads * fix(agents): dedupe repeated whole skill reads * test(agents): prove whole skill read delivery lifecycle Worked on by: - @VACInc Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> * fix(agents): retain skill delivery state on skipped compaction Worked on by: - @VACInc Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> * test(gateway): isolate missing-password auth cases Worked on by: - @VACInc Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> * fix(agents): invalidate skill reads with context replacement Worked on by: - @VACInc Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> * test(agents): implement context replacement hook in session double Worked on by: - @VACInc Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> --------- Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> Co-authored-by: roboclaw-bot <309084314+roboclaw-bot@users.noreply.github.com>
This commit is contained in:
@@ -50,6 +50,7 @@ import { expectReadWriteEditTools } from "./test-helpers/agent-tools-fs-helpers.
|
||||
import { createAgentToolsSandboxContext } from "./test-helpers/agent-tools-sandbox-context.js";
|
||||
import { stubTool } from "./test-helpers/fast-tool-stubs.js";
|
||||
import {
|
||||
createContainerWorkspaceSandboxFsBridge,
|
||||
createHostSandboxFsBridge,
|
||||
createSandboxFsBridgeFromResolver,
|
||||
} from "./test-helpers/host-sandbox-fs-bridge.js";
|
||||
@@ -2774,7 +2775,7 @@ describe("createOpenClawCodingTools read behavior", () => {
|
||||
await fs.writeFile(filePath, "# Demo\ncomplete instructions", "utf8");
|
||||
const sandbox = createAgentToolsSandboxContext({
|
||||
workspaceDir: root,
|
||||
fsBridge: createHostSandboxFsBridge(root),
|
||||
fsBridge: createContainerWorkspaceSandboxFsBridge(root),
|
||||
});
|
||||
const tools = createOpenClawCodingTools({
|
||||
sandbox,
|
||||
@@ -2792,9 +2793,13 @@ describe("createOpenClawCodingTools read behavior", () => {
|
||||
expect(extractToolText(await read.execute("sandbox-skill", { path: relativePath }))).toBe(
|
||||
"# Demo\ncomplete instructions",
|
||||
);
|
||||
await expect(
|
||||
read.execute("sandbox-skill-window", { path: `/workspace/${relativePath}`, cursor: 0 }),
|
||||
).rejects.toThrow(/whole|partial|window/i);
|
||||
for (const window of [{ offset: 2 }, { limit: 1 }, { cursor: 0 }]) {
|
||||
const windowed = await read.execute("sandbox-skill-window", {
|
||||
path: `/workspace/${relativePath}`,
|
||||
...window,
|
||||
});
|
||||
expect(extractToolText(windowed)).toBe("# Demo\ncomplete instructions");
|
||||
}
|
||||
});
|
||||
|
||||
it("reads exact node skill locators without sending them to the filesystem backend", async () => {
|
||||
@@ -2816,14 +2821,132 @@ describe("createOpenClawCodingTools read behavior", () => {
|
||||
const result = await tool.execute("node-skill-read", { path: locator });
|
||||
|
||||
expect(extractToolText(result)).toContain("remote-marker");
|
||||
for (const window of [{ offset: 1 }, { limit: 1 }, { cursor: 0 }]) {
|
||||
await expect(
|
||||
tool.execute("whole-skill-window", { path: locator, ...window }),
|
||||
).rejects.toThrow(/whole|partial|window/i);
|
||||
for (const window of [{ offset: 2 }, { limit: 1 }, { cursor: 0 }]) {
|
||||
const windowed = await tool.execute("whole-skill-window", { path: locator, ...window });
|
||||
expect(extractToolText(windowed)).toContain("# Pond\nremote-marker");
|
||||
}
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("deduplicates sequential and concurrent successful skill reads within one attempt", async () => {
|
||||
const locator = "/skills/pond/SKILL.md";
|
||||
const instructionDeliveryCache = new Map<string, Promise<boolean>>();
|
||||
const instructionDeliveryOptions = { instructionDeliveryCache };
|
||||
const fullResult = {
|
||||
content: [{ type: "text", text: "# Pond\ncomplete instructions" }],
|
||||
details: { kind: "text", content: "# Pond\ncomplete instructions" },
|
||||
} as AgentToolResult<unknown>;
|
||||
let releaseRead = (): void => undefined;
|
||||
const pendingRead = new Promise<AgentToolResult<unknown>>((resolve) => {
|
||||
releaseRead = () => resolve(fullResult);
|
||||
});
|
||||
const execute = vi.fn(() => pendingRead);
|
||||
const tool = wrapReadToolWithSkillContent(
|
||||
{
|
||||
name: "read",
|
||||
label: "read",
|
||||
description: "read a file",
|
||||
parameters: {},
|
||||
execute,
|
||||
} as never,
|
||||
[{ filePath: locator }],
|
||||
instructionDeliveryOptions,
|
||||
);
|
||||
|
||||
const first = tool.execute("first-skill-read", { path: locator });
|
||||
const concurrent = tool.execute("concurrent-skill-read", { path: locator });
|
||||
expect(execute).toHaveBeenCalledTimes(1);
|
||||
releaseRead();
|
||||
|
||||
expect(extractToolText(await first)).toBe("# Pond\ncomplete instructions");
|
||||
expect(extractToolText(await concurrent)).toContain("already served whole");
|
||||
expect(
|
||||
extractToolText(await tool.execute("sequential-skill-read", { path: locator })),
|
||||
).toContain("already served whole");
|
||||
expect(execute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("retries skill reads after failures, whole-read refusals, and optional misses", async () => {
|
||||
const locator = "/skills/pond/SKILL.md";
|
||||
const instructionDeliveryCache = new Map<string, Promise<boolean>>();
|
||||
const instructionDeliveryOptions = { instructionDeliveryCache };
|
||||
const fullResult = {
|
||||
content: [{ type: "text", text: "# Pond\ncomplete instructions" }],
|
||||
details: { kind: "text", content: "# Pond\ncomplete instructions" },
|
||||
} as AgentToolResult<unknown>;
|
||||
const truncatedResult = {
|
||||
content: [{ type: "text", text: "partial" }],
|
||||
details: { kind: "truncated" },
|
||||
} as AgentToolResult<unknown>;
|
||||
const notFoundResult = {
|
||||
content: [{ type: "text", text: `Optional file not found: ${locator}.` }],
|
||||
details: { kind: "not_found", status: "not_found", path: locator, optional: true },
|
||||
} as AgentToolResult<unknown>;
|
||||
const execute = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error("transient read failure"))
|
||||
.mockResolvedValueOnce(truncatedResult)
|
||||
.mockResolvedValueOnce(notFoundResult)
|
||||
.mockResolvedValueOnce(fullResult);
|
||||
const tool = wrapReadToolWithSkillContent(
|
||||
{
|
||||
name: "read",
|
||||
label: "read",
|
||||
description: "read a file",
|
||||
parameters: {},
|
||||
execute,
|
||||
} as never,
|
||||
[{ filePath: locator }],
|
||||
instructionDeliveryOptions,
|
||||
);
|
||||
|
||||
await expect(tool.execute("failed-skill-read", { path: locator })).rejects.toThrow(
|
||||
"transient read failure",
|
||||
);
|
||||
expect(
|
||||
extractToolText(await tool.execute("oversized-skill-read", { path: locator })),
|
||||
).toContain("cannot be partially served");
|
||||
expect(
|
||||
extractToolText(
|
||||
await tool.execute("optional-missing-skill-read", { path: locator, optional: true }),
|
||||
),
|
||||
).toContain("Optional file not found");
|
||||
expect(extractToolText(await tool.execute("retried-skill-read", { path: locator }))).toBe(
|
||||
"# Pond\ncomplete instructions",
|
||||
);
|
||||
expect(execute).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it("serves skill instructions again after model-context compaction invalidates the cache", async () => {
|
||||
const locator = "node://node-1/skills/pond/SKILL.md";
|
||||
const instructionDeliveryCache = new Map<string, Promise<boolean>>();
|
||||
const instructionDeliveryOptions = { instructionDeliveryCache };
|
||||
const tool = wrapReadToolWithSkillContent(
|
||||
{
|
||||
name: "read",
|
||||
label: "read",
|
||||
description: "read a file",
|
||||
parameters: {},
|
||||
execute: vi.fn(),
|
||||
} as never,
|
||||
[{ filePath: locator, readContent: "# Pond\ncomplete instructions" }],
|
||||
instructionDeliveryOptions,
|
||||
);
|
||||
|
||||
expect(extractToolText(await tool.execute("first-skill-read", { path: locator }))).toBe(
|
||||
"# Pond\ncomplete instructions",
|
||||
);
|
||||
expect(extractToolText(await tool.execute("deduped-skill-read", { path: locator }))).toContain(
|
||||
"already served whole",
|
||||
);
|
||||
|
||||
instructionDeliveryCache.clear();
|
||||
|
||||
expect(
|
||||
extractToolText(await tool.execute("post-compaction-skill-read", { path: locator })),
|
||||
).toBe("# Pond\ncomplete instructions");
|
||||
});
|
||||
|
||||
it("uses host decoding only for host-backed sandbox paths", async () => {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sbx-encoding-"));
|
||||
await fs.writeFile(path.join(tmpDir, "notes.txt"), "hello", "utf8");
|
||||
|
||||
@@ -85,6 +85,12 @@ type SkillReadContent = {
|
||||
readContent?: string;
|
||||
};
|
||||
|
||||
export type SkillInstructionDeliveryCache = Map<string, Promise<boolean>>;
|
||||
|
||||
export function createSkillInstructionDeliveryCache(): SkillInstructionDeliveryCache {
|
||||
return new Map();
|
||||
}
|
||||
|
||||
/** Erase a schema-specific session tool only after its input passes that owned schema. */
|
||||
function eraseSessionFileTool<TParameters extends TSchema, TDetails>(
|
||||
tool: AgentTool<TParameters, TDetails>,
|
||||
@@ -1090,6 +1096,7 @@ export function wrapReadToolWithSkillContent(
|
||||
cwd?: string;
|
||||
containerWorkdir?: string;
|
||||
instructionPaths?: readonly string[];
|
||||
instructionDeliveryCache?: SkillInstructionDeliveryCache;
|
||||
},
|
||||
): AnyAgentTool {
|
||||
const cwd = options?.cwd ?? process.cwd();
|
||||
@@ -1119,6 +1126,15 @@ export function wrapReadToolWithSkillContent(
|
||||
if (instructionContent.size === 0) {
|
||||
return tool;
|
||||
}
|
||||
const instructionDeliveryCache = options?.instructionDeliveryCache;
|
||||
const alreadyDeliveredResult = (): AgentToolResult<unknown> => {
|
||||
const text =
|
||||
"Skill instructions were already served whole earlier in the current model context. Reuse that content; the full document will be served again if compaction removes it.";
|
||||
return {
|
||||
content: [{ type: "text", text }],
|
||||
details: { kind: "text", content: text },
|
||||
};
|
||||
};
|
||||
const readContent = (filePath: string): string => {
|
||||
const content = instructionContent.get(filePath);
|
||||
if (content === undefined) {
|
||||
@@ -1136,16 +1152,42 @@ export function wrapReadToolWithSkillContent(
|
||||
const rawPath = record?.path;
|
||||
const normalizedPath =
|
||||
typeof rawPath === "string" ? normalizeFileToolPathParam(rawPath) : undefined;
|
||||
if (!normalizedPath || !instructionContent.has(resolveInstructionPath(normalizedPath))) {
|
||||
const instructionPath = normalizedPath ? resolveInstructionPath(normalizedPath) : undefined;
|
||||
if (!normalizedPath || !instructionPath || !instructionContent.has(instructionPath)) {
|
||||
return tool.execute(toolCallId, args, signal, onUpdate);
|
||||
}
|
||||
if (record && ["offset", "limit", "cursor"].some((key) => record[key] !== undefined)) {
|
||||
throw new Error(
|
||||
"Skill instructions must be read whole; offset, limit, and cursor windows are not allowed.",
|
||||
);
|
||||
for (;;) {
|
||||
const priorDelivery = instructionDeliveryCache?.get(instructionPath);
|
||||
if (!priorDelivery) {
|
||||
break;
|
||||
}
|
||||
const delivered = await priorDelivery;
|
||||
if (instructionDeliveryCache?.get(instructionPath) !== priorDelivery) {
|
||||
continue;
|
||||
}
|
||||
if (delivered) {
|
||||
return alreadyDeliveredResult();
|
||||
}
|
||||
instructionDeliveryCache?.delete(instructionPath);
|
||||
}
|
||||
let settleDelivery = (_delivered: boolean): void => undefined;
|
||||
let delivery: Promise<boolean> | undefined;
|
||||
if (instructionDeliveryCache) {
|
||||
delivery = new Promise<boolean>((resolve) => {
|
||||
settleDelivery = resolve;
|
||||
});
|
||||
// The resolved promise covers sequential and concurrent reads without
|
||||
// changing prior transcript bytes. The compaction owner clears it.
|
||||
instructionDeliveryCache.set(instructionPath, delivery);
|
||||
}
|
||||
const resetDelivery = () => {
|
||||
settleDelivery(false);
|
||||
if (delivery && instructionDeliveryCache?.get(instructionPath) === delivery) {
|
||||
instructionDeliveryCache.delete(instructionPath);
|
||||
}
|
||||
};
|
||||
const instructionTool =
|
||||
typeof instructionContent.get(normalizedPath) === "string"
|
||||
typeof instructionContent.get(instructionPath) === "string"
|
||||
? (virtualRead ??= createOpenClawReadTool(
|
||||
eraseSessionFileTool(
|
||||
createReadTool("/", {
|
||||
@@ -1160,23 +1202,40 @@ export function wrapReadToolWithSkillContent(
|
||||
options,
|
||||
))
|
||||
: tool;
|
||||
const instructionArgs =
|
||||
normalizedPath === rawPath || !record ? args : { ...record, path: normalizedPath };
|
||||
const result = await instructionTool.execute(toolCallId, instructionArgs, signal, onUpdate);
|
||||
const details = result.details;
|
||||
if (
|
||||
details &&
|
||||
typeof details === "object" &&
|
||||
"kind" in details &&
|
||||
details.kind === "truncated"
|
||||
) {
|
||||
const text = `Skill instructions cannot be partially served: the whole document exceeds the ${formatBytes(resolveAdaptiveReadMaxBytes(options))} read budget. Ask the operator to reduce the document or increase the model context.`;
|
||||
return {
|
||||
content: [{ type: "text", text }],
|
||||
details: { kind: "text", content: text },
|
||||
};
|
||||
// Skill instructions are served whole. Some models still send paging arguments,
|
||||
// so windows are dropped rather than rejected.
|
||||
const instructionArgs: Record<string, unknown> = { ...record, path: normalizedPath };
|
||||
for (const key of ["offset", "limit", "cursor"]) {
|
||||
delete instructionArgs[key];
|
||||
}
|
||||
try {
|
||||
const result = await instructionTool.execute(toolCallId, instructionArgs, signal, onUpdate);
|
||||
const details = result.details;
|
||||
const detailsKind =
|
||||
details &&
|
||||
typeof details === "object" &&
|
||||
"kind" in details &&
|
||||
typeof details.kind === "string"
|
||||
? details.kind
|
||||
: undefined;
|
||||
if (detailsKind === "truncated") {
|
||||
resetDelivery();
|
||||
const text = `Skill instructions cannot be partially served: the whole document exceeds the ${formatBytes(resolveAdaptiveReadMaxBytes(options))} read budget. Ask the operator to reduce the document or increase the model context.`;
|
||||
return {
|
||||
content: [{ type: "text", text }],
|
||||
details: { kind: "text", content: text },
|
||||
};
|
||||
}
|
||||
if (detailsKind !== "text") {
|
||||
resetDelivery();
|
||||
return result;
|
||||
}
|
||||
settleDelivery(true);
|
||||
return result;
|
||||
} catch (error) {
|
||||
resetDelivery();
|
||||
throw error;
|
||||
}
|
||||
return result;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -36,7 +36,10 @@ import { bindAssembledAgentToolActionDescriptor } from "./agent-tool-metadata.js
|
||||
import type { ToolOutcomeObserver } from "./agent-tools.before-tool-call.js";
|
||||
import { finalizeAgentTools } from "./agent-tools.finalize.js";
|
||||
import { filterToolsByMessageProvider } from "./agent-tools.message-provider-policy.js";
|
||||
import { wrapToolMemoryFlushAppendOnlyWrite } from "./agent-tools.read.js";
|
||||
import {
|
||||
type SkillInstructionDeliveryCache,
|
||||
wrapToolMemoryFlushAppendOnlyWrite,
|
||||
} from "./agent-tools.read.js";
|
||||
import {
|
||||
getActiveAgentRingZeroTools,
|
||||
mergeAgentRingZeroTools,
|
||||
@@ -317,6 +320,8 @@ type OpenClawCodingToolsOptions = {
|
||||
modelHasVision?: boolean;
|
||||
/** Mutable model-context generation used to expire screenshot coordinate frames. */
|
||||
computerContextEpoch?: { value: number };
|
||||
/** Attempt-local full skill reads that remain visible in the model context. */
|
||||
skillInstructionDeliveryCache?: SkillInstructionDeliveryCache;
|
||||
/** Registers run-owned cleanup for tools that hold node resources. */
|
||||
registerRunCleanup?: (cleanup: (reason: string) => Promise<void>) => void;
|
||||
/** Require explicit message targets (no implicit last-route sends). */
|
||||
@@ -580,6 +585,7 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions)
|
||||
sandbox,
|
||||
skillsSnapshot: options?.skillsSnapshot,
|
||||
skillInstructionPaths: options?.skillUsagePaths?.map((entry) => entry.readPath),
|
||||
skillInstructionDeliveryCache: options?.skillInstructionDeliveryCache,
|
||||
modelContextWindowTokens: options?.modelContextWindowTokens,
|
||||
imageSanitization,
|
||||
modelHasVision: options?.modelHasVision,
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
createSandboxedReadTool,
|
||||
createSandboxedWriteTool,
|
||||
resolveAdaptiveReadMaxBytes,
|
||||
type SkillInstructionDeliveryCache,
|
||||
wrapReadToolWithSkillContent,
|
||||
wrapToolWorkspaceRootGuard,
|
||||
wrapToolWorkspaceRootGuardWithOptions,
|
||||
@@ -75,6 +76,7 @@ type CoreCodingToolsOptions = {
|
||||
sandbox?: SandboxContext;
|
||||
skillsSnapshot?: SkillSnapshot;
|
||||
skillInstructionPaths?: readonly string[];
|
||||
skillInstructionDeliveryCache?: SkillInstructionDeliveryCache;
|
||||
modelContextWindowTokens?: number;
|
||||
imageSanitization?: ImageSanitizationLimits;
|
||||
modelHasVision?: boolean;
|
||||
@@ -164,6 +166,7 @@ export function createCoreCodingTools(options: CoreCodingToolsOptions): AnyAgent
|
||||
cwd: options.codingRoot,
|
||||
containerWorkdir: sandbox?.containerWorkdir,
|
||||
instructionPaths: options.skillInstructionPaths,
|
||||
instructionDeliveryCache: options.skillInstructionDeliveryCache,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1451,6 +1451,34 @@ describe("compactEmbeddedAgentSessionDirect hooks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("creates a distinct skill-instruction delivery cache for each compaction attempt", async () => {
|
||||
await compactEmbeddedAgentSessionDirect({
|
||||
sessionId: "session-1",
|
||||
sessionKey: TEST_SESSION_KEY,
|
||||
sessionFile: TEST_SESSION_KEY,
|
||||
workspaceDir: "/tmp/workspace",
|
||||
});
|
||||
const firstCache = expectRecordFields(
|
||||
mockCallArg(createOpenClawCodingToolsMock),
|
||||
{},
|
||||
).skillInstructionDeliveryCache;
|
||||
|
||||
await compactEmbeddedAgentSessionDirect({
|
||||
sessionId: "session-2",
|
||||
sessionKey: TEST_SESSION_KEY,
|
||||
sessionFile: TEST_SESSION_KEY,
|
||||
workspaceDir: "/tmp/workspace",
|
||||
});
|
||||
const secondCache = expectRecordFields(
|
||||
mockCallArg(createOpenClawCodingToolsMock, 1),
|
||||
{},
|
||||
).skillInstructionDeliveryCache;
|
||||
|
||||
expect(firstCache).toBeInstanceOf(Map);
|
||||
expect(secondCache).toBeInstanceOf(Map);
|
||||
expect(secondCache).not.toBe(firstCache);
|
||||
});
|
||||
|
||||
it("skips runtime tool construction when the compaction model does not support tools", async () => {
|
||||
mockResolvedModel({ supportsTools: false });
|
||||
|
||||
|
||||
@@ -30,6 +30,7 @@ import { createBundleLspToolRuntime } from "../agent-bundle-lsp-runtime.js";
|
||||
import { createBundleMcpToolRuntime } from "../agent-bundle-mcp-tools.js";
|
||||
import { resolveSessionAgentIds } from "../agent-scope.js";
|
||||
import { createOpenClawCodingTools } from "../agent-tools.js";
|
||||
import { createSkillInstructionDeliveryCache } from "../agent-tools.read.js";
|
||||
import { listActiveProcessSessionReferences } from "../bash-process-references.js";
|
||||
import { resolveProcessToolScopeKey } from "../bash-process-scope.js";
|
||||
import {
|
||||
@@ -328,6 +329,7 @@ export async function buildPreparedCompactionRuntime(prepared: DirectCompactionP
|
||||
pluginMetadataSnapshot: params.preparedModelRuntime.metadataSnapshot,
|
||||
});
|
||||
const toolsEnabled = supportsModelTools(effectiveModel);
|
||||
const skillInstructionDeliveryCache = createSkillInstructionDeliveryCache();
|
||||
const toolsRaw = toolsEnabled
|
||||
? createOpenClawCodingTools({
|
||||
exec: {
|
||||
@@ -374,6 +376,7 @@ export async function buildPreparedCompactionRuntime(prepared: DirectCompactionP
|
||||
modelContextWindowTokens: contextTokenBudget,
|
||||
skillsSnapshot: skillsSnapshotForRun,
|
||||
skillUsagePaths,
|
||||
skillInstructionDeliveryCache,
|
||||
conversationCapabilityProfile: runtimeCapabilityProfile,
|
||||
preparedModelRuntime: params.preparedModelRuntime,
|
||||
modelAuthMode: resolveModelAuthMode(effectiveModel.provider, params.config, undefined, {
|
||||
|
||||
@@ -40,6 +40,7 @@ vi.mock("./attempt-timeout-prepare.js", () => ({
|
||||
prepareEmbeddedAttemptTimeout: mocks.prepareTimeout,
|
||||
}));
|
||||
|
||||
import { agentSessionSetContextReplacementHook } from "../../sessions/agent-session-compaction.js";
|
||||
import { runEmbeddedAttemptExecutionPhase } from "./attempt-execution-phase.js";
|
||||
|
||||
type ExecutionInput = Parameters<typeof runEmbeddedAttemptExecutionPhase>[0];
|
||||
@@ -72,7 +73,9 @@ function createFixture(
|
||||
getRunAbortDeadlineAtMs: vi.fn(() => 123),
|
||||
clearTimers: vi.fn(),
|
||||
};
|
||||
const setContextReplacementHook = vi.fn();
|
||||
const activeSession = {
|
||||
[agentSessionSetContextReplacementHook]: setContextReplacementHook,
|
||||
agent: { streamFn: vi.fn() },
|
||||
dispose: vi.fn(),
|
||||
isCompacting: false,
|
||||
@@ -97,6 +100,7 @@ function createFixture(
|
||||
terminal: { kind: "ok" as const },
|
||||
trajectoryEndRecorded: false,
|
||||
};
|
||||
const skillInstructionDeliveryCache = new Map([["skill", Promise.resolve(true)]]);
|
||||
const sessionRuntime = {
|
||||
agentSession: {
|
||||
activeSession,
|
||||
@@ -145,7 +149,7 @@ function createFixture(
|
||||
bundleTools: {},
|
||||
sessionRuntime,
|
||||
systemPrompt: { runtimeChannel: "telegram" },
|
||||
toolBase: { toolSearchTargetTranscriptProjections: new Map() },
|
||||
toolBase: { skillInstructionDeliveryCache, toolSearchTargetTranscriptProjections: new Map() },
|
||||
toolCatalog: {
|
||||
toolSearchRunPlan: {
|
||||
capabilityToolNames: new Set(["read"]),
|
||||
@@ -240,6 +244,8 @@ function createFixture(
|
||||
result,
|
||||
runAbort,
|
||||
sessionManager,
|
||||
setContextReplacementHook,
|
||||
skillInstructionDeliveryCache,
|
||||
setToolSearchCatalogExecutor,
|
||||
state,
|
||||
streamResult,
|
||||
@@ -261,6 +267,11 @@ describe("runEmbeddedAttemptExecutionPhase", () => {
|
||||
const result = await runEmbeddedAttemptExecutionPhase(fixture.input);
|
||||
|
||||
expect(result).toBe(fixture.result);
|
||||
expect(fixture.setContextReplacementHook).toHaveBeenCalledOnce();
|
||||
const replacementHook = fixture.setContextReplacementHook.mock.calls[0]?.[0];
|
||||
expect(replacementHook).toEqual(expect.any(Function));
|
||||
replacementHook?.();
|
||||
expect(fixture.skillInstructionDeliveryCache.size).toBe(0);
|
||||
expect(fixture.order).toEqual([
|
||||
"guards",
|
||||
"stream-ready",
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
projectAgentRunAttemptTerminal,
|
||||
type AgentRunAttemptTerminal,
|
||||
} from "../../agent-run-terminal-outcome.js";
|
||||
import { agentSessionSetContextReplacementHook } from "../../sessions/agent-session-compaction.js";
|
||||
import { log } from "../logger.js";
|
||||
import type { EmbeddedAgentQueueHandle } from "../runs.js";
|
||||
import { flushPendingToolResultsAfterIdle } from "../wait-for-idle-before-flush.js";
|
||||
@@ -58,6 +59,9 @@ export async function runEmbeddedAttemptExecutionPhase(
|
||||
toolCatalog.toolSearchRunPlan;
|
||||
const { runtimeChannel } = systemPrompt;
|
||||
const { toolSearchTargetTranscriptProjections } = toolBase;
|
||||
activeSession[agentSessionSetContextReplacementHook](() =>
|
||||
toolBase.skillInstructionDeliveryCache.clear(),
|
||||
);
|
||||
const hookAgentId = input.setup.sessionAgentId;
|
||||
let repairedRejectedProviderReplay = false;
|
||||
const diagnosticOwner = createDiagnosticEmbeddedRunOwner({
|
||||
|
||||
@@ -29,6 +29,7 @@ import type {
|
||||
MessagingToolSourceReplyPayload,
|
||||
} from "../../embedded-agent-messaging.types.js";
|
||||
import type { AgentMessage, StreamFn } from "../../runtime/index.js";
|
||||
import { agentSessionSetContextReplacementHook } from "../../sessions/agent-session-compaction.js";
|
||||
import {
|
||||
getModelRegistryRuntime,
|
||||
initializeModelRegistryRuntime,
|
||||
@@ -970,6 +971,7 @@ type MutableSession = {
|
||||
abort: () => Promise<void>;
|
||||
dispose: () => void;
|
||||
steer: (text: string) => Promise<void>;
|
||||
[agentSessionSetContextReplacementHook]: (callback: (() => void) | undefined) => void;
|
||||
};
|
||||
|
||||
type SessionPromptOverride = (
|
||||
@@ -1222,6 +1224,7 @@ export function createDefaultEmbeddedSession(params?: {
|
||||
abort: async () => {},
|
||||
dispose: () => {},
|
||||
steer: async () => {},
|
||||
[agentSessionSetContextReplacementHook]: () => {},
|
||||
};
|
||||
|
||||
return session;
|
||||
|
||||
@@ -12,6 +12,7 @@ import { extractModelCompat } from "../../../plugins/provider-model-compat.js";
|
||||
import { getPluginToolMeta } from "../../../plugins/tools.js";
|
||||
import { isSubagentSessionKey } from "../../../routing/session-key.js";
|
||||
import { createOpenClawCodingTools } from "../../agent-tools.js";
|
||||
import { createSkillInstructionDeliveryCache } from "../../agent-tools.read.js";
|
||||
import { getChannelAgentToolMeta } from "../../channel-tools.js";
|
||||
import type { CodeModeSkill } from "../../code-mode-skills.js";
|
||||
import { resolveConversationCapabilityProfile } from "../../conversation-capability-profile.js";
|
||||
@@ -140,6 +141,7 @@ export function prepareEmbeddedAttemptToolBase(params: {
|
||||
// Compaction summaries omit screenshot image blocks. Frames are bound to this
|
||||
// generation so retained tool-result text cannot authorize stale coordinates.
|
||||
const computerContextEpoch: ComputerContextEpoch = { value: 0 };
|
||||
const skillInstructionDeliveryCache = createSkillInstructionDeliveryCache();
|
||||
const toolSearchCatalogRef =
|
||||
toolSearchControlsEnabledForRun || codeModeControlsEnabledForRun
|
||||
? createToolSearchCatalogRef()
|
||||
@@ -339,6 +341,7 @@ export function prepareEmbeddedAttemptToolBase(params: {
|
||||
hasRepliedRef: attempt.hasRepliedRef,
|
||||
modelHasVision: attempt.model.input?.includes("image") ?? false,
|
||||
computerContextEpoch,
|
||||
skillInstructionDeliveryCache,
|
||||
registerRunCleanup: (cleanup) => runCleanups.push(cleanup),
|
||||
requireExplicitMessageTarget:
|
||||
attempt.requireExplicitMessageTarget ?? isSubagentSessionKey(attempt.sessionKey),
|
||||
@@ -394,6 +397,7 @@ export function prepareEmbeddedAttemptToolBase(params: {
|
||||
codeModeControlsEnabledForRun,
|
||||
codeModeSkills,
|
||||
computerContextEpoch,
|
||||
skillInstructionDeliveryCache,
|
||||
cronCreatorToolAllowlist,
|
||||
cronCreatorToolAllowlistCaptureRef,
|
||||
effectiveToolsAllow,
|
||||
|
||||
@@ -7,6 +7,7 @@ import type {
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { MAX_OVERFLOW_COMPACTION_ATTEMPTS } from "../agent-compaction-constants.js";
|
||||
import { subscribeEmbeddedAgentSession } from "../embedded-agent-subscribe.js";
|
||||
import { agentSessionSetContextReplacementHook } from "./agent-session-compaction.js";
|
||||
import {
|
||||
createAssistant,
|
||||
createAssistantResultStream,
|
||||
@@ -289,6 +290,39 @@ describe("AgentSession compaction", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("invalidates context-bound state before the completed event and overflow retry", async () => {
|
||||
const contextState = new Map([["skill", true]]);
|
||||
const contextSizesAtCompactionEnd: number[] = [];
|
||||
let agentRequests = 0;
|
||||
streamMocks.streamSimple.mockImplementation((activeModel: Model) => {
|
||||
agentRequests += 1;
|
||||
if (agentRequests === 2) {
|
||||
expect(contextState.size).toBe(0);
|
||||
}
|
||||
return createAssistantResultStream(
|
||||
agentRequests === 1
|
||||
? createOverflowAssistant(activeModel)
|
||||
: createAssistant(activeModel, [{ type: "text", text: "complete retry" }]),
|
||||
);
|
||||
});
|
||||
const { session } = await createTestSession({
|
||||
settingsManager: createAutoCompactionSettings(),
|
||||
resourceLoader: createResourceLoader(createCompactionHandlers()),
|
||||
});
|
||||
session[agentSessionSetContextReplacementHook](() => contextState.clear());
|
||||
session.subscribe((event) => {
|
||||
if (event.type === "compaction_end" && event.outcome.status === "completed") {
|
||||
contextSizesAtCompactionEnd.push(contextState.size);
|
||||
}
|
||||
});
|
||||
|
||||
await session.prompt("long request");
|
||||
|
||||
expect(agentRequests).toBe(2);
|
||||
expect(contextState.size).toBe(0);
|
||||
expect(contextSizesAtCompactionEnd).toEqual([0]);
|
||||
});
|
||||
|
||||
it("surfaces the shared overflow recovery limit after exhausting it", async () => {
|
||||
let agentRequests = 0;
|
||||
streamMocks.streamSimple.mockImplementation((activeModel: Model) => {
|
||||
|
||||
@@ -37,7 +37,18 @@ export const agentSessionAutomaticCompaction: unique symbol = Symbol.for(
|
||||
"openclaw.agent-session.automatic-compaction",
|
||||
);
|
||||
|
||||
/** Installs a synchronous callback for model-context replacement during compaction. */
|
||||
export const agentSessionSetContextReplacementHook: unique symbol = Symbol.for(
|
||||
"openclaw.agent-session.set-context-replacement-hook",
|
||||
);
|
||||
|
||||
export abstract class AgentSessionCompaction extends AgentSessionInspection {
|
||||
private onContextReplaced?: () => void;
|
||||
|
||||
[agentSessionSetContextReplacementHook](callback: (() => void) | undefined): void {
|
||||
this.onContextReplaced = callback;
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
// Compaction
|
||||
// =========================================================================
|
||||
@@ -258,6 +269,9 @@ export abstract class AgentSessionCompaction extends AgentSessionInspection {
|
||||
// Compaction replaces the request prefix, invalidating retained usage and thinking signatures.
|
||||
// Sanitize at assignment so every continuation driver receives replay-safe history.
|
||||
this.agent.state.messages = sanitizeCompactionReplayMessages(sessionContext.messages);
|
||||
// The retry loop can continue before queued subscribers observe compaction_end.
|
||||
// Invalidate context-bound state synchronously with the authoritative replacement.
|
||||
this.onContextReplaced?.();
|
||||
|
||||
const savedCompactionEntry = newEntries.find(
|
||||
(e) => e.type === "compaction" && e.summary === compactionResult.summary,
|
||||
|
||||
@@ -1196,6 +1196,7 @@ describe("gateway auth", () => {
|
||||
mode: "password",
|
||||
password: { source: "exec", provider: "op", id: "pw" } as never,
|
||||
},
|
||||
env: {},
|
||||
});
|
||||
expect(() =>
|
||||
assertGatewayAuthConfigured(auth, {
|
||||
@@ -1227,7 +1228,7 @@ describe("gateway auth", () => {
|
||||
});
|
||||
|
||||
it("throws generic error when password mode has no password at all", () => {
|
||||
const auth = resolveGatewayAuth({ authConfig: { mode: "password" } });
|
||||
const auth = resolveGatewayAuth({ authConfig: { mode: "password" }, env: {} });
|
||||
expect(() => assertGatewayAuthConfigured(auth, { mode: "password" })).toThrow(
|
||||
"gateway auth mode is password, but no password was configured",
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user