diff --git a/go/core/internal/database/client_test.go b/go/core/internal/database/client_test.go index 16f10bf90..4f99327bb 100644 --- a/go/core/internal/database/client_test.go +++ b/go/core/internal/database/client_test.go @@ -240,6 +240,100 @@ func TestStoreSessionIdempotence(t *testing.T) { require.Error(t, err, "another user's session must not be readable") } +// TestSessionRecreateAfterDelete verifies that a session id can be reused +// after DeleteSession: the upsert resurrects the soft-deleted row and the +// recreated session starts empty (no events, tasks, or shares from the +// previous incarnation). +func TestSessionRecreateAfterDelete(t *testing.T) { + db := setupTestDB(t) + client := NewClient(db) + ctx := context.Background() + + const sessionID = "s-reborn" + const userID = "alice" + + require.NoError(t, client.StoreSession(ctx, &dbpkg.Session{ID: sessionID, UserID: userID})) + require.NoError(t, client.StoreEvents(ctx, + &dbpkg.Event{ID: "e-old-1", SessionID: sessionID, UserID: userID, Data: "{}"}, + &dbpkg.Event{ID: "e-old-2", SessionID: sessionID, UserID: userID, Data: "{}"}, + )) + require.NoError(t, client.StoreTask(ctx, &a2a.Task{ID: "t-old", ContextID: sessionID}, userID)) + _, err := client.CreateSessionShare(ctx, &dbpkg.SessionShare{Token: "share-old", SessionID: sessionID, UserID: userID}) + require.NoError(t, err) + + // An unrelated user's session under the same id must survive alice's + // delete/recreate cycle untouched. + require.NoError(t, client.StoreSession(ctx, &dbpkg.Session{ID: sessionID, UserID: "bob"})) + require.NoError(t, client.StoreEvents(ctx, + &dbpkg.Event{ID: "e-bob", SessionID: sessionID, UserID: "bob", Data: "{}"}, + )) + + require.NoError(t, client.DeleteSession(ctx, sessionID, userID)) + _, err = client.GetSession(ctx, sessionID, userID) + require.Error(t, err, "deleted session must not be readable") + + require.NoError(t, client.StoreSession(ctx, &dbpkg.Session{ID: sessionID, UserID: userID}), + "recreating a deleted session id must succeed") + + recreated, err := client.GetSession(ctx, sessionID, userID) + require.NoError(t, err, "recreated session must be readable") + require.Equal(t, sessionID, recreated.ID) + + events, err := client.ListEventsForSession(ctx, sessionID, userID, dbpkg.QueryOptions{}) + require.NoError(t, err) + require.Empty(t, events, "recreated session must not inherit the previous incarnation's events") + + tasks, err := client.ListTasksForSession(ctx, sessionID, userID) + require.NoError(t, err) + require.Empty(t, tasks, "recreated session must not inherit the previous incarnation's tasks") + + shares, err := client.ListSessionSharesBySession(ctx, sessionID) + require.NoError(t, err) + require.Empty(t, shares, "recreated session must not inherit the previous incarnation's shares") + + // The recreated session behaves like a fresh one. + require.NoError(t, client.StoreEvents(ctx, + &dbpkg.Event{ID: "e-new", SessionID: sessionID, UserID: userID, Data: "{}"}, + )) + events, err = client.ListEventsForSession(ctx, sessionID, userID, dbpkg.QueryOptions{}) + require.NoError(t, err) + require.Len(t, events, 1) + + // bob's same-id session kept its events. + bobEvents, err := client.ListEventsForSession(ctx, sessionID, "bob", dbpkg.QueryOptions{}) + require.NoError(t, err) + require.Len(t, bobEvents, 1, "another user's same-id session must be untouched") +} + +// TestUpsertLiveSessionKeepsHistory verifies that upserting a session that was +// never deleted does not purge its events or reset its creation time. +func TestUpsertLiveSessionKeepsHistory(t *testing.T) { + db := setupTestDB(t) + client := NewClient(db) + ctx := context.Background() + + const sessionID = "s-live" + const userID = "alice" + + require.NoError(t, client.StoreSession(ctx, &dbpkg.Session{ID: sessionID, UserID: userID})) + require.NoError(t, client.StoreEvents(ctx, + &dbpkg.Event{ID: "e-live", SessionID: sessionID, UserID: userID, Data: "{}"}, + )) + created, err := client.GetSession(ctx, sessionID, userID) + require.NoError(t, err) + + name := "renamed" + require.NoError(t, client.StoreSession(ctx, &dbpkg.Session{ID: sessionID, UserID: userID, Name: &name})) + + got, err := client.GetSession(ctx, sessionID, userID) + require.NoError(t, err) + require.Equal(t, created.CreatedAt, got.CreatedAt, "upserting a live session must not reset created_at") + + events, err := client.ListEventsForSession(ctx, sessionID, userID, dbpkg.QueryOptions{}) + require.NoError(t, err) + require.Len(t, events, 1, "upserting a live session must not purge events") +} + func TestListSessionsOrdersByRecentActivity(t *testing.T) { db := setupTestDB(t) client := NewClient(db) diff --git a/go/core/internal/database/gen/querier.go b/go/core/internal/database/gen/querier.go index f897decfe..c91e6c26d 100644 --- a/go/core/internal/database/gen/querier.go +++ b/go/core/internal/database/gen/querier.go @@ -86,6 +86,12 @@ type Querier interface { UpsertCrewAIFlowState(ctx context.Context, arg UpsertCrewAIFlowStateParams) error UpsertCrewAIMemory(ctx context.Context, arg UpsertCrewAIMemoryParams) error UpsertPushNotification(ctx context.Context, arg UpsertPushNotificationParams) error + // UpsertSession resurrects a soft-deleted (id, user_id) as a fresh incarnation: + // deleted_at is cleared, created_at restarts, and the previous incarnation's + // events, owned tasks, and shares are purged so the recreated session starts + // empty. NULL-owned tasks are left alone: the created_at bump already hides + // them from this session (see the ownership bound in tasks.sql), and they may + // still resolve to another user's same-id session. UpsertSession(ctx context.Context, arg UpsertSessionParams) error UpsertShareAccess(ctx context.Context, arg UpsertShareAccessParams) error // UpsertTask returns the upserted id, or no rows when the write was rejected: diff --git a/go/core/internal/database/gen/sessions.sql.go b/go/core/internal/database/gen/sessions.sql.go index 89e86cb8d..6d9ebb5e4 100644 --- a/go/core/internal/database/gen/sessions.sql.go +++ b/go/core/internal/database/gen/sessions.sql.go @@ -192,13 +192,35 @@ func (q *Queries) SoftDeleteSession(ctx context.Context, arg SoftDeleteSessionPa } const upsertSession = `-- name: UpsertSession :exec +WITH prior AS ( + SELECT 1 AS resurrected FROM session + WHERE id = $1 AND user_id = $2 AND deleted_at IS NOT NULL +), +purged_events AS ( + UPDATE event SET deleted_at = NOW() + WHERE session_id = $1 AND user_id = $2 AND deleted_at IS NULL + AND EXISTS (SELECT 1 FROM prior) +), +purged_tasks AS ( + UPDATE task SET deleted_at = NOW() + WHERE session_id = $1 AND user_id = $2 AND deleted_at IS NULL + AND EXISTS (SELECT 1 FROM prior) +), +purged_shares AS ( + DELETE FROM session_share + WHERE session_id = $1 AND user_id = $2 + AND EXISTS (SELECT 1 FROM prior) +) INSERT INTO session (id, user_id, name, agent_id, source, created_at, updated_at) VALUES ($1, $2, $3, $4, $5, NOW(), NOW()) ON CONFLICT (id, user_id) DO UPDATE SET name = EXCLUDED.name, agent_id = EXCLUDED.agent_id, source = EXCLUDED.source, - updated_at = NOW() + updated_at = NOW(), + deleted_at = NULL, + created_at = CASE WHEN session.deleted_at IS NOT NULL THEN NOW() + ELSE session.created_at END ` type UpsertSessionParams struct { @@ -209,6 +231,12 @@ type UpsertSessionParams struct { Source *string } +// UpsertSession resurrects a soft-deleted (id, user_id) as a fresh incarnation: +// deleted_at is cleared, created_at restarts, and the previous incarnation's +// events, owned tasks, and shares are purged so the recreated session starts +// empty. NULL-owned tasks are left alone: the created_at bump already hides +// them from this session (see the ownership bound in tasks.sql), and they may +// still resolve to another user's same-id session. func (q *Queries) UpsertSession(ctx context.Context, arg UpsertSessionParams) error { _, err := q.db.Exec(ctx, upsertSession, arg.ID, diff --git a/go/core/internal/database/queries/sessions.sql b/go/core/internal/database/queries/sessions.sql index d7066a9b9..81463ce00 100644 --- a/go/core/internal/database/queries/sessions.sql +++ b/go/core/internal/database/queries/sessions.sql @@ -32,14 +32,42 @@ WHERE agent_id = $1 AND deleted_at IS NULL AND (source IS NULL OR source != 'agent') ORDER BY updated_at DESC, created_at DESC; +-- UpsertSession resurrects a soft-deleted (id, user_id) as a fresh incarnation: +-- deleted_at is cleared, created_at restarts, and the previous incarnation's +-- events, owned tasks, and shares are purged so the recreated session starts +-- empty. NULL-owned tasks are left alone: the created_at bump already hides +-- them from this session (see the ownership bound in tasks.sql), and they may +-- still resolve to another user's same-id session. -- name: UpsertSession :exec +WITH prior AS ( + SELECT 1 AS resurrected FROM session + WHERE id = $1 AND user_id = $2 AND deleted_at IS NOT NULL +), +purged_events AS ( + UPDATE event SET deleted_at = NOW() + WHERE session_id = $1 AND user_id = $2 AND deleted_at IS NULL + AND EXISTS (SELECT 1 FROM prior) +), +purged_tasks AS ( + UPDATE task SET deleted_at = NOW() + WHERE session_id = $1 AND user_id = $2 AND deleted_at IS NULL + AND EXISTS (SELECT 1 FROM prior) +), +purged_shares AS ( + DELETE FROM session_share + WHERE session_id = $1 AND user_id = $2 + AND EXISTS (SELECT 1 FROM prior) +) INSERT INTO session (id, user_id, name, agent_id, source, created_at, updated_at) VALUES ($1, $2, $3, $4, $5, NOW(), NOW()) ON CONFLICT (id, user_id) DO UPDATE SET name = EXCLUDED.name, agent_id = EXCLUDED.agent_id, source = EXCLUDED.source, - updated_at = NOW(); + updated_at = NOW(), + deleted_at = NULL, + created_at = CASE WHEN session.deleted_at IS NOT NULL THEN NOW() + ELSE session.created_at END; -- name: SoftDeleteSession :exec UPDATE session SET deleted_at = NOW()