diff --git a/packages/web/server/lib/scheduled-tasks/DOCUMENTATION.md b/packages/web/server/lib/scheduled-tasks/DOCUMENTATION.md index 4de46ab5..4be03824 100644 --- a/packages/web/server/lib/scheduled-tasks/DOCUMENTATION.md +++ b/packages/web/server/lib/scheduled-tasks/DOCUMENTATION.md @@ -96,10 +96,11 @@ project write lock on every `syncProject` when the project path is known: - **UI edits** to a loop-sourced task are preserved in the config but the loop file remains authoritative: the next reconciliation re-applies the file's definition (including `enabled`). Use `enabled: false` in the file to - disable. Deleting a loop-sourced task through the API is rejected with a 400 — - the loop file is the removal surface. The scheduled-tasks UI marks loop tasks - as file-managed and disables their edit/enable/delete actions for the same - reason; `run now` remains available. + disable. Deleting a loop-sourced task through the API is rejected with a 400 + while its loop file still exists on disk — the loop file is the removal + surface; once the file is gone, deleting the orphan task is allowed. The + scheduled-tasks UI marks loop tasks as file-managed and disables their + edit/enable/delete actions for the same reason; `run now` remains available. ## Public exports (runtime.js) diff --git a/packages/web/server/lib/scheduled-tasks/runtime.test.js b/packages/web/server/lib/scheduled-tasks/runtime.test.js index 7dafaca9..3a59b19f 100644 --- a/packages/web/server/lib/scheduled-tasks/runtime.test.js +++ b/packages/web/server/lib/scheduled-tasks/runtime.test.js @@ -1,5 +1,15 @@ -import { describe, expect, it } from 'vitest'; -import { computeNextRunAt, expandCommandGoalObjective, formatScheduledSessionTitle, parseScheduledCommandPrompt } from './runtime.js'; +import { describe, expect, it, vi } from 'vitest'; +import os from 'os'; +import path from 'path'; +import { mkdtemp, rm, mkdir, writeFile } from 'fs/promises'; +import { + computeNextRunAt, + expandCommandGoalObjective, + formatScheduledSessionTitle, + parseScheduledCommandPrompt, + createScheduledTasksRuntime, +} from './runtime.js'; +import { createProjectConfigRuntime } from '../projects/project-config.js'; describe('scheduled-tasks runtime helpers', () => { it('computes next daily run in timezone', () => { @@ -109,3 +119,90 @@ describe('scheduled-tasks runtime helpers', () => { .toBe('Review the requested scope.\n\nauth module'); }); }); + +describe('scheduled-tasks runtime syncProject wiring', () => { + const createTempProject = async () => { + const tempRoot = await mkdtemp(path.join(os.tmpdir(), 'oc-runtime-loop-')); + const repoPath = path.join(tempRoot, 'repo'); + await mkdir(path.join(repoPath, '.agents', 'loops'), { recursive: true }); + return { + tempRoot, + repoPath, + cleanup: async () => { + await rm(tempRoot, { recursive: true, force: true }); + }, + }; + }; + + const createProjectConfig = async (tempRoot) => createProjectConfigRuntime({ + fsPromises: await import('fs/promises'), + path, + projectsDirPath: path.join(tempRoot, 'config'), + createTaskID: () => 'task-fixed-id', + }); + + const createRuntimeDeps = (overrides = {}) => ({ + buildOpenCodeUrl: () => 'http://localhost', + getOpenCodeAuthHeaders: () => ({}), + waitForOpenCodeReady: async () => {}, + ...overrides, + }); + + it('reconciles discovered loops when the project path is known', async () => { + const { tempRoot, repoPath, cleanup } = await createTempProject(); + try { + await writeFile(path.join(repoPath, '.agents', 'loops', 'daily.md'), `--- +name: daily +schedule: "0 9 * * *" +enabled: true +model: openai/gpt-5 +--- +Run daily. +`, 'utf8'); + + const projectConfigRuntime = await createProjectConfig(tempRoot); + const runtime = createScheduledTasksRuntime({ + ...createRuntimeDeps(), + projectConfigRuntime, + listProjects: async () => [{ id: 'proj', path: repoPath }], + }); + + await runtime.syncProject('proj'); + + const tasks = await projectConfigRuntime.listScheduledTasks('proj'); + expect(tasks).toHaveLength(1); + expect(tasks[0].id).toBe('loop:project:daily'); + expect(tasks[0].loopFile).toBe(path.join(repoPath, '.agents', 'loops', 'daily.md')); + // syncTaskSchedule computed and persisted the next run for the enabled task. + expect(tasks[0].state.nextRunAt).toBeGreaterThan(0); + } finally { + await cleanup(); + } + }); + + it('falls back to plain listing when the project path cannot be resolved', async () => { + const { tempRoot, cleanup } = await createTempProject(); + try { + const projectConfigRuntime = await createProjectConfig(tempRoot); + const reconcileSpy = vi.spyOn(projectConfigRuntime, 'reconcileLoopTasks'); + const listSpy = vi.spyOn(projectConfigRuntime, 'listScheduledTasks'); + + const runtime = createScheduledTasksRuntime({ + ...createRuntimeDeps(), + projectConfigRuntime, + // Project not registered -> ensureProjectPath cannot resolve a path. + listProjects: async () => [], + }); + + await runtime.syncProject('proj'); + + expect(reconcileSpy).not.toHaveBeenCalled(); + expect(listSpy).toHaveBeenCalledWith('proj'); + expect(await projectConfigRuntime.listScheduledTasks('proj')).toEqual([]); + reconcileSpy.mockRestore(); + listSpy.mockRestore(); + } finally { + await cleanup(); + } + }); +}); diff --git a/packages/web/server/lib/scheduled-tasks/service.js b/packages/web/server/lib/scheduled-tasks/service.js index 33d69f7d..4d94251a 100644 --- a/packages/web/server/lib/scheduled-tasks/service.js +++ b/packages/web/server/lib/scheduled-tasks/service.js @@ -1,3 +1,4 @@ +import fs from 'node:fs'; import path from 'node:path'; import { OpenChamberControlError } from '../openchamber-control/error.js'; @@ -80,10 +81,12 @@ export const createScheduledTaskService = (dependencies) => { if (!normalizedTaskID) throw new OpenChamberControlError('taskId is required', 400); const current = await projectConfigRuntime.listScheduledTasks(projectID); const existing = current.find((task) => task.id === normalizedTaskID) || null; - if (existing?.loopFile) { + if (existing?.loopFile && fs.existsSync(existing.loopFile)) { // Loop tasks are owned by their `.agents/loops` markdown file: deleting - // the JSON row would be silently undone by the next reconcile. The file - // itself is the removal surface. + // the JSON row would be silently undone by the next reconcile while the + // file exists. The file itself is the removal surface. Once the file is + // gone (the task is an orphan that the next sync would remove anyway), + // deleting the row is safe and allowed. throw new OpenChamberControlError( 'Loop task is managed by its .agents/loops markdown file; delete the file to remove the task', 400, diff --git a/packages/web/server/lib/scheduled-tasks/service.test.js b/packages/web/server/lib/scheduled-tasks/service.test.js index 07d6bf70..a90c1b64 100644 --- a/packages/web/server/lib/scheduled-tasks/service.test.js +++ b/packages/web/server/lib/scheduled-tasks/service.test.js @@ -1,4 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; +import os from 'os'; +import path from 'path'; +import { mkdtemp, rm, writeFile } from 'fs/promises'; import { createScheduledTaskService } from './service.js'; const createService = (overrides = {}) => { @@ -32,19 +35,50 @@ const loopTask = { }; describe('scheduled-task service remove', () => { - it('rejects deleting a loop-sourced task without touching storage', async () => { - const { service, projectConfigRuntime, scheduledTasksRuntime } = createService({ - projectConfigRuntime: { - listScheduledTasks: vi.fn(async () => [loopTask]), - }, - }); + it('rejects deleting a loop-sourced task while its loop file still exists', async () => { + const tempRoot = await mkdtemp(path.join(os.tmpdir(), 'oc-loop-delete-')); + try { + const loopFilePath = path.join(tempRoot, 'daily.md'); + await writeFile(loopFilePath, '---\nname: daily-digest\n---\nRun.\n', 'utf8'); - await expect(service.remove('project-test', loopTask.id)).rejects.toMatchObject({ - statusCode: 400, - message: expect.stringContaining('delete the file to remove the task'), - }); - expect(projectConfigRuntime.deleteScheduledTask).not.toHaveBeenCalled(); - expect(scheduledTasksRuntime.syncProject).not.toHaveBeenCalled(); + const { service, projectConfigRuntime, scheduledTasksRuntime } = createService({ + projectConfigRuntime: { + listScheduledTasks: vi.fn(async () => [{ ...loopTask, loopFile: loopFilePath }]), + }, + }); + + await expect(service.remove('project-test', loopTask.id)).rejects.toMatchObject({ + statusCode: 400, + message: expect.stringContaining('delete the file to remove the task'), + }); + expect(projectConfigRuntime.deleteScheduledTask).not.toHaveBeenCalled(); + expect(scheduledTasksRuntime.syncProject).not.toHaveBeenCalled(); + } finally { + await rm(tempRoot, { recursive: true, force: true }); + } + }); + + it('allows deleting a loop-sourced task once its loop file is gone', async () => { + const tempRoot = await mkdtemp(path.join(os.tmpdir(), 'oc-loop-delete-')); + try { + // The loop file was removed from disk; the orphan task is allowed to be + // deleted directly instead of waiting for the next reconcile. + const loopFilePath = path.join(tempRoot, 'gone.md'); + + const { service, projectConfigRuntime, scheduledTasksRuntime } = createService({ + projectConfigRuntime: { + listScheduledTasks: vi.fn(async () => [{ ...loopTask, loopFile: loopFilePath }]), + }, + }); + + const tasks = await service.remove('project-test', loopTask.id); + + expect(projectConfigRuntime.deleteScheduledTask).toHaveBeenCalledWith('project-test', loopTask.id); + expect(scheduledTasksRuntime.syncProject).toHaveBeenCalled(); + expect(Array.isArray(tasks)).toBe(true); + } finally { + await rm(tempRoot, { recursive: true, force: true }); + } }); it('deletes JSON-configured tasks normally', async () => {