Skip to content

Commit cc89a52

Browse files
fix(schedules): clean up orphaned schedule records when repo is deleted (#255)
* fix(schedules): clean up orphaned schedule records and add schedule URL state management * refactor(schedules): consolidate schedule tab state into useScheduleUrlState * fix(schedules): cascade delete schedule runs/jobs and preserve assistant schedules in orphan cleanup * refactor(schedules): replace deleteJobsByRepo with prepareRepoDelete, add lightweight ID query, remove dead mock * refactor(schedules): consolidate PromptTemplateDialog form state into single object
1 parent 70cb62b commit cc89a52

24 files changed

Lines changed: 1937 additions & 326 deletions

backend/src/db/queries.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ export function updateRepoBranch(db: Database, id: number, branch: string): void
209209
}
210210

211211
export function deleteRepo(db: Database, id: number): void {
212+
db.prepare('DELETE FROM schedule_runs WHERE repo_id = ?').run(id)
213+
db.prepare('DELETE FROM schedule_jobs WHERE repo_id = ?').run(id)
212214
const stmt = db.prepare('DELETE FROM repos WHERE id = ?')
213215
stmt.run(id)
214216
}
215-

backend/src/db/schedules.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ export function listScheduleJobsByRepo(db: Database, repoId: number): ScheduleJo
116116
return rows.map(rowToScheduleJob)
117117
}
118118

