mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
fix(hub,web): deduplicate sessions by agent session ID (#448)
* fix(hub,web): deduplicate sessions by agent session ID When multiple CLI wrappers independently resume the same Codex thread, each generates a random tag, causing the hub to create duplicate session records for a single underlying thread. This leads to duplicate conversations in the web UI and messages routing to the wrong session. Add two-layer deduplication: - Hub: when a metadata update sets an agent session ID (codexSessionId, claudeSessionId, etc.) that already exists on another session in the same namespace, automatically merge the duplicate into the current session using the existing mergeSessions logic. - Web: deduplicate the session list display by agentSessionId as a safety net, keeping the active/most-recent session visible. Closes #446 * chore: add review-driven comments for dedup clarity - Explain single-threaded assumption in before/after metadata comparison - Document merge direction rationale (duplicate → active session) - Document deduplicateInProgress guard as known limitation - Add catch comment explaining web safety net fallback * fix: address review feedback from bot, Opus, and Codex - Skip active duplicates during hub-side dedup to avoid deleting sessions with live CLI sockets and pending agent state - Pass selectedSessionId into web dedup sort to prevent hiding the session the user is currently viewing - Add test for active-duplicate-not-merged case * fix: retry dedup on session-end and preserve agentState in merge - Trigger dedup when a session ends (handleSessionEnd), so active duplicates skipped during earlier dedup get merged once they disconnect - Preserve agentState from old session during mergeSessions when the new session has no agentState (mirrors existing model/effort/todos preservation pattern) - Extract triggerDedupIfNeeded helper for reuse across trigger points * fix(web): prefer active session over selected in dedup sort Active session always wins the dedup tie-break so the live connection is never hidden in favor of a selected inactive duplicate. Among inactive duplicates the selected one is still preferred. * fix: dedup on inactivity timeout and deep-merge agentState - expireInactive now returns expired session IDs so SyncEngine can trigger dedup for sessions that timed out (crash/network drop) instead of only on explicit session-end - mergeSessions now deep-merges agentState requests/completedRequests from both sessions instead of only copying when new is null * fix: exclude completed requests from merged pending set Filter out request IDs that already appear in completedRequests when merging agentState, preventing completed permission prompts from resurrecting as pending after session dedup. * fix: guard resume merge against prior auto-dedup The automatic dedup (triggered when the spawned CLI sets its agent session ID) can delete the old session before resumeSession reaches its own explicit mergeSessions call. Skip the merge if the old session no longer exists instead of failing the resume with a false error. * test: add coverage for dedup retry paths and web dedup sort Hub tests: - session-end triggers dedup retry for previously-active duplicates - inactivity timeout expiry triggers dedup retry - agentState deep merge filters completed requests from pending set Web tests: - basic dedup by agentSessionId - active session wins over inactive duplicate - selected session preferred among inactive duplicates - active always wins over selected inactive - sessions without agentSessionId pass through - independent dedup across different agentSessionIds * fix: read latest agentState before merge write to avoid overwriting live updates Re-read the target session's agentState right before writing the merged result, with a version-mismatch retry loop, so concurrent update-state events from the active CLI are not lost during dedup merge. * fix: sort expired sessions by recency before dedup When multiple duplicates for the same agent thread expire in a single sweep, process the most recent one first so it becomes the merge target and survives, rather than keeping the oldest by arbitrary iteration order. * fix: select most recent session as merge target in dedup deduplicateByAgentSessionId now collects all inactive candidates (including the caller) and picks the one with the highest activeAt (then updatedAt) as the merge target. This ensures the newest session survives regardless of which trigger point or ordering calls the dedup.
This commit is contained in:
Haoqing Wang
@@ -440,4 +440,262 @@ describe('session model', () => {
|
||||
engine.stop()
|
||||
}
|
||||
})
|
||||
|
||||
describe('session dedup by agent session ID', () => {
|
||||
it('merges duplicate when codexSessionId collides', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
// Add a message to s1
|
||||
store.messages.addMessage(s1.id, { type: 'text', text: 'hello from s1' }, 'local-1')
|
||||
|
||||
const s2 = cache.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
expect(s1.id).not.toBe(s2.id)
|
||||
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
|
||||
expect(cache.getSession(s1.id)).toBeUndefined()
|
||||
expect(cache.getSession(s2.id)).toBeDefined()
|
||||
|
||||
const messages = store.messages.getMessages(s2.id, 100)
|
||||
expect(messages.length).toBeGreaterThanOrEqual(1)
|
||||
})
|
||||
|
||||
it('preserves sessions with different agent session IDs', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
const s2 = cache.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-Y' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
|
||||
expect(cache.getSession(s1.id)).toBeDefined()
|
||||
expect(cache.getSession(s2.id)).toBeDefined()
|
||||
})
|
||||
|
||||
it('does not merge across namespaces', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'ns1'
|
||||
)
|
||||
const s2 = cache.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'ns2'
|
||||
)
|
||||
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
|
||||
expect(cache.getSession(s1.id)).toBeDefined()
|
||||
expect(cache.getSession(s2.id)).toBeDefined()
|
||||
})
|
||||
|
||||
it('no-op when session has no agent session ID', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
await cache.deduplicateByAgentSessionId(s1.id)
|
||||
|
||||
expect(cache.getSession(s1.id)).toBeDefined()
|
||||
})
|
||||
|
||||
it('does not merge active duplicates', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
// Mark s1 as active (simulating a live CLI connection)
|
||||
cache.handleSessionAlive({ sid: s1.id, time: Date.now(), thinking: false })
|
||||
|
||||
const s2 = cache.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
|
||||
// s1 is active, so it should NOT be merged/deleted
|
||||
expect(cache.getSession(s1.id)).toBeDefined()
|
||||
expect(cache.getSession(s2.id)).toBeDefined()
|
||||
})
|
||||
|
||||
it('merges duplicate after it becomes inactive via session-end', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const engine = new SyncEngine(
|
||||
store,
|
||||
{} as never,
|
||||
new RpcRegistry(),
|
||||
{ broadcast() {} } as never
|
||||
)
|
||||
|
||||
try {
|
||||
const s1 = engine.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
const s2 = engine.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
// Mark s1 as active
|
||||
engine.handleSessionAlive({ sid: s1.id, time: Date.now() })
|
||||
|
||||
// s1 is active, dedup from s2 should skip it
|
||||
const events: SyncEvent[] = []
|
||||
const cache = (engine as any).sessionCache as SessionCache
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
expect(cache.getSession(s1.id)).toBeDefined()
|
||||
expect(cache.getSession(s2.id)).toBeDefined()
|
||||
|
||||
// Now s1 ends — handleSessionEnd should trigger dedup retry
|
||||
engine.handleSessionEnd({ sid: s1.id, time: Date.now() })
|
||||
|
||||
// Give the fire-and-forget dedup a tick to complete
|
||||
await new Promise((r) => setTimeout(r, 50))
|
||||
|
||||
// One of them should be merged away
|
||||
const s1Exists = cache.getSession(s1.id)
|
||||
const s2Exists = cache.getSession(s2.id)
|
||||
expect(!s1Exists || !s2Exists).toBe(true)
|
||||
} finally {
|
||||
engine.stop()
|
||||
}
|
||||
})
|
||||
|
||||
it('merges duplicate after inactivity timeout expires it', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
const s2 = cache.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
null,
|
||||
'default'
|
||||
)
|
||||
|
||||
// Mark s1 as active now
|
||||
cache.handleSessionAlive({ sid: s1.id, time: Date.now() })
|
||||
|
||||
// s1 is active — dedup skips it
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
expect(cache.getSession(s1.id)).toBeDefined()
|
||||
|
||||
// Simulate time passing beyond the 30s timeout
|
||||
const expired = cache.expireInactive(Date.now() + 60_000)
|
||||
expect(expired).toContain(s1.id)
|
||||
|
||||
// Now s1 is inactive — dedup should merge it
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
expect(cache.getSession(s1.id)).toBeUndefined()
|
||||
expect(cache.getSession(s2.id)).toBeDefined()
|
||||
})
|
||||
|
||||
it('deep-merges agentState and filters completed requests', async () => {
|
||||
const store = new Store(':memory:')
|
||||
const events: SyncEvent[] = []
|
||||
const cache = new SessionCache(store, createPublisher(events))
|
||||
|
||||
const s1 = cache.getOrCreateSession(
|
||||
'tag-1',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
{
|
||||
requests: {
|
||||
'req-1': { tool: 'Bash', arguments: {} },
|
||||
'req-2': { tool: 'Bash', arguments: {} }
|
||||
},
|
||||
completedRequests: {}
|
||||
},
|
||||
'default'
|
||||
)
|
||||
const s2 = cache.getOrCreateSession(
|
||||
'tag-2',
|
||||
{ path: '/tmp/project', host: 'localhost', flavor: 'codex', codexSessionId: 'thread-X' },
|
||||
{
|
||||
requests: {
|
||||
'req-3': { tool: 'Bash', arguments: {} }
|
||||
},
|
||||
completedRequests: {
|
||||
'req-1': { tool: 'Bash', arguments: {}, status: 'approved' }
|
||||
}
|
||||
},
|
||||
'default'
|
||||
)
|
||||
|
||||
await cache.deduplicateByAgentSessionId(s2.id)
|
||||
|
||||
const session = cache.getSession(s2.id)
|
||||
expect(session).toBeDefined()
|
||||
const state = session!.agentState!
|
||||
|
||||
// req-1 was completed in s2 — should NOT appear in requests
|
||||
expect(state.requests?.['req-1']).toBeUndefined()
|
||||
// req-2 and req-3 are still pending
|
||||
expect(state.requests?.['req-2']).toBeDefined()
|
||||
expect(state.requests?.['req-3']).toBeDefined()
|
||||
// completedRequests has req-1
|
||||
expect(state.completedRequests?.['req-1']).toBeDefined()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user