Skip to content

fix(db): cascade soft-delete tasks and events on session delete#2207

Open
mesutoezdil wants to merge 1 commit into
kagent-dev:mainfrom
mesutoezdil:fix/delete-session-cascade
Open

fix(db): cascade soft-delete tasks and events on session delete#2207
mesutoezdil wants to merge 1 commit into
kagent-dev:mainfrom
mesutoezdil:fix/delete-session-cascade

Conversation

@mesutoezdil

Copy link
Copy Markdown
Contributor

deleting a session left related task, event, and session_share rows untouched. this wraps DeleteSession in a transaction that soft-deletes tasks and events and hard-deletes session shares before marking the session itself as deleted.

Copilot AI review requested due to automatic review settings July 10, 2026 20:30
@mesutoezdil mesutoezdil requested a review from a team as a code owner July 10, 2026 20:30
@github-actions github-actions Bot added the bug Something isn't working label Jul 10, 2026

Copilot AI left a comment

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.

Pull request overview

This PR updates the Postgres deletion path for sessions so that removing a session also cleans up related data, instead of leaving orphaned task/event/share rows.

Changes:

  • Add sqlc queries to soft-delete all tasks and events for a session.
  • Add a sqlc query to hard-delete all session shares for a session.
  • Wrap DeleteSession in a transaction to execute the cascade cleanup and then soft-delete the session row.

Reviewed changes

Copilot reviewed 4 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
go/core/internal/database/queries/tasks.sql Adds SoftDeleteTasksBySession SQL query.
go/core/internal/database/queries/session_shares.sql Adds DeleteSessionSharesBySession SQL query.
go/core/internal/database/queries/events.sql Adds SoftDeleteEventsBySession SQL query.
go/core/internal/database/gen/tasks.sql.go Adds generated Go method for SoftDeleteTasksBySession.
go/core/internal/database/gen/session_shares.sql.go Adds generated Go method for DeleteSessionSharesBySession.
go/core/internal/database/gen/querier.go Extends the sqlc Querier interface with the new methods.
go/core/internal/database/gen/events.sql.go Adds generated Go method for SoftDeleteEventsBySession.
go/core/internal/database/client_postgres.go Wraps DeleteSession in a transaction and invokes the new cascade operations.
Files not reviewed (4)
  • go/core/internal/database/gen/events.sql.go: Generated file
  • go/core/internal/database/gen/querier.go: Generated file
  • go/core/internal/database/gen/session_shares.sql.go: Generated file
  • go/core/internal/database/gen/tasks.sql.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread go/core/internal/database/client_postgres.go
Comment thread go/core/internal/database/client_postgres.go
@mesutoezdil mesutoezdil force-pushed the fix/delete-session-cascade branch 7 times, most recently from 3873191 to ae4be95 Compare July 14, 2026 13:03
Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil mesutoezdil force-pushed the fix/delete-session-cascade branch from 97b57fb to 96d6214 Compare July 14, 2026 13:48
func (c *postgresClient) DeleteSession(ctx context.Context, sessionID, userID string) error {
return c.q.SoftDeleteSession(ctx, dbgen.SoftDeleteSessionParams{ID: sessionID, UserID: userID})
return c.withTx(ctx, func(q *dbgen.Queries) error {
if _, err := q.GetSession(ctx, dbgen.GetSessionParams{ID: sessionID, UserID: userID}); err != nil {

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.

Any reason we're not using CASCADE deletes via SQL for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Any reason we're not using CASCADE deletes via SQL for this?

no fk exists between session and task/event/session_share right now
even if it did, tasks and events r soft deleted, and ON DELETE CASCADE only fires on a real DELETE and not an UPDATE.
and session itself is soft deleted too, so cascade would never trigger
that's why this is explicit app code instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants