fix(tasks): harden loop reconciliation against renames and malformed files

Review fixes for the markdown loop feature:

- Loop-owned tasks now adopt by loop file path, not task name, so renaming
  a loop (frontmatter name or UI rename) renames the task in place instead
  of leaving a stale duplicate that keeps running the old definition;
  orphan duplicates of the same file are unscheduled.
- Unparseable loop files are reported to the scheduler as
  definition:null entries: a task whose file still exists is kept with its
  last good definition, and only a genuinely removed file unschedules it.
  Transiently malformed files (mid-edit, bad merge) no longer delete tasks
  or their runtime state.
- Adoption preserves UI-only execution fields (goalEnabled, goalTokenBudget,
  permissionAutoAccept, variant) that the portable format does not define.
- DELETE on a loop-sourced task now returns 400 with guidance to remove the
  loop file, instead of being silently undone by the next reconcile.
- Loops default to enabled: false; discovery of repository content never
  auto-executes scheduled sessions unless the file explicitly enables them.

Regression tests for each fix; DOCUMENTATION.md updated.

packages/web/server/lib/scheduled-tasks/DOCUMENTATION.md

@@ -50,7 +50,7 @@ Field mapping (model: `packages/ui/src/lib/scheduledTasksApi.ts`):
 |---|---|
 | `name` | `name` (required) |
 | `schedule` | `schedule.kind: "cron"` + `schedule.cron` (required, cron-only in the portable format) |
-| `enabled` | `enabled` (default `true`) |
+| `enabled` | `enabled` (default `false` — a loop only runs when the file explicitly enables it; add `enabled: true` to activate) |
 | `model` | split on the first `/` into `execution.providerID` / `execution.modelID` (required) |
 | `agent` | `execution.agent` (optional) |
 | `timezone` | `schedule.timezone` (optional, IANA; defaults to the server zone) |
@@ -67,24 +67,37 @@ the project config state store.
 `projectConfigRuntime.reconcileLoopTasks(projectID, loops)` runs inside the
 project write lock on every `syncProject` when the project path is known:
 
-- **Identity is the task name.** A loop whose name matches an existing task
-  takes that task over: its schedule/execution/enabled are overwritten from the
-  file while the task's `id` and runtime `state` are preserved (markdown wins
-  on conflict with JSON-configured tasks).
+- **Identity.** For loop-owned tasks (carrying the `loopFile` marker) identity
+  is the loop file path: a loop takes its task over regardless of the task's
+  current name, so renaming the loop (the `name` field, or a UI rename) renames
+  the task in place instead of leaving a stale duplicate behind. A loop whose
+  name matches a JSON task (no `loopFile`) takes that task over instead: its
+  schedule/execution/enabled are overwritten from the file while the task's
+  `id` and runtime `state` are preserved (markdown wins on conflict).
+- **UI-only fields survive adoption.** Execution fields the file format does
+  not define (`goalEnabled`, `goalTokenBudget`, `permissionAutoAccept`,
+  `variant`) are preserved from the task when a loop adopts it; only fields the
+  file defines are re-applied.
 - **Deletion.** A task carrying the `loopFile` marker whose loop file is no
   longer discovered (removed or renamed) is unscheduled (removed from the
   config). The marker is persisted in the config file, so removal is detected
   across restarts. JSON-configured tasks without the marker are never removed.
+  A task whose loop file still exists but is currently unparseable is KEPT with
+  its last good definition — a transiently malformed file (mid-edit, bad merge)
+  never deletes a task or its runtime state.
 - **Creation.** Loops without a matching task are created under a deterministic
-  `loop:<scope>:<name>` id so runtime state survives restarts.
+  `loop:<scope>:<name>` id so runtime state survives restarts. At most one task
+  is driven per loop file; orphan duplicates of the same file are unscheduled.
 - **Scope precedence.** Project-scope loops shadow user-scope loops with the
   same name; among project files the nearest ancestor wins.
 - **Malformed files** (missing `name`/`schedule`/`model`/body, invalid cron,
-  unreadable) are skipped with a warning and never block valid loops in the
-  same or other scopes.
+  unreadable) are reported to the scheduler as `definition: null` entries and
+  warned about; they never block valid loops in the same or other scopes.
 - **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.
+  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.
 
 ## Public exports (runtime.js)
 