119+
export function listScheduleJobIdsByRepo(db: Database, repoId: number): number[] {
120+
const stmt = db.prepare('SELECT id FROM schedule_jobs WHERE repo_id = ? ORDER BY created_at DESC')
121+
const rows = stmt.all(repoId) as Array<{ id: number }>
122+
return rows.map((row) => row.id)
123+
}
124+
119125
export function listEnabledScheduleJobs(db: Database): ScheduleJob[] {
120126
const stmt = db.prepare('SELECT * FROM schedule_jobs WHERE enabled = 1 ORDER BY id ASC')
121127
const rows = stmt.all() as ScheduleJobRow[]
@@ -200,11 +206,29 @@ export function updateScheduleJob(db: Database, repoId: number, jobId: number, i
200206
}
201207

202208
export function deleteScheduleJob(db: Database, repoId: number, jobId: number): boolean {
209+
db.prepare('DELETE FROM schedule_runs WHERE repo_id = ? AND job_id = ?').run(repoId, jobId)
203210
const stmt = db.prepare('DELETE FROM schedule_jobs WHERE repo_id = ? AND id = ?')
204211
const result = stmt.run(repoId, jobId)
205212
return result.changes > 0
206213
}
207214

215+
export function cleanupOrphanedSchedules(db: Database): { orphanedJobs: number; orphanedRuns: number } {
216+
const runStmt = db.prepare(`
217+
DELETE FROM schedule_runs
218+
WHERE (repo_id != ? AND repo_id NOT IN (SELECT id FROM repos))
219+
OR job_id NOT IN (SELECT id FROM schedule_jobs)
220+
`)
221+
const orphanedRuns = runStmt.run(ASSISTANT_REPO_ID).changes
222+
223+
const jobStmt = db.prepare(`
224+
DELETE FROM schedule_jobs
225+
WHERE repo_id != ? AND repo_id NOT IN (SELECT id FROM repos)
226+
`)
227+
const orphanedJobs = jobStmt.run(ASSISTANT_REPO_ID).changes
228+
229+
return { orphanedJobs, orphanedRuns }
230+
}
231+
208232
export function updateScheduleJobRunState(db: Database, repoId: number, jobId: number, values: { lastRunAt: number; nextRunAt?: number | null }): void {
209233
const stmt = db.prepare('UPDATE schedule_jobs SET last_run_at = ?, next_run_at = ?, updated_at = ? WHERE repo_id = ? AND id = ?')
210234
stmt.run(values.lastRunAt, values.nextRunAt ?? null, Date.now(), repoId, jobId)

backend/src/routes/repos.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ function createTestApp(db: Database, openCodeClient: OpenCodeClient = createStub
3737
listSchedules: () => [],
3838
updateSchedule: () => {},
3939
deleteSchedule: () => {},
40+
prepareRepoDelete: () => {},
4041
} as any
4142
app.route('/repos', createRepoRoutes(db, stubGitAuthService, scheduleService, openCodeClient))
4243
return app

backend/src/routes/repos.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ app.get('/', async (c) => {
273273
return c.json({ error: 'Repo not found' }, 404)
274274
}
275275

276+
scheduleService.prepareRepoDelete(id)
277+
276278
await repoService.deleteRepoFiles(database, id)
277279

278280
return c.json({ success: true })

backend/src/services/schedules.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import { getRepoById } from '../db/queries'
1111
import type { ScheduleJobWithRepo } from '../db/schedules'
1212
import {
13+
cleanupOrphanedSchedules,
1314
createScheduleJob,
1415
createScheduleRun,
1516
deleteScheduleJob,
@@ -19,6 +20,7 @@ import {
1920
listAllScheduleJobsWithRepos,
2021
listAllScheduleRuns,
2122
listEnabledScheduleJobs,
23+
listScheduleJobIdsByRepo,
2224
listScheduleJobsByRepo,
2325
listRunningScheduleRuns,
2426
listScheduleRunsByJob,
@@ -444,6 +446,26 @@ export class ScheduleService {
444446
this.onJobChange?.(null, jobId)
445447
}
446448

449+
prepareRepoDelete(repoId: number): void {
450+
const jobIds = listScheduleJobIdsByRepo(this.db, repoId)
451+
for (const jobId of jobIds) {
452+
this.onJobChange?.(null, jobId)
453+
}
454+
}
455+
456+
/**
457+
* Removes any schedule_jobs and schedule_runs whose repo_id (or job_id for
458+
* runs) no longer exists in the repos / schedule_jobs table. Safe to call
459+
* on every startup — no-op when there are no orphans.
460+
*/
461+
cleanupOrphanedSchedules(): { orphanedJobs: number; orphanedRuns: number } {
462+
const result = cleanupOrphanedSchedules(this.db)
463+
if (result.orphanedJobs > 0 || result.orphanedRuns > 0) {
464+
logger.info(`Cleaned up ${result.orphanedJobs} orphaned schedule job(s) and ${result.orphanedRuns} run(s)`)
465+
}
466+
return result
467+
}
468+
447469
listRuns(repoId: number, jobId: number, limit: number = 20): ScheduleRun[] {
448470
this.assertJob(repoId, jobId)
449471
return listScheduleRunsByJob(this.db, repoId, jobId, limit)
@@ -1120,6 +1142,11 @@ export class ScheduleRunner {
11201142
}
11211143
})
11221144

1145+
// Clean up any schedule records whose repo no longer exists. This handles
1146+
// leftovers from before foreign-key enforcement was enabled, and guards
1147+
// against edge cases where a repo row was removed outside the normal flow.
1148+
this.scheduleService.cleanupOrphanedSchedules()
1149+
11231150
await this.scheduleService.recoverRunningRuns()
11241151
this.registerAllEnabledJobs()
11251152
}

backend/test/db/queries.test.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -258,18 +258,31 @@ describe('Database Queries', () => {
258258
})
259259

260260
describe('deleteRepo', () => {
261-
it('should delete repo by ID', () => {
262-
const stmt = {
261+
it('should delete repo schedules before deleting repo by ID', () => {
262+
const deleteRunsStmt = {
263+
run: vi.fn().mockReturnValue({ changes: 2 })
264+
}
265+
const deleteJobsStmt = {
263266
run: vi.fn().mockReturnValue({ changes: 1 })
264267
}
265-
mockDb.prepare.mockReturnValue(stmt)
268+
const deleteRepoStmt = {
269+
run: vi.fn().mockReturnValue({ changes: 1 })
270+
}
271+
mockDb.prepare
272+
.mockReturnValueOnce(deleteRunsStmt)
273+
.mockReturnValueOnce(deleteJobsStmt)
274+
.mockReturnValueOnce(deleteRepoStmt)
266275

267276
db.deleteRepo(mockDb, 1)
268277

269-
expect(mockDb.prepare).toHaveBeenCalledWith(
278+
expect(mockDb.prepare).toHaveBeenNthCalledWith(1, 'DELETE FROM schedule_runs WHERE repo_id = ?')
279+
expect(deleteRunsStmt.run).toHaveBeenCalledWith(1)
280+
expect(mockDb.prepare).toHaveBeenNthCalledWith(2, 'DELETE FROM schedule_jobs WHERE repo_id = ?')
281+
expect(deleteJobsStmt.run).toHaveBeenCalledWith(1)
282+
expect(mockDb.prepare).toHaveBeenNthCalledWith(3,
270283
'DELETE FROM repos WHERE id = ?'
271284
)
272-
expect(stmt.run).toHaveBeenCalledWith(1)
285+
expect(deleteRepoStmt.run).toHaveBeenCalledWith(1)
273286
})
274287
})
275288

backend/test/db/schedules-assistant.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Database } from 'bun:sqlite'
33
import { migrate } from '../../src/db/migration-runner'
44
import { allMigrations } from '../../src/db/migrations'
55
import {
6+
cleanupOrphanedSchedules,
67
listAllScheduleJobsWithRepos,
78
listAllScheduleRuns,
89
} from '../../src/db/schedules'
@@ -101,4 +102,14 @@ describe('assistant repo (repo_id=0) in global aggregate queries', () => {
101102
expect(run.repoName).toBe('Assistant')
102103
expect(run.repoPath).toBe('assistant')
103104
})
105+
106+
it('cleanupOrphanedSchedules keeps assistant schedules while deleting real repo orphans', () => {
107+
db.exec('DELETE FROM repos WHERE id = 1')
108+
109+
const result = cleanupOrphanedSchedules(db)
110+
111+
expect(result).toEqual({ orphanedJobs: 1, orphanedRuns: 1 })
112+
expect(listAllScheduleJobsWithRepos(db).map((job) => job.repoId)).toEqual([0])
113+
expect(listAllScheduleRuns(db, {}).map((run) => run.repoId)).toEqual([0])
114+
})
104115
})

backend/test/db/schedules.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,19 @@ describe('schedule database queries', () => {
7272
})
7373
})
7474

75+
it('lists schedule job ids without loading full job rows', () => {
76+
const stmt = {
77+
all: vi.fn().mockReturnValue([{ id: 7 }, { id: 8 }]),
78+
}
79+
mockDb.prepare.mockReturnValue(stmt)
80+
81+
const jobIds = schedulesDb.listScheduleJobIdsByRepo(mockDb, 42)
82+
83+
expect(mockDb.prepare).toHaveBeenCalledWith('SELECT id FROM schedule_jobs WHERE repo_id = ? ORDER BY created_at DESC')
84+
expect(stmt.all).toHaveBeenCalledWith(42)
85+
expect(jobIds).toEqual([7, 8])
86+
})
87+
7588
it('creates a schedule job and reloads the inserted row', () => {
7689
const insertStmt = {
7790
run: vi.fn().mockReturnValue({ lastInsertRowid: 7 }),
@@ -173,6 +186,27 @@ describe('schedule database queries', () => {
173186
})
174187
})
175188

189+
it('deletes schedule runs before deleting a schedule job', () => {
190+
const deleteRunsStmt = {
191+
run: vi.fn().mockReturnValue({ changes: 2 }),
192+
}
193+
const deleteJobStmt = {
194+
run: vi.fn().mockReturnValue({ changes: 1 }),
195+
}
196+
197+
mockDb.prepare
198+
.mockReturnValueOnce(deleteRunsStmt)
199+
.mockReturnValueOnce(deleteJobStmt)
200+
201+
const deleted = schedulesDb.deleteScheduleJob(mockDb, 42, 7)
202+
203+
expect(mockDb.prepare).toHaveBeenNthCalledWith(1, 'DELETE FROM schedule_runs WHERE repo_id = ? AND job_id = ?')
204+
expect(deleteRunsStmt.run).toHaveBeenCalledWith(42, 7)
205+
expect(mockDb.prepare).toHaveBeenNthCalledWith(2, 'DELETE FROM schedule_jobs WHERE repo_id = ? AND id = ?')
206+
expect(deleteJobStmt.run).toHaveBeenCalledWith(42, 7)
207+
expect(deleted).toBe(true)
208+
})
209+
176210
it('returns null when updating metadata for a missing run', () => {
177211
const selectStmt = {
178212
get: vi.fn().mockReturnValue(undefined),

backend/test/services/schedules.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ const mocks = vi.hoisted(() => ({
66
createScheduleJob: vi.fn(),
77
createScheduleRun: vi.fn(),
88
deleteScheduleJob: vi.fn(),
9+
cleanupOrphanedSchedules: vi.fn(),
910
getScheduleJobById: vi.fn(),
1011
getRunningScheduleRunByJob: vi.fn(),
1112
getScheduleRunById: vi.fn(),
1213
listEnabledScheduleJobs: vi.fn(),
1314
listRunningScheduleRuns: vi.fn(),
15+
listScheduleJobIdsByRepo: vi.fn(),
1416
listScheduleJobsByRepo: vi.fn(),
1517
listScheduleRunsByJob: vi.fn(),
1618
updateScheduleJob: vi.fn(),
@@ -35,11 +37,13 @@ vi.mock('../../src/db/schedules', () => ({
3537
createScheduleJob: mocks.createScheduleJob,
3638
createScheduleRun: mocks.createScheduleRun,
3739
deleteScheduleJob: mocks.deleteScheduleJob,
40+
cleanupOrphanedSchedules: mocks.cleanupOrphanedSchedules,
3841
getScheduleJobById: mocks.getScheduleJobById,
3942
getRunningScheduleRunByJob: mocks.getRunningScheduleRunByJob,
4043
getScheduleRunById: mocks.getScheduleRunById,
4144
listEnabledScheduleJobs: mocks.listEnabledScheduleJobs,
4245
listRunningScheduleRuns: mocks.listRunningScheduleRuns,
46+
listScheduleJobIdsByRepo: mocks.listScheduleJobIdsByRepo,
4347
listScheduleJobsByRepo: mocks.listScheduleJobsByRepo,
4448
listScheduleRunsByJob: mocks.listScheduleRunsByJob,
4549
updateScheduleJob: mocks.updateScheduleJob,
@@ -724,6 +728,20 @@ describe('ScheduleService', () => {
724728
expect(() => service.getRun(42, 7, 5)).toThrow('Run not found')
725729
})
726730

731+
it('prepares repo deletion by unregistering repo jobs without deleting records', () => {
732+
const service = new ScheduleService({} as never, createOpenCodeClientStub())
733+
const onJobChange = vi.fn()
734+
service.setJobChangeHandler(onJobChange)
735+
mocks.listScheduleJobIdsByRepo.mockReturnValue([7, 8])
736+
737+
service.prepareRepoDelete(42)
738+
739+
expect(mocks.listScheduleJobIdsByRepo).toHaveBeenCalledWith(expect.anything(), 42)
740+
expect(onJobChange).toHaveBeenCalledWith(null, 7)
741+
expect(onJobChange).toHaveBeenCalledWith(null, 8)
742+
expect(onJobChange).toHaveBeenCalledTimes(2)
743+
})
744+
727745
it('cancels by finalizing the run when the assistant already completed', async () => {
728746
const service = new ScheduleService({} as never, createOpenCodeClientStub())
729747
const runningRun: ScheduleRun = {
@@ -998,6 +1016,7 @@ describe('ScheduleRunner', () => {
9981016
beforeEach(() => {
9991017
mockCronInstances.length = 0
10001018
mockCronStop.mockClear()
1019+
mocks.cleanupOrphanedSchedules.mockReturnValue({ orphanedJobs: 0, orphanedRuns: 0 })
10011020
})
10021021

10031022
it('recovers running runs and registers all enabled jobs on start', async () => {

frontend/src/components/navigation/MobileTabBar.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { useLocation, useNavigate } from 'react-router-dom'
33
import { FolderGit2, FolderOpen, CalendarClock, Menu, Info, History, Bot } from 'lucide-react'
44
import { cn } from '@/lib/utils'
55
import { useMobile } from '@/hooks/useMobile'
6-
import { useMobileTabBar, useScheduleTab, type ScheduleTabKey } from '@/hooks/useMobileTabBar'
6+
import { useMobileTabBar } from '@/hooks/useMobileTabBar'
7+
import { useScheduleUrlState, type ScheduleTab } from '@/hooks/useScheduleUrlState'
78
import { useUIState } from '@/stores/uiStateStore'
89
import { getAssistantPath, isAssistantPath } from '@/lib/navigation'
910

@@ -124,7 +125,7 @@ function buildGlobalTabs({ pathname, search, openSheet, open, close, navigate, i
124125
]
125126
}
126127

127-
function buildScheduleTabs(scheduleTab: ScheduleTabKey, setScheduleTab: (tab: ScheduleTabKey) => void): TabDef[] {
128+
function buildScheduleTabs(scheduleTab: ScheduleTab, setScheduleTab: (tab: ScheduleTab) => void): TabDef[] {
128129
return [
129130
{
130131
key: 'jobs',
@@ -189,7 +190,7 @@ export const MobileTabBar = memo(function MobileTabBar() {
189190
const { pathname, search } = useLocation()
190191
const navigate = useNavigate()
191192
const { openSheet, open, close } = useMobileTabBar()
192-
const { scheduleTab, setScheduleTab } = useScheduleTab()
193+
const { scheduleTab, setScheduleTab } = useScheduleUrlState()
193194
const isMobile = useMobile()
194195
const isMoreDrawerOpen = useUIState((state) => state.isMoreDrawerOpen)
195196
const setMoreDrawerOpen = useUIState((state) => state.setMoreDrawerOpen)

0 commit comments

Comments
 (0)