Skip to content

fix(sessions): detach child sessions on delete to avoid FK rollback #710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
laofoot wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sessions): detach child sessions on delete to avoid FK rollback #710

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/main/sessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,9 +724,39 @@ function normalizeSessionIds(sessionIds: string[]): string[] {
return normalized;
}

// sessions has a self-referential FK parent_session_id -> sessions.id (set by
// the agent for subagent runs / branches). Cached after first lookup.
let sessionsHasParentColumn: boolean | null = null;
function hasParentSessionColumn(db: Database.Database): boolean {
if (sessionsHasParentColumn !== null) return sessionsHasParentColumn;
try {
sessionsHasParentColumn =
db
.prepare(
"SELECT 1 FROM pragma_table_info('sessions') WHERE name = 'parent_session_id'",
)
.get() != null;
} catch {
sessionsHasParentColumn = false;
}
return sessionsHasParentColumn;
}
Comment on lines +729 to +743

@greptile-apps greptile-apps Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Module-level cache ignores the db argument

sessionsHasParentColumn is a process-singleton, but hasParentSessionColumn accepts a db argument. Two problems follow:

  1. Schema migration after first call: if the column doesn't exist at startup, the cache is set to false. A migration that later adds parent_session_id (during the same process lifetime) won't be detected, so the detach step is silently skipped and the FK error returns.
  2. Tests or multiple DB paths: any test suite that opens a second database without parent_session_id — or vice versa — will get the wrong cached answer for every subsequent call.

A simple fix is to key the cache on the database filename (db.name) or to clear sessionsHasParentColumn = null whenever the active database handle changes.


function deleteSessionRows(db: Database.Database, sessionId: string): number {
deletePromptImageAttachmentsForSession(db, sessionId);
deleteSessionContinuationForSession(db, sessionId);
// Unlink any child sessions first. better-sqlite3 enables
// PRAGMA foreign_keys=ON by default, so deleting a parent while a child
// still references it via parent_session_id throws "FOREIGN KEY constraint
// failed", which rolls back the whole delete transaction -> 0 rows deleted
// with no surfaced error (and a select-all batch deletes nothing, since one
// violation aborts the entire transaction). Detach children rather than
// cascade-delete so a session the user didn't select is never removed.
if (hasParentSessionColumn(db)) {
db.prepare(
"UPDATE sessions SET parent_session_id = NULL WHERE parent_session_id = ?",
).run(sessionId);
}
db.prepare("DELETE FROM messages WHERE session_id = ?").run(sessionId);
const result = db.prepare("DELETE FROM sessions WHERE id = ?").run(sessionId);
return result.changes;
Expand Down