fix(cron): refresh isolated skill snapshots when filter changes (#13457) (thanks @mcaxtr)
This commit is contained in:
@@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Telegram: retry inbound media `getFile` calls (3 attempts with backoff) and gracefully fall back to placeholder-only processing when retries fail, preventing dropped voice/media messages on transient Telegram network errors. (#16154) Thanks @yinghaosang.
|
||||
- Telegram: finalize streaming preview replies in place instead of sending a second final message, preventing duplicate Telegram assistant outputs at stream completion. (#17218) Thanks @obviyus.
|
||||
- Cron: infer `payload.kind="agentTurn"` for model-only `cron.update` payload patches, so partial agent-turn updates do not fail validation when `kind` is omitted. (#15664) Thanks @rodrigouroz.
|
||||
- Cron/Agents: honor per-agent `skills` allowlists for isolated cron runs and refresh cached skill snapshots when the agent skill filter changes, preventing stale unrestricted skill prompts. (#13457) Thanks @mcaxtr.
|
||||
- Subagents: use child-run-based deterministic announce idempotency keys across direct and queued delivery paths (with legacy queued-item fallback) to prevent duplicate announce retries without collapsing distinct same-millisecond announces. (#17150) Thanks @widingmarcus-cyber.
|
||||
- Discord: ensure role allowlist matching uses raw role IDs for message routing authorization. Thanks @xinhuagu.
|
||||
|
||||
|
||||
@@ -82,6 +82,8 @@ export type SkillEligibilityContext = {
|
||||
export type SkillSnapshot = {
|
||||
prompt: string;
|
||||
skills: Array<{ name: string; primaryEnv?: string }>;
|
||||
/** Normalized agent-level filter used to build this snapshot; undefined means unrestricted. */
|
||||
skillFilter?: string[];
|
||||
resolvedSkills?: Skill[];
|
||||
version?: number;
|
||||
};
|
||||
|
||||
@@ -232,12 +232,17 @@ export function buildWorkspaceSkillSnapshot(
|
||||
const resolvedSkills = promptEntries.map((entry) => entry.skill);
|
||||
const remoteNote = opts?.eligibility?.remote?.note?.trim();
|
||||
const prompt = [remoteNote, formatSkillsForPrompt(resolvedSkills)].filter(Boolean).join("\n");
|
||||
const skillFilter =
|
||||
opts?.skillFilter === undefined
|
||||
? undefined
|
||||
: opts.skillFilter.map((entry) => String(entry).trim()).filter(Boolean);
|
||||
return {
|
||||
prompt,
|
||||
skills: eligible.map((entry) => ({
|
||||
name: entry.skill.name,
|
||||
primaryEnv: entry.metadata?.primaryEnv,
|
||||
})),
|
||||
skillFilter,
|
||||
resolvedSkills,
|
||||
version: opts?.snapshotVersion,
|
||||
};
|
||||
|
||||
@@ -144,6 +144,8 @@ export type GroupKeyResolution = {
|
||||
export type SessionSkillSnapshot = {
|
||||
prompt: string;
|
||||
skills: Array<{ name: string; primaryEnv?: string }>;
|
||||
/** Normalized agent-level filter used to build this snapshot; undefined means unrestricted. */
|
||||
skillFilter?: string[];
|
||||
resolvedSkills?: Skill[];
|
||||
version?: number;
|
||||
};
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// ---------- mocks ----------
|
||||
|
||||
@@ -193,6 +193,16 @@ function makeParams(overrides?: Record<string, unknown>) {
|
||||
// ---------- tests ----------
|
||||
|
||||
describe("runCronIsolatedAgentTurn — skill filter", () => {
|
||||
const originalFastEnv = process.env.OPENCLAW_TEST_FAST;
|
||||
|
||||
beforeAll(() => {
|
||||
process.env.OPENCLAW_TEST_FAST = "0";
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
process.env.OPENCLAW_TEST_FAST = originalFastEnv;
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
buildWorkspaceSkillSnapshotMock.mockReturnValue({
|
||||
@@ -271,4 +281,72 @@ describe("runCronIsolatedAgentTurn — skill filter", () => {
|
||||
// Explicit empty skills list should forward [] to filter out all skills
|
||||
expect(buildWorkspaceSkillSnapshotMock.mock.calls[0][1]).toHaveProperty("skillFilter", []);
|
||||
});
|
||||
|
||||
it("refreshes cached snapshot when skillFilter changes without version bump", async () => {
|
||||
resolveAgentSkillsFilterMock.mockReturnValue(["weather"]);
|
||||
resolveCronSessionMock.mockReturnValue({
|
||||
storePath: "/tmp/store.json",
|
||||
store: {},
|
||||
sessionEntry: {
|
||||
sessionId: "test-session-id",
|
||||
updatedAt: 0,
|
||||
systemSent: false,
|
||||
skillsSnapshot: {
|
||||
prompt: "<available_skills><skill>meme-factory</skill></available_skills>",
|
||||
skills: [{ name: "meme-factory" }],
|
||||
version: 42,
|
||||
},
|
||||
},
|
||||
systemSent: false,
|
||||
isNewSession: true,
|
||||
});
|
||||
|
||||
const { runCronIsolatedAgentTurn } = await import("./run.js");
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(
|
||||
makeParams({
|
||||
cfg: { agents: { list: [{ id: "weather-bot", skills: ["weather"] }] } },
|
||||
agentId: "weather-bot",
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.status).toBe("ok");
|
||||
expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledOnce();
|
||||
expect(buildWorkspaceSkillSnapshotMock.mock.calls[0][1]).toHaveProperty("skillFilter", [
|
||||
"weather",
|
||||
]);
|
||||
});
|
||||
|
||||
it("reuses cached snapshot when version and skillFilter are unchanged", async () => {
|
||||
resolveAgentSkillsFilterMock.mockReturnValue(["weather", "meme-factory"]);
|
||||
resolveCronSessionMock.mockReturnValue({
|
||||
storePath: "/tmp/store.json",
|
||||
store: {},
|
||||
sessionEntry: {
|
||||
sessionId: "test-session-id",
|
||||
updatedAt: 0,
|
||||
systemSent: false,
|
||||
skillsSnapshot: {
|
||||
prompt: "<available_skills><skill>weather</skill></available_skills>",
|
||||
skills: [{ name: "weather" }],
|
||||
skillFilter: ["meme-factory", "weather"],
|
||||
version: 42,
|
||||
},
|
||||
},
|
||||
systemSent: false,
|
||||
isNewSession: true,
|
||||
});
|
||||
|
||||
const { runCronIsolatedAgentTurn } = await import("./run.js");
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(
|
||||
makeParams({
|
||||
cfg: { agents: { list: [{ id: "weather-bot", skills: ["weather", "meme-factory"] }] } },
|
||||
agentId: "weather-bot",
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.status).toBe("ok");
|
||||
expect(buildWorkspaceSkillSnapshotMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -105,6 +105,27 @@ const CRON_SUBAGENT_WAIT_POLL_MS = 500;
|
||||
const CRON_SUBAGENT_WAIT_MIN_MS = 30_000;
|
||||
const CRON_SUBAGENT_FINAL_REPLY_GRACE_MS = 5_000;
|
||||
|
||||
function normalizeSkillFilterForSnapshot(skillFilter?: string[]): string[] | undefined {
|
||||
if (skillFilter === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
return Array.from(
|
||||
new Set(skillFilter.map((entry) => String(entry).trim()).filter(Boolean)),
|
||||
).toSorted();
|
||||
}
|
||||
|
||||
function matchesCachedSkillFilter(cached?: string[], next?: string[]): boolean {
|
||||
const cachedNormalized = normalizeSkillFilterForSnapshot(cached);
|
||||
const nextNormalized = normalizeSkillFilterForSnapshot(next);
|
||||
if (cachedNormalized === undefined || nextNormalized === undefined) {
|
||||
return cachedNormalized === nextNormalized;
|
||||
}
|
||||
if (cachedNormalized.length !== nextNormalized.length) {
|
||||
return false;
|
||||
}
|
||||
return cachedNormalized.every((entry, index) => entry === nextNormalized[index]);
|
||||
}
|
||||
|
||||
function isLikelyInterimCronMessage(value: string): boolean {
|
||||
const text = value.trim();
|
||||
if (!text) {
|
||||
@@ -528,9 +549,11 @@ export async function runCronIsolatedAgentTurn(params: {
|
||||
} else {
|
||||
const existingSnapshot = cronSession.sessionEntry.skillsSnapshot;
|
||||
const skillsSnapshotVersion = getSkillsSnapshotVersion(workspaceDir);
|
||||
const needsSkillsSnapshot =
|
||||
!existingSnapshot || existingSnapshot.version !== skillsSnapshotVersion;
|
||||
const skillFilter = resolveAgentSkillsFilter(cfgWithAgentDefaults, agentId);
|
||||
const needsSkillsSnapshot =
|
||||
!existingSnapshot ||
|
||||
existingSnapshot.version !== skillsSnapshotVersion ||
|
||||
!matchesCachedSkillFilter(existingSnapshot.skillFilter, skillFilter);
|
||||
if (needsSkillsSnapshot) {
|
||||
skillsSnapshot = buildWorkspaceSkillSnapshot(workspaceDir, {
|
||||
config: cfgWithAgentDefaults,
|
||||
|
||||
Reference in New Issue
Block a user