|
| 1 | +import { describe, expect, it } from 'bun:test' |
| 2 | +import type { SyncEvent } from '@hapi/protocol/types' |
| 3 | +import { Store } from '../store' |
| 4 | +import type { EventPublisher } from './eventPublisher' |
| 5 | +import { SessionCache } from './sessionCache' |
| 6 | + |
| 7 | +/** |
| 8 | + * Regression tests for tiann/hapi#920: scratchlist rows must survive |
| 9 | + * the `mergeSessionData` codepath in `SessionCache`. |
| 10 | + * |
| 11 | + * Background: `mergeSessionData` ends with `deleteSession(oldSessionId)` |
| 12 | + * which fires `ON DELETE CASCADE` on every FK-tied table. The |
| 13 | + * `session_scratchlist` table joins on `sessions(id)` with cascade |
| 14 | + * delete, so without an explicit transfer step every dedup |
| 15 | + * (#448 agent-id collision) and every resume-of-inactive path |
| 16 | + * (`syncEngine.resumeSession`) silently destroys the operator's notes. |
| 17 | + * |
| 18 | + * Two codepaths: |
| 19 | + * - `mergeSessions(old, new, ns)` -> `mergeSessionData(deleteOld=true)` |
| 20 | + * - `mergeSessionHistory(old, new, ns, opts)` -> `mergeSessionData(deleteOld=false)` |
| 21 | + * |
| 22 | + * Both must transfer scratchlist rows. We pin both with their own |
| 23 | + * happy-path test plus a PK-collision test (same `entryId` on both |
| 24 | + * sides; the dedup target wins). |
| 25 | + */ |
| 26 | + |
| 27 | +function createCapturingPublisher(events: SyncEvent[]): EventPublisher { |
| 28 | + return { |
| 29 | + emit: (event: SyncEvent) => { |
| 30 | + events.push(event) |
| 31 | + } |
| 32 | + } as unknown as EventPublisher |
| 33 | +} |
| 34 | + |
| 35 | +function setup() { |
| 36 | + const store = new Store(':memory:') |
| 37 | + const events: SyncEvent[] = [] |
| 38 | + const cache = new SessionCache(store, createCapturingPublisher(events)) |
| 39 | + return { store, events, cache } |
| 40 | +} |
| 41 | + |
| 42 | +function makeSessions(cache: SessionCache, ns: string = 'default') { |
| 43 | + const oldSession = cache.getOrCreateSession( |
| 44 | + 'agent-merge-old-' + Math.random().toString(36).slice(2, 8), |
| 45 | + { path: '/tmp/project', host: 'localhost', flavor: 'codex' }, |
| 46 | + null, |
| 47 | + ns |
| 48 | + ) |
| 49 | + const newSession = cache.getOrCreateSession( |
| 50 | + 'agent-merge-new-' + Math.random().toString(36).slice(2, 8), |
| 51 | + { path: '/tmp/project', host: 'localhost', flavor: 'codex' }, |
| 52 | + null, |
| 53 | + ns |
| 54 | + ) |
| 55 | + return { oldSession, newSession } |
| 56 | +} |
| 57 | + |
| 58 | +describe('mergeSessions (deleteOldSession=true) - scratchlist transfer', () => { |
| 59 | + it('moves scratchlist rows from old to new before the cascade-delete fires', async () => { |
| 60 | + const { store, cache } = setup() |
| 61 | + const { oldSession, newSession } = makeSessions(cache) |
| 62 | + |
| 63 | + store.scratchlist.create(oldSession.id, 'note one', { entryId: 'e-1', createdAt: 100 }) |
| 64 | + store.scratchlist.create(oldSession.id, 'note two', { entryId: 'e-2', createdAt: 200 }) |
| 65 | + |
| 66 | + await cache.mergeSessions(oldSession.id, newSession.id, 'default') |
| 67 | + |
| 68 | + // New session now owns the rows. |
| 69 | + const onNew = store.scratchlist.list(newSession.id).map((e) => e.entryId).sort() |
| 70 | + expect(onNew).toEqual(['e-1', 'e-2']) |
| 71 | + |
| 72 | + // Old session is gone (deleteOldSession=true) AND its rows |
| 73 | + // are not stranded on a phantom session id. |
| 74 | + expect(store.scratchlist.list(oldSession.id)).toEqual([]) |
| 75 | + }) |
| 76 | + |
| 77 | + it('handles entryId PK collision by keeping the dedup target row (operator-visible session wins)', async () => { |
| 78 | + const { store, cache } = setup() |
| 79 | + const { oldSession, newSession } = makeSessions(cache) |
| 80 | + |
| 81 | + // Both sessions have an entry at the same id - the new wins. |
| 82 | + store.scratchlist.create(oldSession.id, 'OLD copy', { entryId: 'shared-id', createdAt: 100 }) |
| 83 | + store.scratchlist.create(newSession.id, 'NEW copy', { entryId: 'shared-id', createdAt: 200 }) |
| 84 | + store.scratchlist.create(oldSession.id, 'unique to old', { entryId: 'old-only', createdAt: 50 }) |
| 85 | + |
| 86 | + await cache.mergeSessions(oldSession.id, newSession.id, 'default') |
| 87 | + |
| 88 | + const final = store.scratchlist.list(newSession.id) |
| 89 | + const byId = new Map(final.map((e) => [e.entryId, e.text])) |
| 90 | + expect(byId.get('shared-id')).toBe('NEW copy') |
| 91 | + expect(byId.get('old-only')).toBe('unique to old') |
| 92 | + expect(final).toHaveLength(2) |
| 93 | + }) |
| 94 | + |
| 95 | + it('emits scratchlistUpdatedAt on the new session (and not on the old one - it is about to be removed)', async () => { |
| 96 | + const { store, events, cache } = setup() |
| 97 | + const { oldSession, newSession } = makeSessions(cache) |
| 98 | + store.scratchlist.create(oldSession.id, 'note', { entryId: 'e-1', createdAt: 100 }) |
| 99 | + events.length = 0 |
| 100 | + |
| 101 | + await cache.mergeSessions(oldSession.id, newSession.id, 'default') |
| 102 | + |
| 103 | + const scratchPatches = events.filter((e) => { |
| 104 | + return e.type === 'session-updated' |
| 105 | + && typeof e.data === 'object' && e.data !== null |
| 106 | + && 'scratchlistUpdatedAt' in (e.data as Record<string, unknown>) |
| 107 | + }) |
| 108 | + // Exactly one - on the new session id. |
| 109 | + expect(scratchPatches).toHaveLength(1) |
| 110 | + expect(scratchPatches[0]!.type === 'session-updated' && scratchPatches[0]!.sessionId).toBe(newSession.id) |
| 111 | + }) |
| 112 | + |
| 113 | + it('is a no-op (no extra emit) when the old session has no scratchlist rows', async () => { |
| 114 | + const { events, cache } = setup() |
| 115 | + const { oldSession, newSession } = makeSessions(cache) |
| 116 | + events.length = 0 |
| 117 | + |
| 118 | + await cache.mergeSessions(oldSession.id, newSession.id, 'default') |
| 119 | + |
| 120 | + const scratchPatches = events.filter((e) => { |
| 121 | + return e.type === 'session-updated' |
| 122 | + && typeof e.data === 'object' && e.data !== null |
| 123 | + && 'scratchlistUpdatedAt' in (e.data as Record<string, unknown>) |
| 124 | + }) |
| 125 | + expect(scratchPatches).toHaveLength(0) |
| 126 | + }) |
| 127 | +}) |
| 128 | + |
| 129 | +describe('mergeSessionHistory (deleteOldSession=false) - scratchlist transfer', () => { |
| 130 | + it('moves scratchlist rows even when the old session row stays alive', async () => { |
| 131 | + const { store, cache } = setup() |
| 132 | + const { oldSession, newSession } = makeSessions(cache) |
| 133 | + |
| 134 | + store.scratchlist.create(oldSession.id, 'still-need-this', { entryId: 'e-1', createdAt: 100 }) |
| 135 | + |
| 136 | + // Active-duplicate codepath: keeps the live socket but moves |
| 137 | + // the persisted history into the dedup target. Scratchlist |
| 138 | + // is "persisted history" for this purpose. |
| 139 | + await cache.mergeSessionHistory(oldSession.id, newSession.id, 'default', { mergeAgentState: false }) |
| 140 | + |
| 141 | + expect(store.scratchlist.list(newSession.id).map((e) => e.entryId)).toEqual(['e-1']) |
| 142 | + // Old row is still alive but its scratchlist is empty - the |
| 143 | + // operator-facing dedup target is now the source of truth. |
| 144 | + expect(store.scratchlist.list(oldSession.id)).toEqual([]) |
| 145 | + }) |
| 146 | + |
| 147 | + it('emits scratchlistUpdatedAt on BOTH the new and the still-alive old session id', async () => { |
| 148 | + const { store, events, cache } = setup() |
| 149 | + const { oldSession, newSession } = makeSessions(cache) |
| 150 | + store.scratchlist.create(oldSession.id, 'note', { entryId: 'e-1', createdAt: 100 }) |
| 151 | + events.length = 0 |
| 152 | + |
| 153 | + await cache.mergeSessionHistory(oldSession.id, newSession.id, 'default', { mergeAgentState: false }) |
| 154 | + |
| 155 | + const scratchPatches = events.filter((e) => { |
| 156 | + return e.type === 'session-updated' |
| 157 | + && typeof e.data === 'object' && e.data !== null |
| 158 | + && 'scratchlistUpdatedAt' in (e.data as Record<string, unknown>) |
| 159 | + }) |
| 160 | + // Two emits: one per session id, so any client looking at |
| 161 | + // either side invalidates and refetches. |
| 162 | + const ids = scratchPatches |
| 163 | + .map((e) => e.type === 'session-updated' ? e.sessionId : '') |
| 164 | + .sort() |
| 165 | + expect(ids).toEqual([oldSession.id, newSession.id].sort()) |
| 166 | + }) |
| 167 | +}) |
| 168 | + |
| 169 | +describe('cascade-delete safety (regression)', () => { |
| 170 | + it('without the transfer, the cascade would have nuked them - confirm by deleting the new session at the end', async () => { |
| 171 | + // This is a smoke test for the ON DELETE CASCADE on |
| 172 | + // `session_scratchlist.session_id` itself: after the merge |
| 173 | + // moves rows to the new session and the new session is |
| 174 | + // later deleted (e.g. operator clicks Delete), the rows |
| 175 | + // disappear too. This is the cascade we DO want; the bug |
| 176 | + // is that the merge codepath was triggering it on the OLD |
| 177 | + // id while the operator expected the data to follow the |
| 178 | + // new id. |
| 179 | + const { store, cache } = setup() |
| 180 | + const { oldSession, newSession } = makeSessions(cache) |
| 181 | + store.scratchlist.create(oldSession.id, 'note', { entryId: 'e-1', createdAt: 100 }) |
| 182 | + |
| 183 | + await cache.mergeSessions(oldSession.id, newSession.id, 'default') |
| 184 | + expect(store.scratchlist.list(newSession.id)).toHaveLength(1) |
| 185 | + |
| 186 | + // Now an explicit operator-driven delete of the new session. |
| 187 | + // Mark it inactive first because deleteSession refuses to |
| 188 | + // delete an active session. |
| 189 | + const cached = cache.getSession(newSession.id) |
| 190 | + if (cached) cached.active = false |
| 191 | + await cache.deleteSession(newSession.id) |
| 192 | + expect(store.scratchlist.list(newSession.id)).toEqual([]) |
| 193 | + }) |
| 194 | +}) |
0 commit comments