From f4e20f806e344095c3bd2c720aaecb98d90ab324 Mon Sep 17 00:00:00 2001 From: "clawsweeper[bot]" <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 08:34:34 +0100 Subject: [PATCH] fix(agents): avoid duplicate Claude CLI skill prompts Fix Claude CLI skill prompt handling so native skill plugin materialization is prepared before prompt suppression, with the prompt fallback preserved when plugin args are unavailable. Also keeps direct prepared-run callers covered by an execute-time fallback. Fixes #87063. Co-authored-by: uday --- docs/gateway/cli-backends.md | 15 +- .../agent-command.live-model-switch.test.ts | 4 +- src/agents/cli-runner/claude-skills-plugin.ts | 14 +- src/agents/cli-runner/execute.ts | 27 +- src/agents/cli-runner/prepare.test.ts | 271 +++++++++++++++++- src/agents/cli-runner/prepare.ts | 24 +- src/agents/cli-runner/types.ts | 1 + 7 files changed, 326 insertions(+), 30 deletions(-) diff --git a/docs/gateway/cli-backends.md b/docs/gateway/cli-backends.md index 72fc6bb4c84d..37bd9a86f7cf 100644 --- a/docs/gateway/cli-backends.md +++ b/docs/gateway/cli-backends.md @@ -161,13 +161,14 @@ told us OpenClaw-style Claude CLI usage is allowed again, so OpenClaw treats a new policy. -The bundled Anthropic `claude-cli` backend receives the OpenClaw skills snapshot -two ways: the compact OpenClaw skills catalog in the appended system prompt, and -a temporary Claude Code plugin passed with `--plugin-dir`. The plugin contains -only the eligible skills for that agent/session, so Claude Code's native skill -resolver sees the same filtered set that OpenClaw would otherwise advertise in -the prompt. Skill env/API key overrides are still applied by OpenClaw to the -child process environment for the run. +The bundled Anthropic `claude-cli` backend prefers Claude Code's native skill +resolver for OpenClaw skills. When the current skills snapshot includes at least +one selected skill with a materialized path, OpenClaw passes a temporary Claude +Code plugin with `--plugin-dir` and omits the duplicate OpenClaw skills catalog +from the appended system prompt. If the snapshot has no materialized plugin +skill, OpenClaw keeps the prompt catalog as a fallback. Skill env/API key +overrides are still applied by OpenClaw to the child process environment for the +run. Claude CLI also has its own noninteractive permission mode. OpenClaw maps that to the existing exec policy instead of adding Claude-specific policy config. diff --git a/src/agents/agent-command.live-model-switch.test.ts b/src/agents/agent-command.live-model-switch.test.ts index a34b06b94a18..7860f1e9ce92 100644 --- a/src/agents/agent-command.live-model-switch.test.ts +++ b/src/agents/agent-command.live-model-switch.test.ts @@ -203,7 +203,7 @@ vi.mock("../config/sessions.js", () => ({ mergeSessionEntry: (a: unknown, b: unknown) => ({ ...(a as object), ...(b as object) }), updateSessionStore: vi.fn( async (_path: string, fn: (store: Record) => unknown) => { - const store: Record = {}; + const store = (state.sessionStoreMock ?? {}) as Record; return fn(store); }, ), @@ -1023,7 +1023,7 @@ describe("agentCommand – LiveSessionModelSwitchError retry", () => { expect(attemptCalls[0]?.sessionEntry).toStrictEqual(visibleEntry); expect(state.persistSessionEntryMock).not.toHaveBeenCalled(); expect(state.updateSessionStoreAfterAgentRunMock).not.toHaveBeenCalled(); - expect(sessionStore["agent:main:main"]).toBe(visibleEntry); + expect(sessionStore["agent:main:main"]).toEqual(visibleEntry); }); it("does not duplicate finishing lifecycle when an attempt already emitted finishing", async () => { diff --git a/src/agents/cli-runner/claude-skills-plugin.ts b/src/agents/cli-runner/claude-skills-plugin.ts index 93cab6fdeaf3..715e61b619d6 100644 --- a/src/agents/cli-runner/claude-skills-plugin.ts +++ b/src/agents/cli-runner/claude-skills-plugin.ts @@ -1,3 +1,4 @@ +import { accessSync } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js"; @@ -30,6 +31,15 @@ function sanitizeSkillDirName(name: string, used: Set): string { return candidate; } +export function isClaudeCliSkillFileAccessible(skillFilePath: string): boolean { + try { + accessSync(skillFilePath); + return true; + } catch { + return false; + } +} + async function collectClaudePluginSkills(snapshot?: SkillSnapshot): Promise { const skills = snapshot?.resolvedSkills ?? []; if (skills.length === 0) { @@ -44,9 +54,7 @@ async function collectClaudePluginSkills(snapshot?: SkillSnapshot): Promise entry.replaceAll("{sessionId}", resolvedSessionId ?? "")) : baseArgs; - const claudeSkillsPlugin = await prepareClaudeCliSkillsPlugin({ - backendId: context.backendResolved.id, - skillsSnapshot: params.skillsSnapshot, - }); - let claudeSkillsPluginCleanupOwned = false; + const fallbackClaudeSkillsPlugin = + context.claudeSkillsPluginArgs === undefined + ? await prepareClaudeCliSkillsPlugin({ + backendId: context.backendResolved.id, + skillsSnapshot: params.skillsSnapshot, + }) + : undefined; + let fallbackClaudeSkillsPluginCleanupOwned = false; + const claudeSkillsPluginArgs = + context.claudeSkillsPluginArgs ?? fallbackClaudeSkillsPlugin?.args ?? []; const baseArgsWithSkills = - claudeSkillsPlugin.args.length > 0 - ? [...resolvedArgs, ...claudeSkillsPlugin.args] - : resolvedArgs; + claudeSkillsPluginArgs.length > 0 ? [...resolvedArgs, ...claudeSkillsPluginArgs] : resolvedArgs; const executionBaseArgs = context.backendResolved.resolveExecutionArgs?.({ config: params.config, @@ -453,7 +456,7 @@ export async function executePreparedCliRun( model: context.modelId, backend: context.backendResolved.id, }); - claudeSkillsPluginCleanupOwned = true; + fallbackClaudeSkillsPluginCleanupOwned = true; const ownedPreparedBackendCleanup = context.preparedBackend.cleanup; context.preparedBackend.cleanup = undefined; const liveResult = await runClaudeLiveSessionTurn({ @@ -482,7 +485,7 @@ export async function executePreparedCliRun( }, cleanup: async () => { try { - await claudeSkillsPlugin.cleanup(); + await fallbackClaudeSkillsPlugin?.cleanup(); } finally { await ownedPreparedBackendCleanup?.(); } @@ -753,8 +756,8 @@ export async function executePreparedCliRun( } }); } finally { - if (!claudeSkillsPluginCleanupOwned) { - await claudeSkillsPlugin.cleanup(); + if (!fallbackClaudeSkillsPluginCleanupOwned) { + await fallbackClaudeSkillsPlugin?.cleanup(); } if (systemPromptFile) { await systemPromptFile.cleanup(); diff --git a/src/agents/cli-runner/prepare.test.ts b/src/agents/cli-runner/prepare.test.ts index e3176332fb08..faeed1ed6b8d 100644 --- a/src/agents/cli-runner/prepare.test.ts +++ b/src/agents/cli-runner/prepare.test.ts @@ -205,6 +205,10 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { ), resolveMcpLoopbackScopedTools: vi.fn(() => ({ agentId: "main", tools: [] })), resolveOpenClawReferencePaths: vi.fn(async () => ({ docsPath: null, sourcePath: null })), + prepareClaudeCliSkillsPlugin: vi.fn(async () => ({ + args: [], + cleanup: vi.fn(async () => undefined), + })), }); mockGetGlobalHookRunner.mockReturnValue(null); getRuntimeConfigMock.mockReturnValue({}); @@ -1094,7 +1098,6 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { }, ], }); - const context = await prepareCliRunContext({ sessionId: "session-test", sessionKey: "agent:main:test", @@ -1185,7 +1188,6 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { }, ], }); - const context = await prepareCliRunContext({ sessionId: "session-test", sessionKey: "agent:main:test", @@ -1246,7 +1248,6 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { }, ], }); - const context = await prepareCliRunContext({ sessionId: "session-test", sessionKey: "agent:main:telegram:group:chat123", @@ -1459,6 +1460,270 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { } }); + it("omits Claude CLI prompt skills when the native skills plugin can carry them", async () => { + const { dir, sessionFile } = createSessionFile(); + const skillDir = path.join(dir, "skills", "weather"); + fs.mkdirSync(skillDir, { recursive: true }); + const skillFilePath = path.join(skillDir, "SKILL.md"); + fs.writeFileSync( + skillFilePath, + [ + "---", + "name: weather", + "description: Use weather tools for forecasts.", + "---", + "", + "Read forecast data before replying.", + ].join("\n"), + "utf-8", + ); + + try { + cliBackendsTesting.setDepsForTest({ + resolvePluginSetupCliBackend: () => undefined, + resolveRuntimeCliBackends: () => [ + { + id: "claude-cli", + pluginId: "anthropic", + bundleMcp: false, + config: { + command: "claude", + args: ["--print"], + output: "jsonl", + input: "stdin", + sessionMode: "existing", + }, + }, + ], + }); + setCliRunnerPrepareTestDeps({ + prepareClaudeCliSkillsPlugin: vi.fn(async () => ({ + args: ["--plugin-dir", path.join(dir, "openclaw-skills")], + cleanup: vi.fn(async () => undefined), + pluginDir: path.join(dir, "openclaw-skills"), + })), + }); + + const context = await prepareCliRunContext({ + sessionId: "session-test", + sessionFile, + workspaceDir: dir, + prompt: "latest ask", + provider: "claude-cli", + model: "opus", + timeoutMs: 1_000, + runId: "run-claude-plugin-skills-prompt", + config: createCliBackendConfig({ systemPromptOverride: null }), + skillsSnapshot: { + prompt: [ + "", + " ", + " weather", + " Use weather tools for forecasts.", + ` ${skillFilePath}`, + " ", + "", + ].join("\n"), + skills: [{ name: "weather" }], + resolvedSkills: [ + { + name: "weather", + description: "Use weather tools for forecasts.", + filePath: skillFilePath, + baseDir: skillDir, + source: "test", + sourceInfo: { + path: skillDir, + source: "test", + scope: "project", + origin: "top-level", + baseDir: skillDir, + }, + disableModelInvocation: false, + }, + ], + }, + }); + + expect(context.systemPrompt).not.toContain(""); + expect(context.systemPrompt).not.toContain("weather"); + expect(context.systemPromptReport.skills.promptChars).toBe(0); + expect(context.claudeSkillsPluginArgs).toEqual([ + "--plugin-dir", + path.join(dir, "openclaw-skills"), + ]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("keeps Claude CLI prompt skills when the snapshot has no materialized plugin skills", async () => { + const { dir, sessionFile } = createSessionFile(); + const missingSkillDir = path.join(dir, "skills", "missing"); + const missingSkillFilePath = path.join(missingSkillDir, "SKILL.md"); + + try { + cliBackendsTesting.setDepsForTest({ + resolvePluginSetupCliBackend: () => undefined, + resolveRuntimeCliBackends: () => [ + { + id: "claude-cli", + pluginId: "anthropic", + bundleMcp: false, + config: { + command: "claude", + args: ["--print"], + output: "jsonl", + input: "stdin", + sessionMode: "existing", + }, + }, + ], + }); + + const context = await prepareCliRunContext({ + sessionId: "session-test", + sessionFile, + workspaceDir: dir, + prompt: "latest ask", + provider: "claude-cli", + model: "opus", + timeoutMs: 1_000, + runId: "run-claude-plugin-skills-prompt-fallback", + config: createCliBackendConfig({ systemPromptOverride: null }), + skillsSnapshot: { + prompt: [ + "", + " ", + " weather", + " Use weather tools for forecasts.", + ` ${missingSkillFilePath}`, + " ", + "", + ].join("\n"), + skills: [{ name: "weather" }], + resolvedSkills: [ + { + name: "weather", + description: "Use weather tools for forecasts.", + filePath: missingSkillFilePath, + baseDir: missingSkillDir, + source: "test", + sourceInfo: { + path: missingSkillDir, + source: "test", + scope: "project", + origin: "top-level", + baseDir: missingSkillDir, + }, + disableModelInvocation: false, + }, + ], + }, + }); + + expect(context.systemPrompt).toContain(""); + expect(context.systemPrompt).toContain("weather"); + expect(context.systemPromptReport.skills.promptChars).toBeGreaterThan(0); + expect(context.claudeSkillsPluginArgs).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("keeps Claude CLI prompt skills when plugin materialization produces no args", async () => { + const { dir, sessionFile } = createSessionFile(); + const skillDir = path.join(dir, "skills", "weather"); + fs.mkdirSync(skillDir, { recursive: true }); + const skillFilePath = path.join(skillDir, "SKILL.md"); + fs.writeFileSync( + skillFilePath, + [ + "---", + "name: weather", + "description: Use weather tools for forecasts.", + "---", + "", + "Read forecast data before replying.", + ].join("\n"), + "utf-8", + ); + + try { + cliBackendsTesting.setDepsForTest({ + resolvePluginSetupCliBackend: () => undefined, + resolveRuntimeCliBackends: () => [ + { + id: "claude-cli", + pluginId: "anthropic", + bundleMcp: false, + config: { + command: "claude", + args: ["--print"], + output: "jsonl", + input: "stdin", + sessionMode: "existing", + }, + }, + ], + }); + setCliRunnerPrepareTestDeps({ + prepareClaudeCliSkillsPlugin: vi.fn(async () => ({ + args: [], + cleanup: vi.fn(async () => undefined), + })), + }); + + const context = await prepareCliRunContext({ + sessionId: "session-test", + sessionFile, + workspaceDir: dir, + prompt: "latest ask", + provider: "claude-cli", + model: "opus", + timeoutMs: 1_000, + runId: "run-claude-plugin-skills-prompt-materialization-fallback", + config: createCliBackendConfig({ systemPromptOverride: null }), + skillsSnapshot: { + prompt: [ + "", + " ", + " weather", + " Use weather tools for forecasts.", + ` ${skillFilePath}`, + " ", + "", + ].join("\n"), + skills: [{ name: "weather" }], + resolvedSkills: [ + { + name: "weather", + description: "Use weather tools for forecasts.", + filePath: skillFilePath, + baseDir: skillDir, + source: "test", + sourceInfo: { + path: skillDir, + source: "test", + scope: "project", + origin: "top-level", + baseDir: skillDir, + }, + disableModelInvocation: false, + }, + ], + }, + }); + + expect(context.systemPrompt).toContain(""); + expect(context.systemPrompt).toContain("weather"); + expect(context.systemPromptReport.skills.promptChars).toBeGreaterThan(0); + expect(context.claudeSkillsPluginArgs).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it("does not probe the transcript for non-claude-cli providers", async () => { const { dir, sessionFile } = createSessionFile(); try { diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 3c929ca09c27..7b6eecdb9807 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -59,6 +59,7 @@ import { buildSystemPromptReport } from "../system-prompt-report.js"; import { appendModelIdentitySystemPrompt } from "../system-prompt.js"; import { redactRunIdentifier, resolveRunWorkspaceDir } from "../workspace-run.js"; import { prepareCliBundleMcpConfig } from "./bundle-mcp.js"; +import { prepareClaudeCliSkillsPlugin } from "./claude-skills-plugin.js"; import { buildCliAgentSystemPrompt, normalizeCliModel } from "./helpers.js"; import { cliBackendLog } from "./log.js"; import { @@ -81,6 +82,7 @@ const prepareDeps = { resolveOpenClawReferencePaths: async ( params: Parameters[0], ) => (await import("../docs-path.js")).resolveOpenClawReferencePaths(params), + prepareClaudeCliSkillsPlugin, // Surfaced as a dep so tests can stub the on-disk Claude CLI transcript probe // without touching ~/.claude/projects. claudeCliSessionTranscriptHasContent, @@ -310,6 +312,20 @@ export async function prepareCliRunContext( } } : undefined; + const claudeSkillsPlugin = await prepareDeps.prepareClaudeCliSkillsPlugin({ + backendId: backendResolved.id, + skillsSnapshot: params.skillsSnapshot, + }); + const preparedCleanup = + preparedBackendCleanup || claudeSkillsPlugin.args.length > 0 + ? async () => { + try { + await claudeSkillsPlugin.cleanup(); + } finally { + await preparedBackendCleanup?.(); + } + } + : undefined; const preparedBackendClearEnv = [ ...(preparedBackend.backend.clearEnv ?? []), ...(preparedExecution?.clearEnv ?? []), @@ -323,7 +339,7 @@ export async function prepareCliRunContext( : {}), }, ...(preparedBackendEnv ? { env: preparedBackendEnv } : {}), - ...(preparedBackendCleanup ? { cleanup: preparedBackendCleanup } : {}), + ...(preparedCleanup ? { cleanup: preparedCleanup } : {}), }; const promptTools = bundleMcpEnabled && mcpLoopbackRuntime @@ -405,6 +421,7 @@ export async function prepareCliRunContext( config: params.config, agentId: sessionAgentId, }); + const systemPromptSkillsPrompt = claudeSkillsPlugin.args.length > 0 ? "" : skillsPrompt; const builtSystemPrompt = resolveSystemPromptOverride({ config: params.config, @@ -421,7 +438,7 @@ export async function prepareCliRunContext( heartbeatPrompt, docsPath: openClawReferences.docsPath ?? undefined, sourcePath: openClawReferences.sourcePath ?? undefined, - skillsPrompt, + skillsPrompt: systemPromptSkillsPrompt, tools: promptTools, contextFiles, modelDisplay, @@ -531,7 +548,7 @@ export async function prepareCliRunContext( systemPrompt, bootstrapFiles, injectedFiles: contextFiles, - skillsPrompt, + skillsPrompt: systemPromptSkillsPrompt, tools: promptTools, currentTurn: { ...(params.currentInboundEventKind ? { kind: params.currentInboundEventKind } : {}), @@ -595,6 +612,7 @@ export async function prepareCliRunContext( contextWindowInfo, systemPrompt, systemPromptReport, + claudeSkillsPluginArgs: claudeSkillsPlugin.args, bootstrapPromptWarningLines: bootstrapPromptWarning.lines, ...(openClawHistoryPrompt ? { openClawHistoryPrompt } : {}), heartbeatPrompt, diff --git a/src/agents/cli-runner/types.ts b/src/agents/cli-runner/types.ts index 3b0bd9888a57..b54d5f2e5864 100644 --- a/src/agents/cli-runner/types.ts +++ b/src/agents/cli-runner/types.ts @@ -135,6 +135,7 @@ export type PreparedCliRunContext = { contextWindowInfo?: ContextWindowInfo; systemPrompt: string; systemPromptReport: SessionSystemPromptReport; + claudeSkillsPluginArgs?: string[] | undefined; bootstrapPromptWarningLines: string[]; openClawHistoryPrompt?: string; heartbeatPrompt?: string;