mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 08:59:40 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -4,8 +4,12 @@
|
|||||||
* Combines four read-only views of a session — live (in-memory `Session`),
|
* Combines four read-only views of a session — live (in-memory `Session`),
|
||||||
* persisted (`state.json`), transcript history (`~/.claude/projects`), and the
|
* persisted (`state.json`), transcript history (`~/.claude/projects`), and the
|
||||||
* lifecycle audit log — plus mux process stats, into one de-duplicated list
|
* lifecycle audit log — plus mux process stats, into one de-duplicated list
|
||||||
* keyed by sessionId. Higher-precedence sources overwrite scalar fields when
|
* keyed by sessionId. Transcript-history rows are keyed by the Claude
|
||||||
* present (history < lifecycle < persisted < live), while the `sources` array
|
* 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
|
* always accumulates every contributing view. A "meaningfulness floor" drops
|
||||||
* noise (bare lifecycle/mux-only rows with no name and no first prompt).
|
* noise (bare lifecycle/mux-only rows with no name and no first prompt).
|
||||||
*
|
*
|
||||||
@@ -52,9 +56,11 @@ export type PersistedSessionInput = {
|
|||||||
workingDir?: string;
|
workingDir?: string;
|
||||||
createdAt?: number;
|
createdAt?: number;
|
||||||
lastActivityAt?: 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 = {
|
export type LifecycleInput = {
|
||||||
sessionId: string;
|
sessionId: string;
|
||||||
name?: string;
|
name?: string;
|
||||||
@@ -120,9 +126,23 @@ function overwrite<K extends keyof UnifiedSessionItem>(
|
|||||||
export function mergeUnifiedSessions(sources: UnifiedSources): UnifiedSessionItem[] {
|
export function mergeUnifiedSessions(sources: UnifiedSources): UnifiedSessionItem[] {
|
||||||
const map = new Map<string, UnifiedSessionItem>();
|
const map = new Map<string, UnifiedSessionItem>();
|
||||||
|
|
||||||
// 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<string, string>();
|
||||||
|
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 ?? []) {
|
for (const h of sources.history ?? []) {
|
||||||
const item = ensureItem(map, h.sessionId);
|
const item = ensureItem(map, resolveId(h.sessionId));
|
||||||
addSource(item, 'history');
|
addSource(item, 'history');
|
||||||
overwrite(item, 'workingDir', h.workingDir);
|
overwrite(item, 'workingDir', h.workingDir);
|
||||||
overwrite(item, 'sizeBytes', h.sizeBytes);
|
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;
|
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 ?? []) {
|
for (const l of sources.lifecycle ?? []) {
|
||||||
const item = ensureItem(map, l.sessionId);
|
const item = ensureItem(map, resolveId(l.sessionId));
|
||||||
addSource(item, 'lifecycle');
|
addSource(item, 'lifecycle');
|
||||||
overwrite(item, 'name', l.name);
|
if (item.name === undefined) overwrite(item, 'name', l.name);
|
||||||
overwrite(item, 'mode', l.mode);
|
if (item.mode === undefined) overwrite(item, 'mode', l.mode);
|
||||||
if (item.lastActivityAt === undefined && typeof l.ts === 'number') item.lastActivityAt = l.ts;
|
if (item.lastActivityAt === undefined && typeof l.ts === 'number') item.lastActivityAt = l.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) => ({
|
const persisted: PersistedSessionInput[] = Object.values(ctx.store.getState().sessions).map((p) => ({
|
||||||
id: p.id,
|
id: p.id,
|
||||||
name: p.name,
|
name: p.name,
|
||||||
@@ -1793,6 +1795,7 @@ export function registerSessionRoutes(
|
|||||||
workingDir: p.workingDir,
|
workingDir: p.workingDir,
|
||||||
createdAt: p.createdAt,
|
createdAt: p.createdAt,
|
||||||
lastActivityAt: p.lastActivityAt,
|
lastActivityAt: p.lastActivityAt,
|
||||||
|
claudeSessionId: p.resumeSessionId,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
// Lifecycle audit log (newest-first, capped).
|
// Lifecycle audit log (newest-first, capped).
|
||||||
|
|||||||
@@ -11,7 +11,7 @@
|
|||||||
import { describe, it, expect, afterEach, beforeAll, afterAll, vi } from 'vitest';
|
import { describe, it, expect, afterEach, beforeAll, afterAll, vi } from 'vitest';
|
||||||
import Fastify, { type FastifyInstance } from 'fastify';
|
import Fastify, { type FastifyInstance } from 'fastify';
|
||||||
import fastifyCookie from '@fastify/cookie';
|
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 { tmpdir } from 'node:os';
|
||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
|
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
|
||||||
@@ -92,6 +92,8 @@ describe('GET /api/sessions/unified', () => {
|
|||||||
|
|
||||||
afterEach(async () => {
|
afterEach(async () => {
|
||||||
if (harness) await harness.app.close();
|
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 () => {
|
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.
|
// limit clamps to a minimum of 1, so at most 1 row is returned.
|
||||||
expect(body.data.sessions.length).toBeLessThanOrEqual(1);
|
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');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -30,6 +30,60 @@ describe('mergeUnifiedSessions', () => {
|
|||||||
expect(item.isWorking).toBe(true);
|
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)', () => {
|
it('lets live status win over persisted (precedence)', () => {
|
||||||
const merged = mergeUnifiedSessions({
|
const merged = mergeUnifiedSessions({
|
||||||
persisted: [{ id: 's1', status: 'idle', name: 'Persisted Name' }],
|
persisted: [{ id: 's1', status: 'idle', name: 'Persisted Name' }],
|
||||||
|
|||||||
Reference in New Issue
Block a user