refactor(schedules): replace deleteJobsByRepo with prepareRepoDelete, add lightweight ID query, remove dead mock

Author: chriswritescode-dev

backend/src/db/schedules.ts

@@ -116,6 +116,12 @@ export function listScheduleJobsByRepo(db: Database, repoId: number): ScheduleJo
   return rows.map(rowToScheduleJob)
 }
 
+export function listScheduleJobIdsByRepo(db: Database, repoId: number): number[] {
+  const stmt = db.prepare('SELECT id FROM schedule_jobs WHERE repo_id = ? ORDER BY created_at DESC')
+  const rows = stmt.all(repoId) as Array<{ id: number }>
+  return rows.map((row) => row.id)
+}
+
 export function listEnabledScheduleJobs(db: Database): ScheduleJob[] {
   const stmt = db.prepare('SELECT * FROM schedule_jobs WHERE enabled = 1 ORDER BY id ASC')
   const rows = stmt.all() as ScheduleJobRow[]
@@ -206,13 +212,6 @@ export function deleteScheduleJob(db: Database, repoId: number, jobId: number):
   return result.changes > 0
 }
 
-export function deleteScheduleJobsByRepo(db: Database, repoId: number): number {
-  db.prepare('DELETE FROM schedule_runs WHERE repo_id = ?').run(repoId)
-  const stmt = db.prepare('DELETE FROM schedule_jobs WHERE repo_id = ?')
-  const result = stmt.run(repoId)
-  return result.changes
-}
-
 export function cleanupOrphanedSchedules(db: Database): { orphanedJobs: number; orphanedRuns: number } {
   const runStmt = db.prepare(`
     DELETE FROM schedule_runs

backend/src/routes/repos.test.ts

@@ -37,6 +37,7 @@ function createTestApp(db: Database, openCodeClient: OpenCodeClient = createStub
     listSchedules: () => [],
     updateSchedule: () => {},
     deleteSchedule: () => {},
+    prepareRepoDelete: () => {},
   } as any
   app.route('/repos', createRepoRoutes(db, stubGitAuthService, scheduleService, openCodeClient))
   return app

backend/src/routes/repos.ts

@@ -273,10 +273,7 @@ app.get('/', async (c) => {
         return c.json({ error: 'Repo not found' }, 404)
       }
 
-      // Unregister and delete all schedule jobs/runs for this repo first,
-      // so the ScheduleRunner doesn't hold stale cron timers and the DB
-      // cascade (or explicit delete) cleans up schedule data cleanly.
-      scheduleService.deleteJobsByRepo(id)
+      scheduleService.prepareRepoDelete(id)
 
       await repoService.deleteRepoFiles(database, id)
 

backend/src/services/schedules.ts

@@ -14,13 +14,13 @@ import {
   createScheduleJob,
   createScheduleRun,
   deleteScheduleJob,
-  deleteScheduleJobsByRepo,
   getScheduleJobById,
   getRunningScheduleRunByJob,
   getScheduleRunById,
   listAllScheduleJobsWithRepos,
   listAllScheduleRuns,
   listEnabledScheduleJobs,
+  listScheduleJobIdsByRepo,
   listScheduleJobsByRepo,
   listRunningScheduleRuns,
   listScheduleRunsByJob,
@@ -446,18 +446,11 @@ export class ScheduleService {
     this.onJobChange?.(null, jobId)
   }
 
-  /**
-   * Deletes all schedule jobs (and their runs) for a repo and notifies the
-   * ScheduleRunner so in-memory cron timers are removed. This should be called
-   * **before** the repo row itself is deleted.
-   */
-  deleteJobsByRepo(repoId: number): void {
-    // Notify the runner for each job so in-memory timers are cleaned up.
-    const jobs = listScheduleJobsByRepo(this.db, repoId)
-    for (const job of jobs) {
-      this.onJobChange?.(null, job.id)
+  prepareRepoDelete(repoId: number): void {
+    const jobIds = listScheduleJobIdsByRepo(this.db, repoId)
+    for (const jobId of jobIds) {
+      this.onJobChange?.(null, jobId)
     }
-    deleteScheduleJobsByRepo(this.db, repoId)
   }
 
   /**

backend/test/db/schedules.test.ts

@@ -72,6 +72,19 @@ describe('schedule database queries', () => {
     })
   })
 
+  it('lists schedule job ids without loading full job rows', () => {
+    const stmt = {
+      all: vi.fn().mockReturnValue([{ id: 7 }, { id: 8 }]),
+    }
+    mockDb.prepare.mockReturnValue(stmt)
+
+    const jobIds = schedulesDb.listScheduleJobIdsByRepo(mockDb, 42)
+
+    expect(mockDb.prepare).toHaveBeenCalledWith('SELECT id FROM schedule_jobs WHERE repo_id = ? ORDER BY created_at DESC')
+    expect(stmt.all).toHaveBeenCalledWith(42)
+    expect(jobIds).toEqual([7, 8])
+  })
+
   it('creates a schedule job and reloads the inserted row', () => {
     const insertStmt = {
       run: vi.fn().mockReturnValue({ lastInsertRowid: 7 }),

backend/test/services/schedules.test.ts

@@ -6,13 +6,13 @@ const mocks = vi.hoisted(() => ({
   createScheduleJob: vi.fn(),
   createScheduleRun: vi.fn(),
   deleteScheduleJob: vi.fn(),
-  deleteScheduleJobsByRepo: vi.fn(),
   cleanupOrphanedSchedules: vi.fn(),
   getScheduleJobById: vi.fn(),
   getRunningScheduleRunByJob: vi.fn(),
   getScheduleRunById: vi.fn(),
   listEnabledScheduleJobs: vi.fn(),
   listRunningScheduleRuns: vi.fn(),
+  listScheduleJobIdsByRepo: vi.fn(),
   listScheduleJobsByRepo: vi.fn(),
   listScheduleRunsByJob: vi.fn(),
   updateScheduleJob: vi.fn(),
@@ -37,13 +37,13 @@ vi.mock('../../src/db/schedules', () => ({
   createScheduleJob: mocks.createScheduleJob,
   createScheduleRun: mocks.createScheduleRun,
   deleteScheduleJob: mocks.deleteScheduleJob,
-  deleteScheduleJobsByRepo: mocks.deleteScheduleJobsByRepo,
   cleanupOrphanedSchedules: mocks.cleanupOrphanedSchedules,
   getScheduleJobById: mocks.getScheduleJobById,
   getRunningScheduleRunByJob: mocks.getRunningScheduleRunByJob,
   getScheduleRunById: mocks.getScheduleRunById,
   listEnabledScheduleJobs: mocks.listEnabledScheduleJobs,
   listRunningScheduleRuns: mocks.listRunningScheduleRuns,
+  listScheduleJobIdsByRepo: mocks.listScheduleJobIdsByRepo,
   listScheduleJobsByRepo: mocks.listScheduleJobsByRepo,
   listScheduleRunsByJob: mocks.listScheduleRunsByJob,
   updateScheduleJob: mocks.updateScheduleJob,
@@ -728,6 +728,20 @@ describe('ScheduleService', () => {
     expect(() => service.getRun(42, 7, 5)).toThrow('Run not found')
   })
 
+  it('prepares repo deletion by unregistering repo jobs without deleting records', () => {
+    const service = new ScheduleService({} as never, createOpenCodeClientStub())
+    const onJobChange = vi.fn()
+    service.setJobChangeHandler(onJobChange)
+    mocks.listScheduleJobIdsByRepo.mockReturnValue([7, 8])
+
+    service.prepareRepoDelete(42)
+
+    expect(mocks.listScheduleJobIdsByRepo).toHaveBeenCalledWith(expect.anything(), 42)
+    expect(onJobChange).toHaveBeenCalledWith(null, 7)
+    expect(onJobChange).toHaveBeenCalledWith(null, 8)
+    expect(onJobChange).toHaveBeenCalledTimes(2)
+  })
+
   it('cancels by finalizing the run when the assistant already completed', async () => {
     const service = new ScheduleService({} as never, createOpenCodeClientStub())
     const runningRun: ScheduleRun = {

frontend/src/pages/__tests__/Schedules.test.tsx

@@ -90,7 +90,6 @@ function createMockScheduleUrlState(overrides: Record<string, unknown> = {}) {
     openImportTemplate: vi.fn(),
     closeDialog: vi.fn(),
     closePromptDialog: vi.fn(),
-    selectJob: vi.fn(),
     selectRun: vi.fn(),
     selectJobAndView: vi.fn(),
     selectJobAndCloseDialog: vi.fn(),