From 0ad673794fdcee354ec460c48ff90819a978903d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 12 Jul 2026 12:41:48 +0200 Subject: [PATCH] fix(review): dedupe resumed-session rows + newest-wins lifecycle name/mode (PR #139) - Duplicate rows: transcript-history rows are keyed by the Claude conversation UUID (.jsonl filename stem), which diverges from the Codeman session id for resumed (claudeSessionId = resumeSessionId != id) and /clear-respawned sessions, so one conversation surfaced as both a live row and a history-only row. mergeUnifiedSessions now builds an alias map (claudeSessionId -> Codeman id) from the live + persisted views and resolves history/lifecycle keys through it; the route feeds SessionState.resumeSessionId as the persisted alias. - Inverted precedence: SessionLifecycleLog.query() returns entries NEWEST-first, but the merge loop unconditionally overwrote name/mode so the OLDEST entry in the window won (stale rename/mode). First-seen now wins, mirroring the existing lastActivityAt guard. - Tests: resumed session yields ONE row (service unit + route end-to-end with a real transcript fixture); renamed-then-deleted session surfaces the NEWEST name/mode. All 4 new tests fail against the pre-fix code. Co-Authored-By: Claude Fable 5 --- src/services/unified-session-service.ts | 40 ++++++++++---- src/web/routes/session-routes.ts | 5 +- test/routes/unified-sessions-routes.test.ts | 31 ++++++++++- test/services/unified-session-service.test.ts | 54 +++++++++++++++++++ 4 files changed, 119 insertions(+), 11 deletions(-) diff --git a/src/services/unified-session-service.ts b/src/services/unified-session-service.ts index 3d9c06f8..16fb7d11 100644 --- a/src/services/unified-session-service.ts +++ b/src/services/unified-session-service.ts @@ -4,8 +4,12 @@ * Combines four read-only views of a session — live (in-memory `Session`), * persisted (`state.json`), transcript history (`~/.claude/projects`), and the * lifecycle audit log — plus mux process stats, into one de-duplicated list - * keyed by sessionId. Higher-precedence sources overwrite scalar fields when - * present (history < lifecycle < persisted < live), while the `sources` array + * keyed by sessionId. Transcript-history rows are keyed by the Claude + * conversation UUID (the `.jsonl` filename stem), which diverges from the + * Codeman id for resumed sessions — an alias map (claudeSessionId → Codeman id, + * built from the live/persisted views) folds them into the owning session item. + * Higher-precedence sources overwrite scalar fields when present + * (history < lifecycle < persisted < live), while the `sources` array * always accumulates every contributing view. A "meaningfulness floor" drops * noise (bare lifecycle/mux-only rows with no name and no first prompt). * @@ -52,9 +56,11 @@ export type PersistedSessionInput = { workingDir?: string; createdAt?: number; lastActivityAt?: number; + /** Claude conversation ID this session resumes (`SessionState.resumeSessionId`). */ + claudeSessionId?: string; }; -/** Lifecycle audit-log view. */ +/** Lifecycle audit-log view. Entries are expected NEWEST-first (the order `SessionLifecycleLog.query()` returns). */ export type LifecycleInput = { sessionId: string; name?: string; @@ -120,9 +126,23 @@ function overwrite( export function mergeUnifiedSessions(sources: UnifiedSources): UnifiedSessionItem[] { const map = new Map(); - // 1) history (lowest precedence) + // Alias map: Claude conversation UUID → owning Codeman session id. Resumed + // (claudeSessionId = resumeSessionId != id) and /clear-respawned sessions + // would otherwise surface twice — once as a live/persisted row and once as a + // separate history-only row keyed by the conversation UUID. Live wins over + // persisted on conflicting entries (registered last). + const aliasToOwner = new Map(); + for (const p of sources.persisted ?? []) { + if (p.claudeSessionId !== undefined && p.claudeSessionId !== p.id) aliasToOwner.set(p.claudeSessionId, p.id); + } + for (const v of sources.live ?? []) { + if (v.claudeSessionId !== undefined && v.claudeSessionId !== v.id) aliasToOwner.set(v.claudeSessionId, v.id); + } + const resolveId = (sessionId: string): string => aliasToOwner.get(sessionId) ?? sessionId; + + // 1) history (lowest precedence; keys resolve through the alias map) for (const h of sources.history ?? []) { - const item = ensureItem(map, h.sessionId); + const item = ensureItem(map, resolveId(h.sessionId)); addSource(item, 'history'); overwrite(item, 'workingDir', h.workingDir); overwrite(item, 'sizeBytes', h.sizeBytes); @@ -131,12 +151,14 @@ export function mergeUnifiedSessions(sources: UnifiedSources): UnifiedSessionIte if (!Number.isNaN(ms) && item.lastActivityAt === undefined) item.lastActivityAt = ms; } - // 2) lifecycle + // 2) lifecycle — entries arrive NEWEST-first, so first-seen wins for + // name/mode (mirrors the lastActivityAt guard); unconditional overwrites + // would leave the OLDEST entry in the window (stale name/mode) standing. for (const l of sources.lifecycle ?? []) { - const item = ensureItem(map, l.sessionId); + const item = ensureItem(map, resolveId(l.sessionId)); addSource(item, 'lifecycle'); - overwrite(item, 'name', l.name); - overwrite(item, 'mode', l.mode); + if (item.name === undefined) overwrite(item, 'name', l.name); + if (item.mode === undefined) overwrite(item, 'mode', l.mode); if (item.lastActivityAt === undefined && typeof l.ts === 'number') item.lastActivityAt = l.ts; } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 259e300d..71b90770 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1784,7 +1784,9 @@ export function registerSessionRoutes( }; }); - // Persisted sessions (state.json). + // Persisted sessions (state.json). resumeSessionId is the Claude + // conversation UUID a resumed session continues — feed it to the merge's + // alias map so its transcript row folds into this session. const persisted: PersistedSessionInput[] = Object.values(ctx.store.getState().sessions).map((p) => ({ id: p.id, name: p.name, @@ -1793,6 +1795,7 @@ export function registerSessionRoutes( workingDir: p.workingDir, createdAt: p.createdAt, lastActivityAt: p.lastActivityAt, + claudeSessionId: p.resumeSessionId, })); // Lifecycle audit log (newest-first, capped). diff --git a/test/routes/unified-sessions-routes.test.ts b/test/routes/unified-sessions-routes.test.ts index e20cbb0f..d02a9c54 100644 --- a/test/routes/unified-sessions-routes.test.ts +++ b/test/routes/unified-sessions-routes.test.ts @@ -11,7 +11,7 @@ import { describe, it, expect, afterEach, beforeAll, afterAll, vi } from 'vitest'; import Fastify, { type FastifyInstance } from 'fastify'; import fastifyCookie from '@fastify/cookie'; -import { mkdtempSync, rmSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; @@ -92,6 +92,8 @@ describe('GET /api/sessions/unified', () => { afterEach(async () => { if (harness) await harness.app.close(); + // Drop any per-test transcript fixtures so other tests see an empty home. + rmSync(join(tmpHome, '.claude'), { recursive: true, force: true }); }); it('returns the {sessions,total} envelope with default (testMode) ctx', async () => { @@ -135,4 +137,31 @@ describe('GET /api/sessions/unified', () => { // limit clamps to a minimum of 1, so at most 1 row is returned. expect(body.data.sessions.length).toBeLessThanOrEqual(1); }); + + it('folds a resumed session transcript (claudeSessionId != id) into ONE row', async () => { + // Seed a real transcript fixture keyed by the Claude conversation UUID + // (the .jsonl filename stem), like a resumed session leaves behind. + const uuid = 'aabbccdd-1111-2222-3333-444455556666'; + const projDir = join(tmpHome, '.claude', 'projects', '-tmp-test-workdir'); + mkdirSync(projDir, { recursive: true }); + const line = JSON.stringify({ type: 'user', message: { role: 'user', content: 'resumed prompt' } }) + '\n'; + writeFileSync(join(projDir, `${uuid}.jsonl`), line.repeat(60)); // >4000 bytes so the scanner keeps it + + const ctx = makeLiveCtx(); + // Resumed session: the live Codeman session owns that conversation UUID. + const live = ctx.sessions.get('test-session-1') as unknown as { claudeSessionId?: string }; + live.claudeSessionId = uuid; + harness = await createEnvelopeHarness(ctx); + + const res = await harness.app.inject({ method: 'GET', url: '/api/sessions/unified' }); + expect(res.statusCode).toBe(200); + const rows = res.json().data.sessions as Array<{ sessionId: string; sources: string[] }>; + // No separate history-only row keyed by the conversation UUID… + expect(rows.map((r) => r.sessionId)).not.toContain(uuid); + // …the transcript merged into the owning live session instead. + const row = rows.find((r) => r.sessionId === 'test-session-1'); + expect(row).toBeDefined(); + expect(row!.sources).toContain('live'); + expect(row!.sources).toContain('history'); + }); }); diff --git a/test/services/unified-session-service.test.ts b/test/services/unified-session-service.test.ts index 459a11d2..04073719 100644 --- a/test/services/unified-session-service.test.ts +++ b/test/services/unified-session-service.test.ts @@ -30,6 +30,60 @@ describe('mergeUnifiedSessions', () => { expect(item.isWorking).toBe(true); }); + it('folds a resumed session transcript (claudeSessionId != id) into ONE row', () => { + const merged = mergeUnifiedSessions({ + live: [{ id: 'cm-1', status: 'working', claudeSessionId: 'uuid-resume' }], + history: [ + { + sessionId: 'uuid-resume', // transcript rows are keyed by the conversation UUID + workingDir: '/w', + sizeBytes: 7000, + lastModified: '2026-01-03T00:00:00.000Z', + firstPrompt: 'resumed prompt', + }, + ], + }); + expect(merged).toHaveLength(1); + const item = merged[0]; + expect(item.sessionId).toBe('cm-1'); + expect([...item.sources].sort()).toEqual(['history', 'live']); + expect(item.firstPrompt).toBe('resumed prompt'); + expect(item.sizeBytes).toBe(7000); + }); + + it('resolves history rows through a persisted claudeSessionId alias', () => { + const merged = mergeUnifiedSessions({ + persisted: [{ id: 'cm-2', name: 'Resumed', claudeSessionId: 'uuid-p' }], + history: [{ sessionId: 'uuid-p', workingDir: '/w', sizeBytes: 4200, lastModified: '2026-01-04T00:00:00.000Z' }], + }); + expect(merged).toHaveLength(1); + expect(merged[0].sessionId).toBe('cm-2'); + expect([...merged[0].sources].sort()).toEqual(['history', 'persisted']); + }); + + it('surfaces the NEWEST lifecycle name/mode (entries arrive newest-first)', () => { + const merged = mergeUnifiedSessions({ + // query() returns newest-first: the rename must win over the original + // name — an unconditional overwrite would leave the OLDEST standing. + lifecycle: [ + { sessionId: 'del-1', name: 'Renamed', mode: 'claude', ts: 2000, event: 'deleted' }, + { sessionId: 'del-1', name: 'Original', mode: 'shell', ts: 1000, event: 'created' }, + ], + history: [ + { + sessionId: 'del-1', + workingDir: '/w', + sizeBytes: 5000, + lastModified: '2026-01-01T00:00:00.000Z', + firstPrompt: 'hello', + }, + ], + }); + expect(merged).toHaveLength(1); + expect(merged[0].name).toBe('Renamed'); + expect(merged[0].mode).toBe('claude'); + }); + it('lets live status win over persisted (precedence)', () => { const merged = mergeUnifiedSessions({ persisted: [{ id: 's1', status: 'idle', name: 'Persisted Name' }],