From 053affffecec45aa88fea29e123b69236b4bc1ab Mon Sep 17 00:00:00 2001 From: Marcus Castro Date: Tue, 10 Feb 2026 11:47:28 -0300 Subject: [PATCH] fix(cron): pass agent-level skill filter to isolated cron sessions Isolated cron sessions called buildWorkspaceSkillSnapshot without the skillFilter parameter, causing all skills to be included even when an agent had a restricted skills list via agents.list[].skills. Resolves the filter using resolveAgentSkillsFilter and passes it through, aligning isolated cron with main session behavior. Fixes #10804 --- .../isolated-agent/run.skill-filter.test.ts | 256 ++++++++++++++++++ src/cron/isolated-agent/run.ts | 5 +- 2 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 src/cron/isolated-agent/run.skill-filter.test.ts diff --git a/src/cron/isolated-agent/run.skill-filter.test.ts b/src/cron/isolated-agent/run.skill-filter.test.ts new file mode 100644 index 0000000000..6463e4fe81 --- /dev/null +++ b/src/cron/isolated-agent/run.skill-filter.test.ts @@ -0,0 +1,256 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// ---------- mocks ---------- + +const buildWorkspaceSkillSnapshotMock = vi.fn(); +const resolveAgentSkillsFilterMock = vi.fn(); + +vi.mock("../../agents/agent-scope.js", () => ({ + resolveAgentConfig: vi.fn().mockReturnValue(undefined), + resolveAgentDir: vi.fn().mockReturnValue("/tmp/agent-dir"), + resolveAgentModelFallbacksOverride: vi.fn().mockReturnValue(undefined), + resolveAgentWorkspaceDir: vi.fn().mockReturnValue("/tmp/workspace"), + resolveDefaultAgentId: vi.fn().mockReturnValue("default"), + resolveAgentSkillsFilter: resolveAgentSkillsFilterMock, +})); + +vi.mock("../../agents/skills.js", () => ({ + buildWorkspaceSkillSnapshot: buildWorkspaceSkillSnapshotMock, +})); + +vi.mock("../../agents/skills/refresh.js", () => ({ + getSkillsSnapshotVersion: vi.fn().mockReturnValue(42), +})); + +vi.mock("../../agents/workspace.js", () => ({ + ensureAgentWorkspace: vi.fn().mockResolvedValue({ dir: "/tmp/workspace" }), +})); + +vi.mock("../../agents/model-catalog.js", () => ({ + loadModelCatalog: vi.fn().mockResolvedValue({ models: [] }), +})); + +vi.mock("../../agents/model-selection.js", () => ({ + getModelRefStatus: vi.fn().mockReturnValue({ allowed: false }), + isCliProvider: vi.fn().mockReturnValue(false), + resolveAllowedModelRef: vi.fn().mockReturnValue({ ref: { provider: "openai", model: "gpt-4" } }), + resolveConfiguredModelRef: vi.fn().mockReturnValue({ provider: "openai", model: "gpt-4" }), + resolveHooksGmailModel: vi.fn().mockReturnValue(null), + resolveThinkingDefault: vi.fn().mockReturnValue(undefined), +})); + +vi.mock("../../agents/model-fallback.js", () => ({ + runWithModelFallback: vi.fn().mockResolvedValue({ + result: { + payloads: [{ text: "test output" }], + meta: { agentMeta: { usage: { input: 10, output: 20 } } }, + }, + provider: "openai", + model: "gpt-4", + }), +})); + +vi.mock("../../agents/pi-embedded.js", () => ({ + runEmbeddedPiAgent: vi.fn().mockResolvedValue({ + payloads: [{ text: "test output" }], + meta: { agentMeta: { usage: { input: 10, output: 20 } } }, + }), +})); + +vi.mock("../../agents/context.js", () => ({ + lookupContextTokens: vi.fn().mockReturnValue(128000), +})); + +vi.mock("../../agents/date-time.js", () => ({ + formatUserTime: vi.fn().mockReturnValue("2026-02-10 12:00"), + resolveUserTimeFormat: vi.fn().mockReturnValue("24h"), + resolveUserTimezone: vi.fn().mockReturnValue("UTC"), +})); + +vi.mock("../../agents/timeout.js", () => ({ + resolveAgentTimeoutMs: vi.fn().mockReturnValue(60_000), +})); + +vi.mock("../../agents/usage.js", () => ({ + deriveSessionTotalTokens: vi.fn().mockReturnValue(30), + hasNonzeroUsage: vi.fn().mockReturnValue(false), +})); + +vi.mock("../../agents/subagent-announce.js", () => ({ + runSubagentAnnounceFlow: vi.fn().mockResolvedValue(true), +})); + +vi.mock("../../agents/cli-runner.js", () => ({ + runCliAgent: vi.fn(), +})); + +vi.mock("../../agents/cli-session.js", () => ({ + getCliSessionId: vi.fn().mockReturnValue(undefined), + setCliSessionId: vi.fn(), +})); + +vi.mock("../../auto-reply/thinking.js", () => ({ + normalizeThinkLevel: vi.fn().mockReturnValue(undefined), + normalizeVerboseLevel: vi.fn().mockReturnValue("off"), + supportsXHighThinking: vi.fn().mockReturnValue(false), +})); + +vi.mock("../../cli/outbound-send-deps.js", () => ({ + createOutboundSendDeps: vi.fn().mockReturnValue({}), +})); + +vi.mock("../../config/sessions.js", () => ({ + resolveAgentMainSessionKey: vi.fn().mockReturnValue("main:default"), + resolveSessionTranscriptPath: vi.fn().mockReturnValue("/tmp/transcript.jsonl"), + updateSessionStore: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../routing/session-key.js", () => ({ + buildAgentMainSessionKey: vi.fn().mockReturnValue("agent:default:cron:test"), + normalizeAgentId: vi.fn((id: string) => id), +})); + +vi.mock("../../infra/agent-events.js", () => ({ + registerAgentRunContext: vi.fn(), +})); + +vi.mock("../../infra/outbound/deliver.js", () => ({ + deliverOutboundPayloads: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../infra/skills-remote.js", () => ({ + getRemoteSkillEligibility: vi.fn().mockReturnValue({}), +})); + +vi.mock("../../logger.js", () => ({ + logWarn: vi.fn(), +})); + +vi.mock("../../security/external-content.js", () => ({ + buildSafeExternalPrompt: vi.fn().mockReturnValue("safe prompt"), + detectSuspiciousPatterns: vi.fn().mockReturnValue([]), + getHookType: vi.fn().mockReturnValue("unknown"), + isExternalHookSession: vi.fn().mockReturnValue(false), +})); + +vi.mock("../delivery.js", () => ({ + resolveCronDeliveryPlan: vi.fn().mockReturnValue({ requested: false }), +})); + +vi.mock("./delivery-target.js", () => ({ + resolveDeliveryTarget: vi.fn().mockResolvedValue({ + channel: "discord", + to: undefined, + accountId: undefined, + error: undefined, + }), +})); + +vi.mock("./helpers.js", () => ({ + isHeartbeatOnlyResponse: vi.fn().mockReturnValue(false), + pickLastDeliverablePayload: vi.fn().mockReturnValue(undefined), + pickLastNonEmptyTextFromPayloads: vi.fn().mockReturnValue("test output"), + pickSummaryFromOutput: vi.fn().mockReturnValue("summary"), + pickSummaryFromPayloads: vi.fn().mockReturnValue("summary"), + resolveHeartbeatAckMaxChars: vi.fn().mockReturnValue(100), +})); + +const resolveCronSessionMock = vi.fn(); +vi.mock("./session.js", () => ({ + resolveCronSession: resolveCronSessionMock, +})); + +vi.mock("../../agents/defaults.js", () => ({ + DEFAULT_CONTEXT_TOKENS: 128000, + DEFAULT_MODEL: "gpt-4", + DEFAULT_PROVIDER: "openai", +})); + +// ---------- helpers ---------- + +function makeJob(overrides?: Record) { + return { + id: "test-job", + name: "Test Job", + schedule: { kind: "cron", expr: "0 9 * * *", tz: "UTC" }, + sessionTarget: "isolated", + payload: { kind: "agentTurn", message: "test" }, + ...overrides, + } as never; +} + +function makeParams(overrides?: Record) { + return { + cfg: {}, + deps: {} as never, + job: makeJob(), + message: "test", + sessionKey: "cron:test", + ...overrides, + }; +} + +// ---------- tests ---------- + +describe("runCronIsolatedAgentTurn — skill filter", () => { + beforeEach(() => { + vi.clearAllMocks(); + buildWorkspaceSkillSnapshotMock.mockReturnValue({ + prompt: "", + resolvedSkills: [], + version: 42, + }); + resolveAgentSkillsFilterMock.mockReturnValue(undefined); + // Fresh session object per test — prevents mutation leaking between tests + resolveCronSessionMock.mockReturnValue({ + storePath: "/tmp/store.json", + store: {}, + sessionEntry: { + sessionId: "test-session-id", + updatedAt: 0, + systemSent: false, + skillsSnapshot: undefined, + }, + systemSent: false, + isNewSession: true, + }); + }); + + it("passes agent-level skillFilter to buildWorkspaceSkillSnapshot", async () => { + resolveAgentSkillsFilterMock.mockReturnValue(["meme-factory", "weather"]); + + const { runCronIsolatedAgentTurn } = await import("./run.js"); + + const result = await runCronIsolatedAgentTurn( + makeParams({ + cfg: { agents: { list: [{ id: "scout", skills: ["meme-factory", "weather"] }] } }, + agentId: "scout", + }), + ); + + expect(result.status).toBe("ok"); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledOnce(); + expect(buildWorkspaceSkillSnapshotMock.mock.calls[0][1]).toHaveProperty("skillFilter", [ + "meme-factory", + "weather", + ]); + }); + + it("omits skillFilter when agent has no skills config", async () => { + resolveAgentSkillsFilterMock.mockReturnValue(undefined); + + const { runCronIsolatedAgentTurn } = await import("./run.js"); + + const result = await runCronIsolatedAgentTurn( + makeParams({ + cfg: { agents: { list: [{ id: "general" }] } }, + agentId: "general", + }), + ); + + expect(result.status).toBe("ok"); + expect(buildWorkspaceSkillSnapshotMock).toHaveBeenCalledOnce(); + // When no skills config, skillFilter should be undefined (no filtering applied) + expect(buildWorkspaceSkillSnapshotMock.mock.calls[0][1].skillFilter).toBeUndefined(); + }); +}); diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index 116507f8e4..f4f36f031d 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -6,6 +6,7 @@ import { resolveAgentConfig, resolveAgentDir, resolveAgentModelFallbacksOverride, + resolveAgentSkillsFilter, resolveAgentWorkspaceDir, resolveDefaultAgentId, } from "../../agents/agent-scope.js"; @@ -520,7 +521,7 @@ export async function runCronIsolatedAgentTurn(params: { `${commandBody}\n\nReturn your summary as plain text; it will be delivered automatically. If the task explicitly calls for messaging a specific external recipient, note who/where it should go instead of sending it yourself.`.trim(); } - let skillsSnapshot = cronSession.sessionEntry.skillsSnapshot; +let skillsSnapshot = cronSession.sessionEntry.skillsSnapshot; if (isFastTestEnv) { // Fast unit-test mode: avoid scanning the workspace and writing session stores. skillsSnapshot = skillsSnapshot ?? { prompt: "", skills: [] }; @@ -529,9 +530,11 @@ export async function runCronIsolatedAgentTurn(params: { const skillsSnapshotVersion = getSkillsSnapshotVersion(workspaceDir); const needsSkillsSnapshot = !existingSnapshot || existingSnapshot.version !== skillsSnapshotVersion; + const skillFilter = resolveAgentSkillsFilter(cfgWithAgentDefaults, agentId); if (needsSkillsSnapshot) { skillsSnapshot = buildWorkspaceSkillSnapshot(workspaceDir, { config: cfgWithAgentDefaults, + skillFilter, eligibility: { remote: getRemoteSkillEligibility() }, snapshotVersion: skillsSnapshotVersion, });