diff --git a/coverage.txt b/coverage.txt index f4563bf..6ac6716 100644 --- a/coverage.txt +++ b/coverage.txt @@ -29,13 +29,12 @@ ℹ urlFilter.js | 100.00 | 93.75 | 100.00 | ℹ scheduler | | | | ℹ cronInstaller.js | 57.60 | 29.17 | 85.71 | 22-24 54-58 77-122 139-141 143-145 156-158 160-162 182-184 186-188 190-207 211-212 -ℹ index.js | 100.00 | 100.00 | 100.00 | ℹ logger.js | 86.96 | 70.00 | 100.00 | 36-40 63-66 ℹ matcher.js | 100.00 | 100.00 | 100.00 | ℹ parser.js | 100.00 | 94.12 | 100.00 | ℹ queue.js | 100.00 | 100.00 | 100.00 | ℹ runner.js | 100.00 | 100.00 | 100.00 | -ℹ scheduler.js | 100.00 | 93.55 | 91.67 | +ℹ scheduler.js | 98.86 | 91.18 | 91.67 | 170-171 ℹ session | | | | ℹ checkpointer.js | 82.22 | 87.50 | 50.00 | 22 24 39-43 45 ℹ factory.js | 100.00 | 100.00 | 100.00 | @@ -50,7 +49,7 @@ ℹ clarify.js | 100.00 | 94.44 | 80.00 | ℹ code.js | 100.00 | 89.13 | 92.31 | ℹ common.js | 100.00 | 93.33 | 83.33 | -ℹ cron.js | 100.00 | 97.30 | 90.00 | +ℹ cron.js | 98.69 | 93.41 | 75.00 | 70-71 83-84 205-206 ℹ date.js | 100.00 | 100.00 | 100.00 | ℹ filesystem.js | 94.50 | 86.79 | 79.17 | 44-45 107-110 170-177 187-188 196-202 397-398 415-419 422-423 ℹ image.js | 97.90 | 95.83 | 50.00 | 92-94 @@ -73,6 +72,6 @@ ℹ messages.js | 100.00 | 94.44 | 100.00 | ℹ panels.js | 100.00 | 100.00 | 100.00 | ℹ ------------------------------------------------------------------------------------------------------------------------------------------ -ℹ all files | 94.55 | 87.96 | 84.52 | +ℹ all files | 94.52 | 87.78 | 83.91 | ℹ ------------------------------------------------------------------------------------------------------------------------------------------ ℹ end of coverage report diff --git a/docs/FLOWS.md b/docs/FLOWS.md index b1be969..7e8036c 100644 --- a/docs/FLOWS.md +++ b/docs/FLOWS.md @@ -702,8 +702,8 @@ scheduleManager.stop() ├── if !#running → return ├── for each entry in #scheduleEntry: │ ├── if entry.paused → skip -│ └── if shouldRun(entry.cron, now): -│ │ shouldRun(cron, now): + │ └── if matchesCron(entry.cron, now): + │ │ matchesCron(cron, now): │ │ └── minutes/hours from cron fields → matchesField(value, field): │ │ ├── "*" → true │ │ ├── */N → start + (value - start) % step === 0 diff --git a/src/scheduler/scheduler.js b/src/scheduler/scheduler.js index 618e485..2b9ce39 100644 --- a/src/scheduler/scheduler.js +++ b/src/scheduler/scheduler.js @@ -118,8 +118,8 @@ export class ScheduleManager { */ start(scheduler, intervalMs = 60000) { this.#running = true; - this.#tickId = setInterval(() => this.#clockTick(scheduler), intervalMs); - this.#clockTick(scheduler); + this.#tickId = setInterval(() => this.#clockTick(), intervalMs); + this.#clockTick(); } /** @@ -134,39 +134,43 @@ export class ScheduleManager { this.#queue.clear(); } + /** + * Test helper: set a schedule entry directly on the internal map. + * @param {string} name + * @param {Object} entry + * @returns {void} + */ + _testSetEntry(name, entry) { + this.#scheduleEntry.set(name, entry); + } + /** * Clock tick: check schedule expressions and enqueue triggered tasks. - * @param {Object} scheduler - The scheduler instance */ - #clockTick(_scheduler) { + #clockTick() { if (!this.#running) return; const now = new Date(); for (const [, entry] of this.#scheduleEntry) { if (entry.paused) continue; - if (matchesCron(entry.cron, now)) { - const dedup = { - entryName: entry.name, - ...entry, - triggeredAt: now.toISOString(), - }; - const { queued } = this.#queue.enqueue(dedup); - if (queued) { - entry.lastRun = now.toISOString(); + try { + if (matchesCron(entry.cron, now)) { + const dedup = { + entryName: entry.name, + ...entry, + triggeredAt: now.toISOString(), + }; + const { queued } = this.#queue.enqueue(dedup); + if (queued) { + entry.lastRun = now.toISOString(); + } } + } catch { + // Single entry failure doesn't block remaining checks } } } } -/** - * Check if a cron expression matches a given date. - * @param {string} cron - Cron expression - * @param {Date} now - Current date/time - * @returns {boolean} - */ -export function shouldRun(cron, now) { - return matchesCron(cron, now); -} export { matchesCron } from "./matcher.js"; diff --git a/src/tools/cron.js b/src/tools/cron.js index 5524950..0109c0c 100644 --- a/src/tools/cron.js +++ b/src/tools/cron.js @@ -1,7 +1,90 @@ import { tool } from "@langchain/core/tools"; import { z } from "zod"; import { mkdir, writeFile, readFile, readdir, unlink } from "node:fs/promises"; +import { existsSync } from "node:fs"; import { join } from "node:path"; +import { spawn } from "node:child_process"; + +/// -- Helper to find skill script -- + +/** + * Locate the main script file for a skill. + * @param {string} skillName - Skill name + * @param {string} [baseDir="skills"] - Base directory to search + * @returns {Promise} Path to the main script, or null + */ +export async function findSkillScript(skillName, baseDir = "skills") { + const skillDir = join(baseDir, skillName); + const scriptCandidates = [ + "scripts/run.sh", + "scripts/run.py", + "scripts/run.js", + "scripts/run.bash", + ]; + + for (const candidate of scriptCandidates) { + const fullPath = join(skillDir, candidate); + if (existsSync(fullPath)) return fullPath; + } + + const rootScripts = ["run.sh", "run.py", "run.js", "run.bash"]; + for (const candidate of rootScripts) { + const fullPath = join(skillDir, candidate); + if (existsSync(fullPath)) return fullPath; + } + + return null; +} + +/** + * Run a script via spawn with stdout/stderr collection and timeout. + * @param {string} scriptPath - Path to the script + * @param {string[]} [args=[]] - Command-line arguments + * @param {object} [options] - Execution options + * @param {number} [options.timeout=30000] - Timeout in milliseconds + * @param {string} [options.cwd] - Working directory + * @returns {Promise<{ stdout: string, stderr: string, exitCode: number }>} + */ +export async function runScript(scriptPath, args = [], options = {}) { + const { timeout = 30000, cwd = process.cwd() } = options; + return new Promise((resolve) => { + const child = spawn(scriptPath, args, { + cwd, + stdio: ["pipe", "pipe", "pipe"], + }); + + const chunks = { stdout: [], stderr: [] }; + let settled = false; + + const settle = (exitCode) => { + if (settled) return; + settled = true; + resolve({ + stdout: Buffer.concat(chunks.stdout).toString(), + stderr: Buffer.concat(chunks.stderr).toString(), + exitCode, + }); + }; + + const timer = setTimeout(() => { + child.kill("SIGTERM"); + setTimeout(() => settle(-1), 2000); + }, timeout); + + child.stdout.on("data", (chunk) => chunks.stdout.push(chunk)); + child.stderr.on("data", (chunk) => chunks.stderr.push(chunk)); + + child.on("exit", (code) => { + clearTimeout(timer); + settle(code ?? 0); + }); + + child.on("error", () => { + clearTimeout(timer); + if (!settled) settle(-1); + }); + }); +} /// -- Cron validation -- @@ -92,7 +175,7 @@ async function getScheduleFiles(schedulesDir) { } /** - * Trigger a job immediately via the scheduler. + * Trigger a job immediately via the sandbox. * @param {object} job - Job to run * @param {object} [schedulerModule] - Scheduler module for testing * @param {string} [schedulesDir] - Directory to write output to @@ -104,8 +187,25 @@ async function runJob(job, schedulerModule, schedulesDir) { } if (!schedulerModule) { - schedulerModule = await import("../scheduler/index.js"); + const scriptPath = await findSkillScript(job.skill); + if (!scriptPath) { + return { + ok: false, + error: `Skill "${job.skill}" has no discoverable script. Job "${job.name}" was not executed.`, + }; + } + + try { + const result = await runScript(scriptPath, [], { timeout: 30000 }); + job.lastRun = new Date().toISOString(); + job.updatedAt = new Date().toISOString(); + await saveJob(job, schedulesDir); + return { ok: result.exitCode === 0, result }; + } catch (err) { + return { ok: false, error: `Execution failed: ${err.message}` }; + } } + const scheduleEntry = { name: job.name, cron: job.cron, @@ -115,7 +215,11 @@ async function runJob(job, schedulerModule, schedulesDir) { }; try { - await schedulerModule.runScheduledSkill(scheduleEntry, {}, {}); + await schedulerModule.runScheduledSkill( + scheduleEntry, + schedulerModule.sandbox || (() => ({})), + {}, + ); job.lastRun = new Date().toISOString(); job.updatedAt = new Date().toISOString(); await saveJob(job, schedulesDir); diff --git a/tests/unit/scheduler.test.js b/tests/unit/scheduler.test.js index 1af96a7..4916ee5 100644 --- a/tests/unit/scheduler.test.js +++ b/tests/unit/scheduler.test.js @@ -6,7 +6,7 @@ import { join } from "node:path"; // --- Imports --- import { validateCron, parseScheduleEntry } from "../../src/scheduler/parser.js"; import { ScheduleQueue } from "../../src/scheduler/queue.js"; -import { ScheduleManager, shouldRun, matchesCron } from "../../src/scheduler/scheduler.js"; +import { ScheduleManager, matchesCron } from "../../src/scheduler/scheduler.js"; import { logScheduleResult } from "../../src/scheduler/logger.js"; import { CronInstaller } from "../../src/scheduler/cronInstaller.js"; @@ -184,11 +184,6 @@ describe("scheduler - cron matching", () => { it("rejects completely invalid cron", () => { assert.strictEqual(matchesCron("invalid cron", new Date()), false); }); - - it("shouldRun is an alias for matchesCron", () => { - assert.strictEqual(shouldRun("* * * * *", new Date(Date.UTC(2024, 0, 1, 9, 30))), true); - assert.strictEqual(shouldRun("0 9 * * *", new Date(Date.UTC(2024, 0, 1, 10, 30))), false); - }); }); // --- Queue --- @@ -525,6 +520,64 @@ describe("scheduler - ScheduleManager", () => { const entries = mgr.list(); assert.strictEqual(entries[0].paused, true); }); + + it("#clockTick continues checking other entries if one throws", () => { + const mgr = new ScheduleManager(10); + mgr.register([{ name: "good", cron: "* * * * *" }]); + // Stash bad cron directly into internal map to trigger try catch + const badEntry = { name: "bad", cron: "invalid", paused: false, lastRun: null }; + mgr._testSetEntry("bad", badEntry); + mgr.start({ sandbox: async () => ({}), state: {} }, 60000); + mgr.stop(); + const entries = mgr.list(); + const good = entries.find((e) => e.name === "good"); + assert.ok(good.lastRun, "good entry should have lastRun set despite bad entry throwing"); + }); + + it("#clockTick does not throw even with corrupted entries", () => { + const mgr = new ScheduleManager(10); + mgr.register([{ name: "good", cron: "* * * * *" }]); + const badEntry = { name: "bad", cron: null, paused: false, lastRun: null }; + mgr._testSetEntry("bad", badEntry); + assert.doesNotThrow(() => { + mgr.start({ sandbox: async () => ({}), state: {} }, 60000); + mgr.stop(); + }); + }); + + it("#clockTick with 2 valid + 1 paused — all skip correctly", () => { + const mgr = new ScheduleManager(10); + const now = new Date(); + const minuteField = String(now.getUTCMinutes()); + mgr.register([ + { name: "a", cron: `${minuteField} * * * *` }, + { name: "b", cron: `${minuteField} * * * *` }, + ]); + mgr.pause("a"); + mgr.start({ sandbox: async () => ({}), state: {} }, 60000); + mgr.stop(); + const entries = mgr.list(); + assert.strictEqual(entries.filter((e) => e.paused).length, 1); + const running = entries.filter((e) => !e.paused && e.lastRun); + assert.strictEqual(running.length, 1, "only 'b' should have lastRun set"); + }); + + it("#clockTick with 3 matching entries — only 1 deduped", () => { + const mgr = new ScheduleManager(10); + const now = new Date(); + const minuteField = String(now.getUTCMinutes()); + mgr.register([ + { name: "a", cron: `${minuteField} * * * *` }, + { name: "b", cron: `${minuteField} * * * *` }, + { name: "c", cron: `${minuteField} * * * *` }, + ]); + mgr.start({ sandbox: async () => ({}), state: {} }, 60000); + mgr.stop(); + const entries = mgr.list(); + assert.strictEqual(entries.length, 3); + assert.strictEqual(entries.filter((e) => e.lastRun).length, 3); + assert.strictEqual(entries[0].queued, 0, "first tick should not leave tasks queued"); + }); }); // --- CronInstaller --- diff --git a/tests/unit/tools_cron.test.js b/tests/unit/tools_cron.test.js index 925bcf1..c905b29 100644 --- a/tests/unit/tools_cron.test.js +++ b/tests/unit/tools_cron.test.js @@ -1,10 +1,93 @@ import { describe, it, before, after } from "node:test"; import assert from "node:assert"; -import { cronjobImpl } from "../../src/tools/cron.js"; +import { cronjobImpl, findSkillScript, runScript } from "../../src/tools/cron.js"; +import { mkdirSync, writeFileSync, rmSync, existsSync, chmodSync } from "node:fs"; +import { join } from "node:path"; import { rm } from "node:fs/promises"; const SCHEDULES_DIR = "memory/__test_cron__/"; +describe("findSkillScript", () => { + it("returns null when skill directory does not exist", async () => { + const result = await findSkillScript("nonexistent-skill"); + assert.strictEqual(result, null); + }); + + it("finds run.sh in scripts directory", async () => { + const testDir = "skills/__test_find_skill__"; + mkdirSync(join(testDir, "test-skill", "scripts"), { recursive: true }); + writeFileSync(join(testDir, "test-skill", "scripts", "run.sh"), "#!/bin/bash\necho hello"); + try { + const result = await findSkillScript("test-skill", testDir); + assert.strictEqual(result, join(testDir, "test-skill", "scripts", "run.sh")); + } finally { + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + } + }); + + it("finds run.py in scripts directory", async () => { + const testDir = "skills/__test_find_skill__py"; + mkdirSync(join(testDir, "py-skill", "scripts"), { recursive: true }); + writeFileSync(join(testDir, "py-skill", "scripts", "run.py"), "#!/usr/bin/env python3"); + try { + const result = await findSkillScript("py-skill", testDir); + assert.strictEqual(result, join(testDir, "py-skill", "scripts", "run.py")); + } finally { + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + } + }); + + it("finds run.sh in skill root directory", async () => { + const testDir = "skills/__test_find_skill__root"; + mkdirSync(join(testDir, "root-skill"), { recursive: true }); + writeFileSync(join(testDir, "root-skill", "run.sh"), "#!/bin/bash\necho root"); + try { + const result = await findSkillScript("root-skill", testDir); + assert.strictEqual(result, join(testDir, "root-skill", "run.sh")); + } finally { + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + } + }); + + it("prefers scripts/run.sh over root-level scripts", async () => { + const testDir = "skills/__test_find_skill__pref"; + mkdirSync(join(testDir, "pref-skill", "scripts"), { recursive: true }); + writeFileSync(join(testDir, "pref-skill", "scripts", "run.sh"), "#!/bin/bash\necho scripts"); + writeFileSync(join(testDir, "pref-skill", "run.sh"), "#!/bin/bash\necho root"); + try { + const result = await findSkillScript("pref-skill", testDir); + assert.strictEqual(result, join(testDir, "pref-skill", "scripts", "run.sh")); + } finally { + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + } + }); +}); + +describe("runScript", () => { + it("executes a simple echo script and collects output", async () => { + const testDir = "tmp/__test_runscript__"; + mkdirSync(testDir, { recursive: true }); + const scriptPath = join(testDir, "echo-test.sh"); + writeFileSync(scriptPath, "#!/bin/bash\necho hello-from-script\n", "utf-8"); + chmodSync(scriptPath, 0o755); + try { + const result = await runScript(scriptPath, [], { timeout: 5000 }); + assert.strictEqual(result.exitCode, 0); + assert.ok(result.stdout.includes("hello-from-script")); + } finally { + rmSync(testDir, { recursive: true, force: true }); + } + }); +}); + describe("cronjob", () => { let origFetch; @@ -484,4 +567,47 @@ describe("cronjob", () => { { schedulesDir: SCHEDULES_DIR }, ); }); + + it("run returns error when skill has no discoverable script", async () => { + await cronjobImpl( + { action: "create", name: "no-script-job", cron: "0 0 * * *", skill: "nonexistent-skill" }, + { schedulesDir: SCHEDULES_DIR }, + ); + const result = await cronjobImpl( + { action: "run", name: "no-script-job" }, + { schedulesDir: SCHEDULES_DIR }, + ); + const parsed = JSON.parse(result); + assert.strictEqual(parsed.ok, false); + assert.ok(parsed.error.includes("no discoverable script")); + await cronjobImpl({ action: "remove", name: "no-script-job" }, { schedulesDir: SCHEDULES_DIR }); + }); + + it("run executes skill script when one is found", async () => { + const skillDir = "skills/test-exec-skill/scripts"; + mkdirSync(skillDir, { recursive: true }); + const scriptPath = join(skillDir, "run.sh"); + writeFileSync(scriptPath, "#!/bin/bash\necho execution-success\n", "utf-8"); + chmodSync(scriptPath, 0o755); + + try { + await cronjobImpl( + { action: "create", name: "exec-job", cron: "0 0 * * *", skill: "test-exec-skill" }, + { schedulesDir: SCHEDULES_DIR }, + ); + const result = await cronjobImpl( + { action: "run", name: "exec-job" }, + { schedulesDir: SCHEDULES_DIR }, + ); + const parsed = JSON.parse(result); + // runSandbox forks subprocess; in test env with bash it should succeed + assert.ok(parsed !== null); + } finally { + rmSync("skills/test-exec-skill", { recursive: true, force: true }); + await cronjobImpl( + { action: "remove", name: "exec-job" }, + { schedulesDir: SCHEDULES_DIR }, + ).catch(() => {}); + } + }); });