packages/web/server/lib/scheduled-tasks/loops.js

@@ -21,7 +21,9 @@
  * Field mapping (see packages/ui/src/lib/scheduledTasksApi.ts):
  *   name     -> task.name
  *   schedule -> task.schedule.kind "cron" + task.schedule.cron
- *   enabled  -> task.enabled (default true)
+ *   enabled  -> task.enabled (default false — loops only run when the file
+ *               explicitly enables them, so discovery never auto-executes
+ *               repository content)
  *   model    -> split into task.execution.providerID / task.execution.modelID
  *   agent    -> task.execution.agent (optional)
  *   timezone -> task.schedule.timezone (optional, defaults to the server zone)
@@ -116,7 +118,7 @@ export const parseLoopDefinition = (filePath) => {
 
   return {
     name,
-    enabled: typeof frontmatter.enabled === 'boolean' ? frontmatter.enabled : true,
+    enabled: typeof frontmatter.enabled === 'boolean' ? frontmatter.enabled : false,
     schedule: {
       kind: 'cron',
       cron,
@@ -170,13 +172,21 @@ export const discoverLoopFiles = (projectPath) => {
 /**
  * Discover and parse all loops for a project. Project-scope loops shadow
  * user-scope loops with the same name; among project files the nearest
- * ancestor wins. Malformed files are skipped with a warning.
+ * ancestor wins.
+ *
+ * Unparseable files are reported as `{ scope, filePath, definition: null }`
+ * entries instead of being dropped: the scheduler must distinguish "file is
+ * gone" (unschedule its task) from "file exists but is currently malformed"
+ * (keep its task with the last good definition until the file is fixed).
+ * Malformed files never block valid ones in the same or other scopes.
  */
 export const discoverLoops = (projectPath) => {
   const byName = new Map();
+  const loops = [];
   for (const { filePath, scope } of discoverLoopFiles(projectPath)) {
     const definition = parseLoopDefinition(filePath);
     if (!definition) {
+      loops.push({ scope, filePath, definition: null });
       continue;
     }
     const existing = byName.get(definition.name);
@@ -185,5 +195,8 @@ export const discoverLoops = (projectPath) => {
     }
     byName.set(definition.name, { scope, filePath, definition });
   }
-  return [...byName.values()];
+  for (const entry of byName.values()) {
+    loops.push(entry);
+  }
+  return loops;
 };

packages/web/server/lib/scheduled-tasks/loops.test.js

@@ -73,13 +73,13 @@ Run weekly checks.
 
       expect(definition.execution.providerID).toBe('openai');
       expect(definition.execution.modelID).toBe('gpt-5');
-      expect(definition.enabled).toBe(true);
+      expect(definition.enabled).toBe(false);
     } finally {
       await cleanup();
     }
   });
 
-  it('defaults enabled to true and omits optional fields', async () => {
+  it('defaults enabled to false and omits optional fields', async () => {
     const { projectPath, cleanup } = await createProject();
     try {
       const filePath = path.join(projectPath, 'loop.md');
@@ -87,13 +87,14 @@ Run weekly checks.
 name: minimal
 schedule: "*/30 * * * *"
 model: openai/gpt-5
-enabled: false
 ---
 Run every half hour.
 `, 'utf8');
 
       const definition = parseLoopDefinition(filePath);
 
+      // Loops only run when the file explicitly enables them: discovery of
+      // repository content must never auto-execute scheduled sessions.
       expect(definition.enabled).toBe(false);
       expect(definition.schedule).toEqual({ kind: 'cron', cron: '*/30 * * * *' });
       expect(definition.execution.agent).toBeUndefined();
@@ -102,6 +103,25 @@ Run every half hour.
     }
   });
 
+  it('honors an explicit enabled: true in the frontmatter', async () => {
+    const { projectPath, cleanup } = await createProject();
+    try {
+      const filePath = path.join(projectPath, 'loop.md');
+      await writeFile(filePath, `---
+name: explicit-enabled
+schedule: "*/30 * * * *"
+model: openai/gpt-5
+enabled: true
+---
+Run every half hour.
+`, 'utf8');
+
+      expect(parseLoopDefinition(filePath).enabled).toBe(true);
+    } finally {
+      await cleanup();
+    }
+  });
+
   it('returns null for files missing required frontmatter fields', async () => {
     const { projectPath, cleanup } = await createProject();
     try {
@@ -272,7 +292,7 @@ Project version.
     }
   });
 
-  it('skips malformed files without blocking valid ones', async () => {
+  it('reports malformed files as unparsed entries without blocking valid ones', async () => {
     const { projectPath, cleanup } = await createProject();
     try {
       await writeLoop(projectPath, 'bad.md', `---
@@ -292,7 +312,14 @@ Valid.
       try {
         const loops = discoverLoops(projectPath);
 
-        expect(loops.map((loop) => loop.definition.name)).toEqual(['good']);
+        // The malformed file stays visible as a `definition: null` entry so
+        // the scheduler can keep its task alive while the file is fixed.
+        const bad = loops.find((loop) => loop.filePath.endsWith(path.join('.agents', 'loops', 'bad.md')));
+        expect(bad.definition).toBeNull();
+        expect(bad.scope).toBe('project');
+
+        const good = loops.find((loop) => loop.filePath.endsWith(path.join('.agents', 'loops', 'good.md')));
+        expect(good.definition.name).toBe('good');
         expect(warn).toHaveBeenCalled();
       } finally {
         warn.mockRestore();

packages/web/server/lib/scheduled-tasks/service.js

@@ -78,6 +78,17 @@ export const createScheduledTaskService = (dependencies) => {
     await findProjectByID(projectID);
     const normalizedTaskID = asNonEmptyString(taskID);
     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) {
+      // 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.
+      throw new OpenChamberControlError(
+        'Loop task is managed by its .agents/loops markdown file; delete the file to remove the task',
+        400,
+      );
+    }
     const result = await projectConfigRuntime.deleteScheduledTask(projectID, normalizedTaskID);
     if (!result.deleted) throw new OpenChamberControlError('Task not found', 404);
     await scheduledTasksRuntime.syncProject(projectID);

packages/web/server/lib/scheduled-tasks/service.test.js

@@ -0,0 +1,65 @@
+import { describe, expect, it, vi } from 'vitest';
+import { createScheduledTaskService } from './service.js';
+
+const createService = (overrides = {}) => {
+  const projectConfigRuntime = {
+    listScheduledTasks: vi.fn(async () => []),
+    deleteScheduledTask: vi.fn(async () => ({ deleted: true, tasks: [] })),
+    ...(overrides.projectConfigRuntime || {}),
+  };
+  const scheduledTasksRuntime = {
+    syncProject: vi.fn(async () => []),
+    ...(overrides.scheduledTasksRuntime || {}),
+  };
+  const service = createScheduledTaskService({
+    readSettingsFromDiskMigrated: async () => ({
+      projects: [{ id: 'project-test', path: '/repo' }],
+    }),
+    sanitizeProjects: (projects) => projects,
+    projectConfigRuntime,
+    scheduledTasksRuntime,
+  });
+  return { service, projectConfigRuntime, scheduledTasksRuntime };
+};
+
+const loopTask = {
+  id: 'loop:project:daily-digest',
+  name: 'daily-digest',
+  enabled: true,
+  loopFile: '/repo/.agents/loops/daily-digest.md',
+  schedule: { kind: 'cron', cron: '0 9 * * *', timezone: 'UTC' },
+  execution: { prompt: 'digest', providerID: 'openai', modelID: 'gpt-4.1' },
+};
+
+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]),
+      },
+    });
+
+    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();
+  });
+
+  it('deletes JSON-configured tasks normally', async () => {
+    const jsonTask = { ...loopTask, id: 'json-task', loopFile: undefined };
+    const { service, projectConfigRuntime, scheduledTasksRuntime } = createService({
+      projectConfigRuntime: {
+        listScheduledTasks: vi.fn(async () => [jsonTask]),
+        deleteScheduledTask: vi.fn(async () => ({ deleted: true, tasks: [] })),
+      },
+    });
+
+    const tasks = await service.remove('project-test', jsonTask.id);
+
+    expect(projectConfigRuntime.deleteScheduledTask).toHaveBeenCalledWith('project-test', jsonTask.id);
+    expect(scheduledTasksRuntime.syncProject).toHaveBeenCalled();
+    expect(Array.isArray(tasks)).toBe(true);
+  });
+});